-
Notifications
You must be signed in to change notification settings - Fork 253
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
Move interrupt related bits out of lib.rs and into the interrupt module #2613
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
esp-hal/CHANGELOG.md
Outdated
@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
### Fixed | |||
|
|||
- Xtensa devices now correctly enable the `esp-hal-procmacros/rtc-slow` feature (#2594) | |||
- Removed interrupt related items from lib.rs, moved to the `interrupt` module (#2613) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Removed interrupt related items from lib.rs, moved to the `interrupt` module (#2613) | |
- Moved interrupt related items from lib.rs to the `interrupt` module (#2613) |
Besides the CI run this looks good to me - one thing I just discovered: Shouldn't we have an inherent |
Frankly speaking I don't see too much value in the trait at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, would also probably like to see InterruptConfigurable
go away unless there is some reason to keep it, but given that it already existed can be done here or in another PR, doesn't matter to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - instead of removing InterruptConfigurable
we can also just make sure we have an inherent set_interrupt_handler
function everywhere - but not in the scope of this PR
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 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra: