Skip to content

Conversation

@bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Oct 21, 2024

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

Since users seem to struggle with including the rom_functions.x from esp-wifi we now include everything needed in esp-hal which shouldn't hurt. Hopefully also the process of updating is a bit easier now.

Once this PR landed there will be a follow up PR to remove the linker scripts from esp-wifi-sys.

skip-changelog because of changes in esp-hal

Testing

Examples still work

@bjoernQ bjoernQ added the skip-changelog No changelog modification needed label Oct 21, 2024
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Changes LGTM, thanks for taking care of this!

@bugadani
Copy link
Contributor

bugadani commented Oct 21, 2024

skip-changelog because of changes in esp-hal

To be fair I'm pretty sure that "No need to include rom_functions.x manually" belongs to the changelog :D That, or the migration guide is in the wrong crate :)

@MabezDev MabezDev removed the skip-changelog No changelog modification needed label Oct 21, 2024
@bjoernQ
Copy link
Contributor Author

bjoernQ commented Oct 21, 2024

skip-changelog because of changes in esp-hal

To be fair I'm pretty sure that "No need to include rom_functions.x manually" belongs to the changelog :D That, or the migration guide is in the wrong crate :)

mhhhh - but that applies to esp-wifi not esp-hal (and it's mentioned in esp-wifi's changelog)

@bugadani
Copy link
Contributor

But the migration guide you edited applies to esp-hal 🙃

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Oct 21, 2024

But the migration guide you edited applies to esp-hal 🙃

oops - yeah that's the wrong crate

@bjoernQ bjoernQ added the skip-changelog No changelog modification needed label Oct 22, 2024
@bjoernQ
Copy link
Contributor Author

bjoernQ commented Oct 22, 2024

skip-changelog again - see above

@bjoernQ bjoernQ force-pushed the include-rom-api-symbols branch from a29bae3 to 2023458 Compare October 22, 2024 08:16
Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Thanks!

@MabezDev MabezDev added this pull request to the merge queue Oct 22, 2024
Merged via the queue into esp-rs:main with commit c574834 Oct 22, 2024
@bjoernQ bjoernQ deleted the include-rom-api-symbols branch November 26, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog No changelog modification needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants