espressif / esptool

Espressif SoC serial bootloader utility
https://docs.espressif.com/projects/esptool
GNU General Public License v2.0
5.6k stars 1.39k forks source link

Image2hex (ESPTOOL-818) #959

Closed LeZerb closed 4 months ago

LeZerb commented 9 months ago

This modification adds a function image2hex which will parse an image and generate an intel hex file containing all segments from the provided image.

I have tested this change with the following hardware & software combinations:

Windows 11, ESP32C3 SuperMini

I have run the esptool.py automated integration tests with this change and the above hardware:

NO TESTING

github-actions[bot] commented 9 months ago
Warnings
:warning: **Some issues found for the commit messages in this PR:** - the commit message `"added image2hex command"`: - *summary* looks empty - *type/action* looks empty *** **Please fix these commit messages** - here are some basic tips: - follow [Conventional Commits style](https://www.conventionalcommits.org/en/v1.0.0/) - correct format of commit message should be: `(): `, for example `fix(esp32): Fixed startup timeout issue` - allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` - sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter - avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the [Conventional Precommit Linter](https://github.com/espressif/conventional-precommit-linter)).

👋 Hello LeZerb, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by :no_entry_sign: dangerJS against 77f79fecf705afa58943aaba089bdc47d477d167

radimkarnis commented 9 months ago

Hello @LeZerb, thank you very much for your contribution and the effort to improve esptool!

esptool supports generating intel files! Please see the command marge_bin. You can specify the output to be in the hex format using the --format option. Please see the related documentation.

We try not to add new commands to esptool if not absolutely needed. The proposed image2hex command can already be achieved by running the elf2image and merge_bin commands. Even if your contribution can save one command run in some cases (generating the hex file from elf file directly), for this reason, I ama afraid it cannot be accepted.

Do you have any specific use case where such a command is needed?

LeZerb commented 9 months ago

Hello, thanks for your response.

Probably I am misunderstanding your description. I do not see a way to get from an ESP image to a hex file. I do not have the elf file available (previous build that I did not correctly archive).

The use case for me was that I needed to binary patch an ESP image. Since the image has a magic byte and a checksum it cannot be easily done directly. So I wanted to convert the image first into an editable file (in my case a hex file where I don't "lose" the address information). Then I edited the hex file. And then I wanted to convert back to an ESP image (PR 960).

radimkarnis commented 9 months ago

I do not see a way to get from an ESP image to a hex file

You can run: 1) esptool.py --chip esp32c3 elf2image image.elf --output binary.bin 2) esptool.py --chip esp32c3 merge_bin 0x0 binary.bin --format hex --output binary.hex

This way you'll convert an elf file to hex like this: elf -> bin -> hex. Running merge_bin also allows you to use other options like --flash_freq, --flash_mode, --flash_size to edit the image header.

LeZerb commented 9 months ago

I understand that.

In my use case however, I do not have the elf file to begin with. I am starting with an ESP application binary image.

ESP image -> hex -> ESP image

radimkarnis commented 9 months ago

In that case, just skip step 1).

merge_bin can do conversion from ESP application binary image to a hex file (and back to binary).

LeZerb commented 9 months ago

I cannot see how this would work. From what I can see in the codebase there is nothing in there that would do what I expect.

When converting from the ESP image to hex I need to provide an address and then merge_bin just puts the whole binary blob at that address. What I would expect is that the segments that are defined in the ESP image are also present in the hex file. Providing an address in that case should not be necessary at all.

Maybe an example is helpful. This is output from image_info

Entry point: 40380250 6 segments

Segment 1: len 0x82318 load 0x3c0d0020 file_offs 0x00000018 [DROM] Segment 2: len 0x02fc8 load 0x3fc9a400 file_offs 0x00082338 [DRAM,BYTE_ACCESSIBLE] Segment 3: len 0x0ad08 load 0x40380000 file_offs 0x00085308 [IRAM] Segment 4: len 0xc54f8 load 0x42000020 file_offs 0x00090018 [IROM] Segment 5: len 0x0f500 load 0x4038ad08 file_offs 0x00155518 [IRAM] Segment 6: len 0x00004 load 0x50000010 file_offs 0x00164a20 [RTC_IRAM,RTC_DRAM]

What I would expect is that the output hex file also contains exactly those sections with the gaps inbetween. At least that is what this PR is trying to achieve.

peterdragun commented 9 months ago

Hi @LeZerb,

When converting from the ESP image to hex I need to provide an address and then merge_bin just puts the whole binary blob at that address.

Correct, with one small addition, merge_bin can merge binaries, shift them and also convert them to necessary format. So in your case you can provide address 0x0 which won't do any shifting. If you provide just one binary, it will also skip the merging part and you can use the last mentioned feature to just convert from raw binary file to hex format. This was suggested by Radim in one of the comments above.

What I would expect is that the segments that are defined in the ESP image are also present in the hex file.

Those segments of original image should be there in hex file as well. When doing just the conversion the hex file should contain exactly the same data as the binary, but in a different format. Converting back to binary should result in same data. Have you tried running command suggested by Radim and there was something missing?

Providing an address in that case should not be necessary at all.

For your use case it is not needed, but we are using a command that is used in a first place for merging the binaries, the conversion is just additional feature that this command can do. We are trying to avoid adding a new command in case the same can be done by adding correct arguments to already existing command.

The commands that you are trying to add can be done like this:

  1. Convert bin to hex with: esptool.py --chip esp32s3 merge_bin 0x0 blink.bin --format hex --output blink.hex
  2. Update the hex per your needs
  3. Convert updated hex back to bin: esptool.py --chip esp32s3 merge_bin 0x0 blink.hex --format raw --output blink2.bin

If I don't change anything in the hex file I get exactly the same response from the image info. Btw you can even check the produced hex file directly with image_info command and verify that all segments are there at the same place.

See the output below, I have used a blink example that was built for ESP32-C6, converted it with a command mentioned above and then used image_info on hex file directly. I get the same response if I run the image_info on original bin, hex, a file converted twice: bin->hex->bin:

esptool.py image_info blink.hex                                                                                                                              
esptool.py v4.7.0
File size: 167216 (bytes)
Detected image type: ESP32-C6
Image version: 1
Entry point: 40800254
5 segments

Segment 1: len 0x0a440 load 0x42018020 file_offs 0x00000018 [IROM]
Segment 2: len 0x05bb0 load 0x40800000 file_offs 0x0000a460 [DRAM,BYTE_ACCESSIBLE,IRAM]
Segment 3: len 0x1236c load 0x42000020 file_offs 0x00010018 [IROM]
Segment 4: len 0x05608 load 0x40805bb0 file_offs 0x0002238c [DRAM,BYTE_ACCESSIBLE,IRAM]
Segment 5: len 0x01360 load 0x4080b1c0 file_offs 0x0002799c [DRAM,BYTE_ACCESSIBLE,IRAM]
Checksum: 0d (valid)
Validation Hash: 55aaac7088b50906518d48eb21b831d3950b6cf8e5c00eb51a5c256173a94ac3 (valid)

Have you tried the commands mentioned above and you experienced any issues with them? If yes, please share more details so we can investigate, this should work.

LeZerb commented 9 months ago

I think we are still not talking about the same thing.

Lets say I have an ESP image file blink.bin (lets say the size would be 0x3020) with the following content

Header: 0x20 bytes at @ 0x0 Segment 1: 0x1000 bytes at 0x10000 Segment 2: 0x2000 bytes at 0x20000

When I call "esptool.py --chip esp32s3 merge_bin 0x0 blink.bin --format hex --output blink.hex" I am getting a hex file that has 1 segment sized 0x3020 at offset 0.

What I would expect is the hex file to not include the header at all and to define 1 segment of 0x1000 bytes at 0x10000 and 1 segment of 0x2000 bytes at offset 0x20000.

The next step would then be to edit this hex file and then convert the hex file back to an ESP image including a valid header (other PR)

radimkarnis commented 8 months ago

Hi @LeZerb, we have discussed this and we agree that the hex file shouldn't have 1 segment, but the same amount and distribution as the original image. Also, if we want to do this, we need to keep the header and footer information.

We plan to change the current merge_bin implementation to do this. Unfortunately, we would rather not add any new commands.

For this reason, this PR won't be accepted. but we'll keep this open until your desired functionality can be achieved with merge_bin.

peterdragun commented 4 months ago

Hi @LeZerb, I am sorry for the late reply. I have investigated this further and we have discussed this also with our team. Unfortunately, I don't think we can implement this feature. When converting to IntelHex we need to keep the information that is stored in the header and footer of binary file. Using your approach we would lose a lot of necessary metadata when flashing the binary. There isn't any good way how we could maintain this metadata in IntelHex. It would require a lot of changes to the current approach with little or no real benefits other than hacking the binary content.

The current approach is easy to maintain while also providing an advantage over raw binary files in skipping the filler bytes, which was the main goal of this feature.

Because of those reasons, we have decided to keep the current implementation of IntelHex as is for now. Thank you for understanding.