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

Save each partition as individual binaries #717

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chris-subtlebytes
Copy link

Fixes #714

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

I guess we need to decide a solution, I like what @ivmarkov suggested here: #714 (comment)

I would try to avoid having major breaking changes as we recently released 3.0

@chris-subtlebytes
Copy link
Author

chris-subtlebytes commented Dec 31, 2024

What are our options?

  1. Add new commands save-firmware, save-part-table, and save-bootloader.

    From a user point of view, it becomes unwieldy to juggle three different commands especially because the partition table and partition table offset needs to be specified for each one. I already needed to juggle a handful while signing and encrypting. Remembering to update three different commands would have caused many more errors on my end. I was modifying the partition table to add nvs-keys, modifying sdkconfig.defaults to change bootloader flags, and messing about with the actual firmware image all at the same time. Having many commands that all need the same flags (bootloader and partition table) increases the odds that one of them has an incorrect value when the user is running each one separately.

  2. Break compatibility by making --merge work as per docs and flag.

    Most users are going to use espflash flash, which isn't affected. I didn't need to save-image until I needed to sign and encrypt the bootloader and app image. Nobody can do that today because the functionality is broken. Every use case for save-image I can think of usually requires the individual parts, such as OTAs or inspecting the ELF.

    In my opinion, this still beta software that should be expected to break for bugfixes. If anybody has strong opinions on stability, I would hope they use esptool. The most impact a user would have is to add --merge to their command.

  3. Update the default for --merge to true, but that's a bit tricky according to Add support for automatic negation flags clap-rs/clap#815

    This keeps the default behavior and docs the same.

  4. Rename the --merge flag to --separate-bins
    This is similar to reversing the default value for --merge but still causes a minor breakage. I don't think anyone is using --merge because it's broken.

  5. Is there any inspiration we can draw from esptool? I haven't used it much.

@ivmarkov
Copy link
Contributor

ivmarkov commented Dec 31, 2024

What are our options?

  1. Add new commands save-firmware, save-part-table, and save-bootloader.
    From a user point of view, it becomes unwieldy to juggle three different commands especially because the partition table and partition table offset needs to be specified for each one.

I don't think this statement is correct.
(Please check my understanding below, and if you think I'm wrong on any of those, if you can show the relevant code in espflash or ESP-IDF, really important to get this straight. :-) )

  1. save-bootloader would not need neither the partition table, nor the partition table offset.

While save-bootloader can patch the built-in bootloader binary image to support a different flash size, turns out - just checked this morning - it cannot patch the built-in bootloader to support a different partition table offset than the default 0x8000 (@SergioGasquez right?).

Also, if you need a bootloader with a different partition table offset than 0x8000 OR a bootloader with secure boot capabilities (as I have doubts the built-in one is compiled with secure boot capabilities), save-bootloader is useless to you. I - for one - am instead using the custom-built bootloader that you always get with every single esp-idf-*-based binary crate, out of the box.

save-bootloader might use a user-supplied partition table offset to only check whether the built-in bootloader fits before the parrtition table start, but that's it.

  1. save-firmware does not need the partition table itself or its offset either for the same reasons: it cannot patch an already compiled app image to a different partition table offset. The app image needs to know the partition table offset (but not the partition table CSV or BIN!) during OTA (updated: and also for all esp_partition_t-based APIs of course). The only way to do it is to compile the app image from the beginning with a new value for CONFIG_PARTITION_TABLE_OFFSET (if you use the esp-idf-* crates) or whatever ad-hoc method you use for baremetal, as OTA updates are not yet "officially" supported there, with esp-hal.

save-firmware might take advantage of a user-provided partition-table to check if the app image being converted to .BIN fits in the factory or one of the OTA partitions of the table, but that's it. And the current check for this is anyway a bit heuristical, as save-image can't actually know against which partition in the partition table to check the size of the app image anyway.

  1. Obviously, save-part-table does not need to know the partition table offset either.

I already needed to juggle a handful while signing and encrypting. Remembering to update three different commands would have caused many more errors on my end. I was modifying the partition table to add nvs-keys, modifying sdkconfig.defaults to change bootloader flags, and messing about with the actual firmware image all at the same time. Having many commands that all need the same flags (bootloader and partition table) increases the odds that one of them has an incorrect value when the user is running each one separately.

I get it that it is difficult (I'm suffering myself through this), but we also have to be crystal clear among the three of us (@SergioGasquez included) what we need and what we don't / won't need/do w.r.t. support for Secure Boot in espflash.

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