Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

micropython/espflash: A minimal ESP32 bootloader protocol implementation. #555

Closed
wants to merge 1 commit into from

Conversation

iabdalkader
Copy link
Contributor

@iabdalkader iabdalkader commented Oct 20, 2022

This is a minimal implementation of the ESP32 ROM bootloader protocol, which allows a board running MicroPython to flash a firmware image to an ESP32 chip. This is not a port of esptool.py because a) It's too complex b) It's GPL, and it's not implemented as a serial pass-through (from esptool.py <-> mcu <-> ESP32) either, because it would still require installing esptool first and running the serial pass-through code. Note this is for ESP32 not ESP32_S/C (although it may still work, because it talks to the ROM bootloader not a stub) and while it's mainly for updating the Nina WiFi firmware, I chose to implement it in Python (vs ioctls) because it makes easier to extend and use for other ESP chips if needed. Example usage, updating the Nina WiFi module firmware, copy firmware binary to storage (with mpremote or MSC) and:

import mesptool

# pre-calculated because RP2 doesn't have MD5.
md5sum = b"9a6cf1257769c9f1af08452558e4d60e"
path = "NINA_W102-v1.5.0-Nano-RP2040-Connect.bin"

esp = mesptool()
esp.bootloader()
esp.set_baudrate(921600) # or slower
esp.flash_init()
esp.flash_write(path)
esp.flash_verify(path, md5sum)
esp.reboot()

Fixes micropython/micropython#8896

Example output:

Failed to read response to command 8. # normal baudrate sync message, might repeat for 1-3 times.
Changing baudrate => 921600
Flash write size: 1129472 total_blocks: 276 block size: 4096
Writing sequence number 0/276...
Writing sequence number 1/276...
Writing sequence number 2/276...
Writing sequence number 3/276...
Writing sequence number 4/276...
Writing sequence number 5/276...
....
Writing sequence number 273/276...
Writing sequence number 274/276...
Writing sequence number 275/276...
Flash write finished
Flash verify file MD5 b'9a6cf1257769c9f1af08452558e4d60e'
Flash verify flash MD5 b'9a6cf1257769c9f1af08452558e4d60e'
Firmware write verified

@iabdalkader
Copy link
Contributor Author

iabdalkader commented Oct 20, 2022

@jimmo

I think you could plausibly make an argument for putting it in python-ecosys/esptool but perhaps micropython/esp32/esptool ? (Not sure the m prefix is necessary? Maybe we could call it espflash or something that conveys exactly what it does?)

No I agree it definitely belongs to micropython because it's not a compatible-with-reduced functionality module (in the sense of API compatibility, functionally maybe 🤔 ), but since it's a micropython-specific module it can be called anything else, the m is to avoid any issues with ESP/ "esptool" and means micropython's esptool. I can change it to espflash but I think if more functionality is added in the future, it might be limiting to name it espflash.

The other reason for putting it in micropython-lib is that we may (?) choose to freeze it into the Nano Connect

Yes, that's the plan once this settled and merged will update the manifest, name might change.

@iabdalkader iabdalkader force-pushed the add_mesptool branch 2 times, most recently from 9b0a4d6 to c8f8446 Compare October 20, 2022 06:28
@iabdalkader iabdalkader changed the title tools/mesptool.py: A minimal ESP32 bootloader protocol implementation. micropython/mesptool.py: A minimal ESP32 bootloader protocol implementation. Oct 20, 2022
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
Copy link
Member

@jimmo jimmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really keen on the "m" prefix -- we don't do that anywhere else in micropython-lib. "u" would at least be consistent, but I'd rather give it a different name (espboot, espflash, etc).

micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
micropython/mesptool/mesptool.py Outdated Show resolved Hide resolved
@iabdalkader
Copy link
Contributor Author

