embedded-hal icon indicating copy to clipboard operation
embedded-hal copied to clipboard

Add embedded-hal-bus atomic-device features

Open 9names opened this issue 1 year ago • 1 comments
trafficstars

Add atomic-device and portable-atomic features as described in this comment The tasks from the comment are copied below for convenience

  • 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

Note that the entire util.rs module is only ever used by AtomicDevice and the i2c/spi atomic impls, so we could sensibly rename it to atomic_util.rs and gate that that file instead of everything defined inside it.

Note: untested at this point. Will add comment once testing is done.

Resolves #598

9names avatar Jun 08 '24 12:06 9names

CI errors, because cargo clippy --all-features is definitely not compatible with these new, mutually incompatible features

9names avatar Jun 08 '24 12:06 9names

perhaps we can instead do the following:

  • Add a feature portable-atomic
  • Document that if you enable it, you have to add a dep on portable-atomic and enable one of the two polyfill mode features on it.
  • Enable AtomicDevice if any(feature = "portable-atomic", target_has_atomic = "8")

Advantages

  • Generalizes better if we add more things requiring atomics in the future (the portable-atomic feature would enable all "things" requiring atomics, if we name the feature atomic-device we'd need one per "thing")
  • Generalizes better if portable-atomic adds more polyfill modes in the future.
  • Avoids non-additive features in embedded-hal-bus.

Dirbaio avatar Jul 26 '24 10:07 Dirbaio

Certainly looks cleaner @Dirbaio. I've also added require-cas to the enabled features of portable-atomic as per the docs, so that users that fail to add an appropriate polyfill feature on portable-atomic will get a link error: image

9names avatar Jul 27 '24 11:07 9names