search

esp-rs / espflash

Serial flasher utility for Espressif SoCs and modules based on esptool.py
Apache License 2.0
455 stars 110 forks source link

Add `checksum-md5` command #536

Closed bjoernQ closed 6 months ago

bjoernQ commented 6 months ago

This adds a checksum-md5 command to espflash.

e.g.

❯ espflash checksum-md5 --address 0x10000 --length 0x1000 --no-stub
[2023-12-27T11:39:27Z INFO ] Serial port: 'COM22'
[2023-12-27T11:39:27Z INFO ] Connecting...
[2023-12-27T11:39:27Z WARN ] Setting baud rate higher than 115,200 can cause issues
0x95d0df6456de4ac114251a67a5f85

While this is not too exciting on its own, it's the ground work for #259 and #479

It's very unfortunate that the Rust flasher-stub currently doesn't implement the required command while the ROM code and the C based flasher stub does. (Verified by temporarily reverting the stubs locally). Maybe we should revert the change to use the Rust flasher-stub (but use it based on a feature at build time) until it's supported there

I checked the result with reading a flash region with esptool and running md5sum on it.

SergioGasquez commented 6 months ago

It's very unfortunate that the Rust flasher-stub currently doesn't implement the required command while the ROM code and the C based flasher stub does. (Verified by temporarily reverting the stubs locally). Maybe we should revert the change to use the Rust flasher-stub (but use it based on a feature at build time) until it's supported there:

The initial idea of the Rust stub, was to put them behind a feature, but we decided not to do that because nobody will use the feature. That being said, we now have an argument to do so or to go back to the C stubs or just force to use --no-stub for this subcommand until https://github.com/esp-rs/esp-flasher-stub/issues/43 is implemented

bjoernQ commented 6 months ago

It's very unfortunate that the Rust flasher-stub currently doesn't implement the required command while the ROM code and the C based flasher stub does. (Verified by temporarily reverting the stubs locally). Maybe we should revert the change to use the Rust flasher-stub (but use it based on a feature at build time) until it's supported there:

The initial idea of the Rust stub, was to put them behind a feature, but we decided not to do that because nobody will use the feature. That being said, we now have an argument to do so or to go back to the C stubs or just force to use --no-stub for this subcommand until esp-rs/esp-flasher-stub#43 is implemented

BTW I also currently cannot flash the H2 with the current stub - we really need to be very confident that the stub works for us (including MD5) before we force every user to use it (while I agree that no average user will use such a feature we could agree to use it in our team - it's kind of a chicken and egg problem)

Update: same for C6

SergioGasquez commented 6 months ago

Current main of esp-flasher-stub should be able to flash all targets, we should probably cut a release and update them in espflash. But I agree, current state of espflash is not good as you can't flash many targets, I'm happy to add the Rust stubs behind a feature.

SergioGasquez commented 6 months ago

In any case, that behind the scope of this PR, I'll do some testing tomorrow morning and update with the results!

bjoernQ commented 6 months ago

I locally updated the flasher stubs ( commit 2b0b0f0de5020e6d5e2bf39e8950d49737fd5edb ) and it works for all chips - also flashing H2 works now for me

I'm still not sure if we should add a feature to choose the old vs the new stub. e.g. the new stub doesn't support 26 MHz xtals - maybe having at least a feature to force using the old stubs might be helpful for those users (but certainly not scope of this PR)

SergioGasquez commented 6 months ago

I'm still not sure if we should add a feature to choose the old vs the new stub. e.g. the new stub doesn't support 26 MHz xtals - maybe having at least a feature to force using the old stubs might be helpful for those users (but certainly not scope of this PR)

Is the 26 MHz a stub issue? I don't have any 26 MHz board so I cant test it. We have https://github.com/esp-rs/espflash/issues/526, but that's a bootloader issue. This definitely needs some investigation, so I'll try to get some 26 MHz devkit

bjoernQ commented 6 months ago

I'm still not sure if we should add a feature to choose the old vs the new stub. e.g. the new stub doesn't support 26 MHz xtals - maybe having at least a feature to force using the old stubs might be helpful for those users (but certainly not scope of this PR)

Is the 26 MHz a stub issue? I don't have any 26 MHz board so I cant test it. We have #526, but that's a bootloader issue. This definitely needs some investigation, so I'll try to get some 26 MHz devkit

Yes it works with --no-stub ... so probably we don't need that feature ... I forgot there is --no-stub 😆