`check-app-descriptor true` is `esp-hal`-specific

[ivmarkov]

It is esp-hal specific, because it searches for the esp_app_desc symbol in an ELF section whose name is hard-coded to be .rodata_desc.

While the esp-hal linker script is putting the esp_app_desc symbol in a section under that name I'm sure, the linker script of ESP-IDF is a bit more complex than that, in that it merges the .rodata_desc section as well as a few others - in predefined order though - into a larger .flash.rodata section.

So if espflash is to support ESP-IDF app images (Rust or C ones), then this check needs to be relaxed. Perhaps, by also searching in a section named .flash_rodata.

Sure, one can always skip the check with --check-app-descriptor false, but you could imagine - with the new espflash release - we have to explain this to every single user trying out the esp-idf-* crates which is not ideal.

Furthermore, I would suggest relaxing the error message to mention that using the esp-bootloader-esp-idf is esp-hal specific, i.e.:

• Now: "The app descriptor is not present in the project. You need to add the https://github.com/esp-rs/esp-hal/tree/main/esp-bootloader-esp-idf to your project."
• Then: "The app descriptor is not present in the project. If using esp-hal, add the https://github.com/esp-rs/esp-hal/tree/main/esp-bootloader-esp-idf to your project. If using esp-idf-*, the app descriptor is added implicitly, but you can override its default values by declaring esp_idf_svc::sys::esp_app_desc! {}".
• Then, Option 2: "The app descriptor is not present in the project. If using esp-hal, add the https://github.com/esp-rs/esp-hal/tree/main/esp-bootloader-esp-idf to your project. (As this error should never, ever appear when using ESP-IDF anyway. Reason is, even if the user does not call the esp_app_desc macro, there is weakly-defined esp_app_desc which is always present in ESP-IDF, so users need the explicit macro only prior to prod release, when they want their true Cargo binary package and other stuff present in the app desc)

[ivmarkov]

@bugadani FYI ^^^

[MabezDev]

While the esp-hal linker script is putting the esp_app_desc symbol in a section under that name I'm sure, the linker script of ESP-IDF is a bit more complex than that, in that it merges the .rodata_desc section as well as a few others - in predefined order though - into a larger .flash.rodata section.

Ah my bad, I thought they were both using the same section names. We can definitely add an additional check for the idf linker layout.

Furthermore, I would suggest relaxing the error message to mention that using the esp-bootloader-esp-idf is esp-hal specific, i.e.:

I started to agree with this, but your last point:

As this error should never, ever appear when using ESP-IDF anyway. Reason is, even if the user does not call the esp_app_desc macro, there is weakly-defined esp_app_desc which is always present in ESP-IDF, so users need the explicit macro only prior to prod release, when they want their true Cargo binary package and other stuff present in the app desc

If idf users won't ever run into it, is there much point in renaming the message? One thing we could do is have a more generic message (i.e, app desc not found), but we can also detect if the elf is from esp-hal with metadata in the elf (I think, cc: @bugadani @bjoernQ ) and output the extra message about depending on esp-bootloader-esp-idf. What do you think?

[ivmarkov]

I started to agree with this, but your last point:

As this error should never, ever appear when using ESP-IDF anyway. Reason is, even if the user does not call the esp_app_desc macro, there is weakly-defined esp_app_desc which is always present in ESP-IDF, so users need the explicit macro only prior to prod release, when they want their true Cargo binary package and other stuff present in the app desc

If idf users won't ever run into it, is there much point in renaming the message? One thing we could do is have a more generic message (i.e, app desc not found), but we can also detect if the elf is from esp-hal with metadata in the elf (I think, cc: @bugadani @bjoernQ ) and output the extra message about depending on esp-bootloader-esp-idf. What do you think?

I agree with you and my favorite is actually "Then, Option 2" which only adds "If using esp-hal" to the existing message. Which I think is not bad, given that the suggested approach (esp-bootloader-esp-idf) is baremetal-specific anyway.

[bjoernQ]

Only doing the check if we know it's an esp-hal based project (which we can know by looking at the esp-hal specific metadata) is probably the best option