esp*: updates to the programmer configuration #11646
Conversation
cladmi
added
Area: build system
cladmi
added
the
Type: enhancement
cpu/esp32/Makefile.include
Outdated
| @@ -59,7 +59,7 @@ PSEUDOMODULES += esp_wifi_any | |||
|
|
|||
| export CPU ?= esp32 | |||
| export TARGET_ARCH ?= xtensa-esp32-elf | |||
| export ESPTOOL ?= $(ESP32_SDK_DIR)/components/esptool_py/esptool/esptool.py | |||
| export ESPTOOL ?= esptool.py | |||
There was a problem hiding this comment.
If you use the esptool package from the distribution, its version might be too old. For example, Ubuntu 18.04 LTS comes with version 2.1 which should work, but Ubuntu 16.04 comes with version 0.4. The SDK comes with version 2.6.
On the other hand, the Makefile.inclue in master requires that ESP-IDF in installed which wouldn't be necessary if RIOT docker image is used and esptool in system installation is only used for flashing.
There was a problem hiding this comment.
My reasoning to completely remove $(ESP32_SDK_DIR)/components/esptool_py/esptool/ is that RESET already uses only esptool.py without the path in the ESP32_SDK_DIR. So it is required to have esptool.py in your PATH.
The esp8266 also uses esptool.py. And if it is coherent for both, I can have the same handling for the two architectures.
It may require a documentation update too.
Also a fallback with $(or $(wildcard $(ESP32_SDK_DIR)/components/esptool_py/esptool/esptool.py),esptool.py) could easily work for me, but reset still uses esptool.py directly.
It could require additional cleanups, I first just did the minimum to make it work in my configuration. (DOCKER, no toolchain).
There was a problem hiding this comment.
If you prefer, I can currently remove this change from this PR and change the way of handling it in a dedicated discussion.
There was a problem hiding this comment.
@cladmi I agree with you, that the goal should be to be able to flash the device without having the ESP32 SDK installed. It wouldn't make sense to install the ESP32 SDK when riotdocker is used for compilation. Thus, I agree to remove the whole path.
Therefore, I have checked the documentation of ESP32 MCU.
If the riotdocker is used, everything should be clear. In section Preparing the Enviroment of chapter RIOT Docker Toolchain (riotdocker) , esptool.py is documented as following:
The ESP flasher program esptool.py is available at GitHub. Don't use the the esptool package of your OS. esptool.py requires either Python 2.7 or Python 3.4 or later. The latest stable version of esptool.py can be installed with pip:
pip install esptool
If the user doesn't follow the documentation, it is his fault.
However, if the user installs the toolchain and follows the Manual Toolchain Installation, there is no word left about esptool.py. The reason is that it was supposed that esptool.py is used from $(ESP32_SDK_DIR)/components/esptool_py/esptool/ in that case.
Therefore, we have to change the documentation and I would restructure it a bit once I'm touching it.
There was a problem hiding this comment.
Thanks I will look at it.
| @@ -132,7 +132,7 @@ LINKFLAGS += -Wl,--warn-unresolved-symbols | |||
| FLASHFILE ?= $(ELFFILE) | |||
|
|
|||
| # configure preflasher to convert .elf to .bin before flashing | |||
| FLASH_SIZE = -fs 8m | |||
| FLASH_SIZE = -fs 1MB | |||
There was a problem hiding this comment.
Makes sense to avoid the warning.
boards/common/esp32/Makefile.include
Outdated
| @@ -8,4 +8,5 @@ PORT_DARWIN ?= $(firstword $(sort $(wildcard /dev/tty.SLAB_USBtoUART*))) | |||
| include $(RIOTMAKE)/tools/serial.inc.mk | |||
|
|
|||
| # reset tool configuration | |||
| RESET = esptool.py --before default_reset run | |||
| RESET ?= esptool.py | |||
| RESET_FLAGS ?= --before default_reset run | |||
There was a problem hiding this comment.
What is the advantage of splitting it into RESET and RESET_FLAGS variable?
There was a problem hiding this comment.
It is our normal scheme so it makes it compliant to the way it is done in RIOT. Which means, not another use case to handle.
Also, with it you can overwrite only the tool used for resetting. You could do RESET=path_to_my_esptool_executable only.
| @@ -9,4 +9,4 @@ include $(RIOTMAKE)/tools/serial.inc.mk | |||
|
|
|||
| # reset tool configuration | |||
| RESET ?= esptool.py | |||
| RESET_FLAGS ?= --before default_reset run | |||
| RESET_FLAGS ?= --port $(PORT) --before default_reset run | |||
cladmi
force-pushed
the
pr/esp/programmer_update
branch
from
1b354ba to
bc80d25
Compare
|
Rebased as #11649 was merged. |
Update to use RESET and RESET_FLAGS variables set conditionally.
This allows selecting the correct device when multiple boards are connected.
Append to FLASHDEPS instead of overwriting/lazy setting it.
cladmi
force-pushed
the
pr/esp/programmer_update
branch
from
bc80d25 to
4d7b421
Compare
|
I just noticed that I get .elf.bin files for some reason... |
It is what is done in the port. RIOT/cpu/esp32/Makefile.include Lines 150 to 153 in b1babe5 Before it was |
|
Is it a problem? It is simply easier than handling it with make's |
|
If you want to have just However it is a different file from the one created with Lines 499 to 500 in b1babe5 So keeping a different name is not a bad thing. For |
|
@gschorcht do you agree? Can we get this moving? |
|
@gschorcht do you want to update the |
As you prefer. I can modify #12028 to change |
|
I will remove the |
Using flash size in megabits is deprecated by esptool.
Use the new megabyte notation.
WARNING: Flash size arguments in megabits like '8m' are deprecated.
Please use the equivalent size '1MB'.
Megabit arguments may be removed in a future release.
esptool.py v2.6
cladmi
force-pushed
the
pr/esp/programmer_update
branch
from
4d7b421 to
422644b
Compare
|
Updated. |
gschorcht
added
CI: ready for build
|
Thank you for the review ! |
|
Now either the documentation or RIOT-Xtensa-ESP8266-toolchain need an update too. When following the instructions, esptool.py v2.5.1 gets installed which doesn't know about this new notation. |
|
The Update of the documentation and |
|
Oh I misread. The issue is with the |
|
Yes, see #12247 |
|
If you follow http://doc.riot-os.org/group__cpu__esp8266.html#esp8266_precompiled_toolchain you should get The flag was supported in |
|
The Precompiled Toolchain section doesn't mention I suppose a simple documentation update is enough then. (although an update to the toolchain repo would be even nicer - I really like that you can install the entire toolchain with just one command) |
|
Indeed, I searched 'esptool.py' in the page and did not notice it was the other section. |
Contribution description
This updates/cleans up some the declaration of the flasher/reset interaction.
esptool.pyand no toolchain can correctly flash the device (Split in cpu/esp*: use FLASHFILE for esp32 and esp8266 boards #11648)use esptool.py from the pathremoved from this oneRESET_FLAGSand use?=for affectationPORTforresettoo.FLASHDEPSThese updates are required for me for flashing the boards that are on another machine without toolchain with multiple boards connected. #10870
Testing procedure
Flashing and resetting with
esptool.pyinstalled from pip and without theSDKinstalled usingPORT.PORT=/dev/serial/by-id/usb-Silicon_Labs_CP2102N_USB_to_UART_Bridge_Controller_d27cb210df3de81181be72c97a7060d0-if00-port0 BOARD=esp32-wroom-32 BUILD_IN_DOCKER=1 DOCKER="sudo docker" make -C examples/default/ reset flash termPORT=/dev/serial/by-id/usb-Silicon_Labs_CP2102_USB_to_UART_Bridge_Controller_8266-if00-port0 BOARD=esp8266-esp-12x BUILD_IN_DOCKER=1 DOCKER="sudo docker" make -C examples/default/ reset flash termFLASHDEPScan already contain values, these two printsinfo-buildbefore flashingRESETcan be overwritten from the environmentIssues/PRs references
#10870
Depends on #11648