I'll get around to processing all the comments later today, but espflash.py is okay I guess. I also wanted to try to make this work locally, so it doubles as something you can import if the firmware on storage, or a tool you can run like espflash.py --firmware NINA_1.5.0.bin --port /dev/ttyACM0.

@dpgeorge
Copy link
Member

I also wanted to try to make this work locally,

Other tools (see eg mip and unittest) do this by having the library be a package, and then there being an additional, optional library which provides __main__.py for the (optional) command-line behaviour. So you'd install espflash for the basic behaviour, and espflash-cmdline for the command-line version (which depends on espflash).

@iabdalkader
Copy link
Contributor Author

iabdalkader commented Oct 28, 2022

Where would that optional library be ? in micropython-lib ? It would be very convenient if this is somehow accessible from /tools/ like mpremote, because then you wouldn't have to install anything, but it's also probably going to import mpremote (or duplicate some of the functions). For this to work, it will need to switch the board to raw mode, execute a serial pass-through script (something like this one in the docs), and then read/write the from the port, was thinking of using mpremote for that.

@iabdalkader iabdalkader force-pushed the add_mesptool branch 2 times, most recently from 5da3fa8 to ad6d040 Compare October 31, 2022 17:33
@iabdalkader iabdalkader changed the title micropython/mesptool.py: A minimal ESP32 bootloader protocol implementation. micropython/espflash: A minimal ESP32 bootloader protocol implementation. Oct 31, 2022
@iabdalkader
Copy link
Contributor Author

iabdalkader commented Nov 3, 2022

@dpgeorge Actually I realized I don't need to implement any of that serial passthrough stuff, because I could mpremote mount the firmware dir, which removes the copy firmware to storage step, so I just wrote this instead:

https://github.com/micropython/micropython-lib/blob/24abd009cc43accdc5be87b89a2d08d3e4380e56/micropython/espflash/espflash-cmd.py

If espflash is frozen, and mpremote is in the path, you can just do this:

python espflash-cmd.py -f ~/path/to//NINA_W102-v1.5.0-Nano-RP2040-Connect.bin

It will also generate the file MD5 hash, and pass it to the script.

@iabdalkader iabdalkader force-pushed the add_mesptool branch 2 times, most recently from ec4f431 to 2674d83 Compare November 3, 2022 22:12
@dpgeorge
Copy link
Member

dpgeorge commented Nov 4, 2022

Actually I realized I don't need to implement any of that serial passthrough stuff, because I could mpremote mount the firmware dir

That sounds much better! I see you use subprocess to run mpremote, but I think you can just as easily do import mpremote and use it as a library.

I see you intend to run espflash-cmd.py via CPython (not unix MicroPython). So that script is not really part of the espflash package but rather a completely separate thing, that should probably live on PyPI (just like mpremote lives on PyPI). There's still a question of where to host the code though...

@iabdalkader
Copy link
Contributor Author

I see you use subprocess to run mpremote, but I think you can just as easily do import mpremote and use it as a library.

Yes that works and it's much easier, but I have mpremote symlink in /bin, to always use the latest from the repo, so subprocess was just more convenient for me, but I can switch it to import mpremote.main and just call it.

So that script is not really part of the espflash package but rather a completely separate thing, that should probably live on PyPI (just like mpremote lives on PyPI). There's still a question of where to host the code though...

Yes it's not part of the package, I just wanted to share the code. I have no idea where to host it, I can remove it from the PR and try to publish it to pypi if it makes sense.

…tation.

This tool implements a subset of the ESP32 ROM bootloader protocol, and
it's mainly intended for updating Nina WiFi firmware from MicroPython,
but can be used to flash any ESP32 chip.
@iabdalkader
Copy link
Contributor Author

I think I fixed all the pending issues, anything else I missed ? I squashed all commits, renamed to espflash, and removed the command line tool for now. Note this will also generate the MD5 hash when verify is called, if hashlib is enabled, and hashlib.md5 enabled and user didn't supply a hash.

@dpgeorge
Copy link
Member

dpgeorge commented Nov 8, 2022

Thanks for updating, this looks good now. Rebased and merged in 82f6b18

@dpgeorge dpgeorge closed this Nov 8, 2022
@iabdalkader iabdalkader deleted the add_mesptool branch November 8, 2022 09:07
@robert-hh
Copy link
Contributor

@iabdalkader coming along that path again: Where do I find the actual NINA firmware, you mentioned NINA_W102-v1.5.0-Nano-RP2040-Connect.bin?

@iabdalkader
Copy link
Contributor Author

@robert-hh You can find it here, starting from 1.5.0 the binaries are attached to releases:

https://github.com/arduino/nina-fw/releases

@robert-hh
Copy link
Contributor

Thanks. I looked at https://github.com/arduino/nina-fw and did not spot it, just up to 1.4.8.

@iabdalkader
Copy link
Contributor Author

* is the path esp-hosted/esp_hosted_fg/esp/esp_driver/network_adapter the one with the code for the ESP?

Yes

* Which esp-idf version did you use to compile the ESP32 firmware?

It builds with IDF-5 just fine.

* the readme.md tells, the BT/BLE is not readily available. Does that just mean the e.g. the NIMBLE stack has to be used, like you do in the Portenta C33 port?

As far as I know yes, the host must run the BT/Networking stacks.

@robert-hh
Copy link
Contributor

Thank you for your swift reply. The firmware build fine here as well. Just a few more questions:

  • did you enable BT over UART in the sdkconfig files?
  • I noticed that the build of the ESP32 firmware included mbedtls. Does that mean that the host does not have to include mbdetls in it's build? That would be a great advantage.

@iabdalkader
Copy link
Contributor Author

Like I said, we only patched the pinout to match the C33, and no the host must have mbedtls too.

@robert-hh
Copy link
Contributor

I started to write a complaint about esp_hosted_hal_init() etc. assuming that a SPI declaration is done by device number only, but then I noticed that you declared it as MP_WEAK, allowing to have port-specific versions of these functions. That's convenient.

@robert-hh
Copy link
Contributor

robert-hh commented Jul 17, 2023

Below is a modified version of NINA's combine.py for creating a single file with the esp-hosted firmware, that can be uplaoded with your espflash.py utility. That's for ESP32. For other other ESP32 variants that offsets may differ.

#!/usr/bin/env python

import sys;

booloaderData = open("build/bootloader/bootloader.bin", "rb").read()
partitionData = open("build/partition_table/partition-table.bin", "rb").read()
otaData = open("build/ota_data_initial.bin", "rb").read()
network_adapterData = open("build/network_adapter.bin", "rb").read()

# calculate the output binary size, app offset 
outputSize = 0x10000 + len(network_adapterData)
if (outputSize % 1024):
	outputSize += 1024 - (outputSize % 1024)

# allocate and init to 0xff
outputData = bytearray(b'\xff') * outputSize

# copy data: bootloader, partitions, app
for i in range(0, len(booloaderData)):
	outputData[0x1000 + i] = booloaderData[i]

for i in range(0, len(partitionData)):
	outputData[0x8000 + i] = partitionData[i]

for i in range(0, len(otaData)):
        outputData[0xd000 + i] = otaData[i]

for i in range(0, len(network_adapterData)):
	outputData[0x10000 + i] = network_adapterData[i]

outputFilename = "esp_hosted.bin"
if (len(sys.argv) > 1):
	outputFilename = sys.argv[1]

# write out
with open(outputFilename,"w+b") as f:
	f.seek(0)
	f.write(outputData)

@robert-hh
Copy link
Contributor

note we patched the firmware to fix the pinout

I#m a little bit lost again with the ESP32 SDK.
At which place did you patch the pinout. I see it in spi_slave_api.c, but as well there seem to be settings to be made in the sdkconfig files, like for ESP_SPI_GPIO_HANDSHAKE and ESP_SPI_GPIO_DATAREADY.

@iabdalkader
Copy link
Contributor Author

At which place did you patch the pinout.

You can find the patches here along with all the other binaries, and it seems there's already a combine.py script:

https://github.com/arduino/ArduinoCore-renesas/tree/main/libraries/WiFi/extra

@robert-hh
Copy link
Contributor

robert-hh commented Jul 18, 2023

Thanks. I try to adapt this driver to the Adafruit airlift and NINA W102 hardware. So many of the pin setting have to be changed.

Did you make your changes to these files, or did you use the patches as they are in this link to modify the espressif esp_hosted files. It seems that the sdkconfig.default at your link is the sdkconfig after a build has been made of esp_hosted driver.
Looking at the patch files, it seems that the patches only affect the UART for BTM part, not the SPI interface.

b.t.w.: Does our PR for the Renesas port require to upload a different firmware to the C33 board, or is the proper firmware already installed/provided by Arduino?

What I also did not find are things like CONFIG_ESP_SPI_GPIO_DATA_READY, which are used in the code but not set at any place, not even in the build directory.

Another difficulty: the NINA/Adafruit port uses GPIO33 for the handshake. The esp_hosted code uses direct port access to set & clear the pins by creating a bit mask in the form 1 << GPIO_NR. That fits only for GPIO numbers < 32.

The whole approach would fit well for the MIMXRT port, since it makes use of LWIP. Then WiFi can coexist with Ethernet. It does not fit for the SAMD port, since including LWIP will increase the code by another ~100k, and that's too much for most SAMD51 ports. It might work for BT only.

Edit: Found the settings in idf.py menuconfig. Surprise: reading the instructions helps!

@robert-hh
Copy link
Contributor

So it compiles now, only for the MIMXRT1010 MCU it's hardly feasible. The RAM use is increased by ~30k, leaving only 32k for the heap. That's not good. It could work with MIMXRT1020 and up, like Teensy 4.x. They have much more RAM, and LWIP is already included.

@iabdalkader
Copy link
Contributor Author

Did you make your changes to these files

No those are Arduino's patches for the pins, and yes seems they only patch BT pins. The patched firmware is provided in the link, but it also ships with any C33 (firmware is preinstalled at the factory).

The whole approach would fit well for the MIMXRT port

I have a PR to add networking/bluetooth support for mimxrt (lwip/nimble/cyw43).

@robert-hh
Copy link
Contributor

I have a PR to add networking/bluetooth support for mimxrt (lwip/nimble/cyw43).

That's a very good solution, but limited to a single board, as far as I could tell. And the extension HW seems not to b readily available. Using esp_hosted or NINA fw one can use Adafruit add-on boards with the MIMXRT EVK or Teensy 4.x boards.

@iabdalkader
Copy link
Contributor Author

I'm not saying it's better, just saying that some of the work required is already done here:

micropython/micropython#11397

@robert-hh
Copy link
Contributor

Yes, I know. But, as said, there seem to be no easily available and usable breakout board for the CYW43, opposed to ESP32 boards or modules.
The PR for esp_hosted can be transferred easily to the MIMXRT port. It "just" requires adding some lines for mpconfigboard.x, Makefile and testing ............

@robert-hh
Copy link
Contributor

robert-hh commented Jul 19, 2023

There is a subtle hiccup in esp_hosted_hal.c and the way, the symbols like MICROPY_HW_WIFI_DATAREADY are use. The actual code assumes, that these are constants. In the actual MIMXRT port, they are defined e.g. as a function call, like pin_find(MP_OBJ_NEW_SMALL_INT(6)), which create a new pin object with default configuration on every call. So using expressions like mp_hal_pin_write(MICROPY_HW_WIFI_SPI_CS, 0) may fail.. So it's not critical, but "just" some overhead.

