Skip to content
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

embedded-hal-bus no longer builds with target thumbv6m-none-eabi #598

Open
Tschrock opened this issue Apr 26, 2024 · 5 comments
Open

embedded-hal-bus no longer builds with target thumbv6m-none-eabi #598

Tschrock opened this issue Apr 26, 2024 · 5 comments

Comments

@Tschrock
Copy link

Added embedded-hal-bus = "0.2.0" to my project and it failed to build.
Using target thumbv7m-none-eabi works, using target thumbv6m-none-eabi fails. Version 0.1.0 works.

Trying to build the latest master branch:

./embedded-hal-bus> cargo build --target thumbv6m-none-eabi
   Compiling embedded-hal v1.0.0 (/home/cyber/Documents/VSCode/embedded-hal/embedded-hal)
   Compiling portable-atomic v1.6.0
   Compiling embedded-io-adapters v0.6.1 (/home/cyber/Documents/VSCode/embedded-hal/embedded-io-adapters)
   Compiling embedded-io-async v0.6.1 (/home/cyber/Documents/VSCode/embedded-hal/embedded-io-async)
   Compiling embedded-hal-async v1.0.0 (/home/cyber/Documents/VSCode/embedded-hal/embedded-hal-async)
   Compiling nb v1.1.0
   Compiling embedded-io v0.6.1 (/home/cyber/Documents/VSCode/embedded-hal/embedded-io)
   Compiling embedded-hal-bus v0.2.0 (/home/cyber/Documents/VSCode/embedded-hal/embedded-hal-bus)
   Compiling critical-section v1.1.2
   Compiling embedded-can v0.4.1 (/home/cyber/Documents/VSCode/embedded-hal/embedded-can)
   Compiling embedded-hal-nb v1.0.0 (/home/cyber/Documents/VSCode/embedded-hal/embedded-hal-nb)
error[E0599]: no method named `compare_exchange` found for struct `portable_atomic::AtomicBool` in the current scope
   --> embedded-hal-bus/src/i2c/atomic.rs:122:14
    |
120 | /         self.bus
121 | |             .busy
122 | |             .compare_exchange(
    | |             -^^^^^^^^^^^^^^^^ method not found in `AtomicBool`
    | |_____________|
    | 

error[E0599]: no method named `compare_exchange` found for struct `portable_atomic::AtomicBool` in the current scope
   --> embedded-hal-bus/src/spi/atomic.rs:127:14
    |
125 | /         self.bus
126 | |             .busy
127 | |             .compare_exchange(
    | |             -^^^^^^^^^^^^^^^^ method not found in `AtomicBool`
    | |_____________|
    | 

For more information about this error, try `rustc --explain E0599`.
error: could not compile `embedded-hal-bus` (lib) due to 2 previous errors

Seems to be caused by #593

self.bus
.busy
.compare_exchange(
false,
true,
core::sync::atomic::Ordering::SeqCst,
core::sync::atomic::Ordering::SeqCst,
)
.map_err(|_| AtomicError::Busy)?;

self.bus
.busy
.compare_exchange(
false,
true,
core::sync::atomic::Ordering::SeqCst,
core::sync::atomic::Ordering::SeqCst,
)
.map_err(|_| AtomicError::<T::Error>::Busy)?;

ARMv6-M does not have atomic CAS, so portable_atomic does not implement AtomicBool::compare_exchange on this target.

@Rahix
Copy link

Rahix commented Apr 27, 2024

I guess the most straight forward solution to fix this regression would be to add a feature-flag for AtomicDevice. We did the same thing back in shared-bus to cater for platforms without the necessary atomics support.

Another solution would be to allow using AtomicDevice on single-core microcontrollers by use of critical sections instead of real atomics. Not sure how best to expose this API-wise, though.

@Rahix
Copy link

Rahix commented Apr 27, 2024

Also, maybe a lesson learned from this regression could be that we need to set up CI infrastructure in embedded-hal to check builds on all relevant targets? There may be more subtle incompatibilities between platforms that could hit in this way, I think.

@dan-corneanu
Copy link

I have stumbled upon this issue right now. Is there any workaround for the time being?

@adamgreig
Copy link
Member

Because AtomicDevice uses the portable-atomic crate, you can enable support for CAS on targets that don't normally have it with either the critical-section or unsafe-assume-single-core features: https://crates.io/crates/portable-atomic#optional-features-critical-section

Having to add an explicit dependency on portable-atomic and enable the feature just to get this crate to build isn't great, but it should work as a workaround now.

@adamgreig
Copy link
Member

We discussed this in today's meeting and the plan is:

  • Add a new atomic-device feature, default off
  • Only enable AtomicDevice when either the atomic-device feature is enabled or cfg(target_has_atomic...) is set, so it's by default available on platforms with CAS
  • Add new portable-atomic-critical-section and portable-atomic-unsafe-assume-single-core features which proxy to the corresponding portable-atomic features (so users don't have to depend on portable-atomic directly) and enable our atomic-device feature
  • Document these new features
  • Release as 0.3

sjoerdsimons added a commit to sjoerdsimons/embedded-hal that referenced this issue May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants