-
Notifications
You must be signed in to change notification settings - Fork 33
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
GPIO: embedded-hal 1.0 compatibility #148
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.
Could you share a minimum example that's fixed by 64c3af9? Or, could you help me understand why we need this fix?
64c3af9 adds critical sections inside routines that (indirectly) require a &mut Port<...>
exclusive reference. By requiring the exclusive reference, we've made it the user's responsibility to avoid this race condition. If they're sharing their Port
object across execution contexts, they'll need to wrap it in something like critical_section::Mutex
.
You might be right, this might have been an oversight on my part. I might have carried over some thoughts from LPSPI, where it will be necessary that an interrupt disables itself upon getting fired, making this critical section necessary there. I will revert that change. |
@mciantyre Ready for more reviews. Any leftover criticism/comments? Otherwise I'd like to merge this as a base for the other eh1 implementations. Especially as it's only a non-breaking minor change. |
@mciantyre Ping :) |
@mciantyre Ready for review again. Now update to eh-1.0 release. |
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.
I rebased onto commits that fix clippy warnings in other code, simplified the history, and added a CHANGELOG entry.
I dropped the MSRV recommendation. It's not clear why we should target 1.60, or why it's necessary to adopt an MSRV for this PR. And without a CI job, I'm afraid it would be difficult to maintain the MSRV ourselves. If you have a need for an MSRV, we can discuss in a separate issue.
A agree with all of your points. |
No description provided.