@robert-hh
Copy link
Contributor

Some progress with a MIMXRT1020 EVK board. That board has wired LAN too, but I cannot run them both at the same time. The esphosted variant connects and supports basic operations. Still slow and not overly reliable. Ping success rate is low and slow. Ping IN is fine, Ping out success rate is about 50%.
What took me longest was the pin selection in the esphosted firmware. The default is Pin 2 for handshake. But Pin 2 is also a strapping pin, causing the ESP32 firmware to stall. Changing it to a different Pin made it work.
When in extmod/esphosted DEBUG is enabled, I get a lot of warning about invalid data frames. When tracing the SPI bus, I see a lot of transmissions with all bits 0.

The test adapter is a Arduino shaped hat with a generic ESP32 handwired to the pins of the hat. That way I hav access to all pins and can load firmware using the ESP32's USB port.

@iabdalkader
Copy link
Contributor Author

iabdalkader commented Jul 19, 2023

Ping success rate is low and slow. Ping IN is fine, Ping out success rate is about 50%.

Yes I noticed UDP performance is bad, the udp unittests always fails, I have no idea why, might be something to tune in IDF config, otherwise everything else seems okay.

When in extmod/esphosted DEBUG is enabled, I get a lot of warning about invalid data frames. When tracing the SPI bus, I see a lot of transmissions with all bits 0.

I know, it seems to be a bug in the firmware it just sends a buffer full of zeros, but it's harmless.

@robert-hh
Copy link
Contributor

I have a simple udp echo test with a small server on a RPi in my LAN. That test works fine, albeit slow.
So I have to do some polishing to make it more reliable, and have to change the ESP32 firmware, such that Pin 33 can be used for handshake or data_ready. I do not know why they made direct port access instead of the API calls. That is a useless optimization.

@robert-hh
Copy link
Contributor

Attached is a copy of the esp_hosted_hal.c file, adapted for the mimxrt port. Instead of using the #define macro names for the pins, it assigns them to local variables and uses these. That way, fewer assumption are made about the content of the defines. Only that they create a lvalue of type mp_hal_pin_obj_t.

esp_hosted_hal.zip

@robert-hh
Copy link
Contributor

Now I started to get BLE running with the ESP hosted firmware. The Arduino patches you linked were helpful, but there was more to be configured, especially telling the ESP hosted firmware to use UART instead of SPI. The complication with the Adafruit modules is, that they have UART at pins 1 and 3, which is as well used a log port of the esp_hosted firmware. But BLE overrides that, and so it seems to work now. Baud rate switching and test is t.b.d.

I do not know whether in in the esp_hosted firmware both WiFi and Bluetooth is active at the same time. Otherwise the SPI pins could be used for UART. Maybe using SPI for communication is an option.

b.t.w.: I see that the Copyright notice for the RA port's mpbthciport.c file is now Arduino SA, while a previous version for the RP2 port shows your name, even if these two files are mostly the same. And all are in the essential logic similar to the initial version made by Damien and more similar to the version I tailored for SAMD. So I think it's not fair to omit Damien or you in the notice.

@iabdalkader
Copy link
Contributor Author

b.t.w.: I see that the Copyright notice for the RA port's mpbthciport.c file is now Arduino SA, while a previous version for the RP2 port shows your name, even if these two files are mostly the same

If you remove the license and weak/empty functions, mpbthciport.c in renesas-ra and stm32 for example have only 34% in common (including function names), so no they are not mostly the same, that needed to be said and to be clear, but I will add Damien to the file.

@robert-hh
Copy link
Contributor

Test coverage looks much better than for the NINA fw:

18 tests performed
12 tests passed
2 tests skipped: multi_bluetooth/ble_l2cap.py multi_bluetooth/perf_l2cap.py
4 tests failed: multi_bluetooth/ble_deepsleep.py multi_bluetooth/ble_mtu_peripheral.py multi_bluetooth/perf_gatt_notify.py multi_bluetooth/stress_log_filesystem.py

