-
Notifications
You must be signed in to change notification settings - Fork 251
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
Slight cleanup throughout #2575
Conversation
@@ -601,10 +601,6 @@ impl<'a> I2cFuture<'a> { | |||
|
|||
w.arbitration_lost().set_bit(); | |||
w.time_out().set_bit(); | |||
|
|||
#[cfg(esp32)] |
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.
These are part of I2cFuture
which we don't even implement for esp32
|
||
| Feature | Supported CPUs | | ||
| --------- | ---------------- | | ||
| `esp32` | ESP32 (_LX6_) | |
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.
The features don't exist
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 guess having to update the version of critical-section
in two places is fine given they appear very close together (and it would be fine even if not)
I wonder if we should have direct dependencies on xtensa-lx
and riscv
given they are re-exported from the rt
crates? But upsteam riscv-rt
doesn't so maybe just keep it
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, LGTM
Split out of #2357 as these don't really belong there - most of the changes were done because of PAC updates, which landed separately.