-
Notifications
You must be signed in to change notification settings - Fork 221
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
SPI type erasure #2334
SPI type erasure #2334
Conversation
65aa39b
to
9414b09
Compare
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.
Very excited to have this land! This PR LGTM, two notes though:
- We should try and reorder (and maybe erase) every driver that has an instance to get that done in this release to avoid inconsistencies
- We should add a note in the API-GUIDELINES (don't worry, not in this PR but I'll add to the boiler plate tracking issue) about the param ordering so that we put the instance last so we can do this trick.
@@ -38,8 +38,8 @@ crate::peripherals! { | |||
HINF <= HINF, | |||
I2C0 <= I2C0, | |||
I2C1 <= I2C1, | |||
I2S0 <= I2S0 (I2S0), | |||
I2S1 <= I2S1 (I2S1), | |||
[I2s0] I2S0 <= I2S0 (I2S0), |
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 please elaborate what does this mean?
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.
Added a doc comment for the peripherals macro
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.
Thanks for addressing the comments. There's a lot to take in here, but overall I think it looks fine, CI and HIL are happy at least. 🤷🏻♂️
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:
Pull Request Details 📖
Changes also include #2335