about (C): I have a version of mpbthciport.c adapted for SAMD, which looks almost identical to the renesas version, besides the way the UART is configured. For that version I kept your name in the copyright notice. I cannot recall from which port it was, but cannot be the Renesas one, and the rp2 one is different as well. Maybe a mix of all.
b.t.w.: Did you move from openmv.io to Arduino?

@iabdalkader
Copy link
Contributor Author

cannot recall from which port it was, but cannot be the Renesas one, and the rp2 one is different as well. Maybe a mix of all.

Most likely from the rp2 port, I sent that one too and it was adapted from stm32 (they all are) but this renesas work is made specifically for Arduino, so it has Arduino copyright notice, and I added Damien too.

b.t.w.: Did you move from openmv.io to Arduino?

No I just work full-time for Arduino now.

@robert-hh
Copy link
Contributor

Most likely from the rp2 port,

Kind of. The rp2 port uses the Pico alarm mechanism for timing, while the SAMD one uses SoftTimer like the STM32. So it's more likely an evolution of both.

@robert-hh
Copy link
Contributor

but I will add Damien to the file.

With all the forth & back, the (C) note of Damien disappeared again. And keeping it is not only a matter of respect. Reading the Copyright note it seems clear to me that even if you modify code and add your own code to it, the initial Copyright notice must be retained. And because it's hard to write code for MicroPython which does not contain code designed by Damien, I keep him in every file.

@iabdalkader
Copy link
Contributor Author

With all the forth & back, the (C) note of Damien disappeared again

Which file are you referring to ? Please comment on the PR/file you think is missing a License. I added Damien to all of the BT files copyright notices.

@robert-hh
Copy link
Contributor

I thought I had looked at mpbthciport.c. Looking again, Damien is indeed mentioned. Thank you.

On a completely different topic:
Last week I spent a day to implement at the MIMXRT esp_hosted trial a callback on rx line idle, which is called at the end of a received data chunk. The hardware supports it, only I had to modify fsl_lpuart.c to work like is should. In that callback mp_bluetooth_hci_poll_now() is called, similar to the STM32 port. Technically it works, and I can see in the logic analyzer that the response to a message from the esp_hosted firmware is sent earlier, usually about after 1-2 ms instead polling_time/2. But a) the total execution time of the BLE tests does not improve, b) transfer rates in the perf_xxx.py tests are the same, and c) the test coverage is somewhat worse. Maybe a) and b) are not surprising. Most of the time the board waits for data exchange cycle. But c) is disappointing.

@iabdalkader
Copy link
Contributor Author

I still can't test anything right now, I will be back next week, but I'm not too concerned about performance at the moment, I only want to get something stable merged then anyone is welcome to improve it.

@robert-hh
Copy link
Contributor

robert-hh commented Aug 15, 2023

No problem. The Portenta C33 port has to be merged first as the baseline. I do not know whether the C33 UART support an idle event like the STM32 or MIMXRT. The rp2 and SAMD do not.

@iabdalkader
Copy link
Contributor Author

I thought I had looked at mpbthciport.c. Looking again, Damien is indeed mentioned. Thank you.

@robert-hh maybe you meant the mpbthciport.c from the imxrt CYW43 support PR ?

https://github.com/micropython/micropython/pull/11397/files#diff-4b7d7538852dcd8a55e7c899c645e989058b83c508673de245b8104c664e74a2

Although I did write it all, including sched_node bits which I first used in rp2 as far as I remember, but it does share the stubs, so I will amend the license for that one too.

@robert-hh
Copy link
Contributor

Maybe it was this one. It looks as well pretty similar to the mpbthciport.c file of the stm32 port. Anyhow it's hard to write code for MicroPython without using code and designs made by Damien.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arduino RP2 Nano connect: Unexpected OSError (regression)
4 participants