Skip to content

Check app_desc _only_ for esp-hal projects #892

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JurajSadel
Copy link
Contributor

Note: For testing non-esp-hal based project, I used ESP-IDF's hello_world example, and I hope it is valid test.

closes #886

@JurajSadel JurajSadel requested a review from MabezDev June 18, 2025 13:54
@JurajSadel JurajSadel added the skip-changelog Skips the changelog CI check label Jun 18, 2025
if !section || !symbol {
// esp-hal specific, because it searches for the hard-coded esp_app_desc symbol
// in an ELF section
if esp_hal && (!section || !symbol) {
return Err(Error::AppDescriptorNotPresent).into_diagnostic();
Copy link
Member

Choose a reason for hiding this comment

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

I think this bit still needs some more work, based on my comment here: #886 (comment)

We should still return an error if the section is missing from either type of binary, but we should only tell users to use esp-bootloader-idf if we detect that it's an esp-hal binary. I'm hoping we can customize the error context with thiserror or miette for this purpose 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skips the changelog CI check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check-app-descriptor true is esp-hal-specific
2 participants