e-h-bus: Adding AtomicDevice for I2C and SPI bus sharing in multiple interrupt contexts

embedded-hal-bus/CHANGELOG.md

@@ -7,7 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
-No unreleased changes
+### Added
+- Added a new `AtomicDevice` for I2C and SPI to enable bus sharing across multiple contexts.
+
+
+### Fixed
+- Fixed an issue with SPI `ExclusiveDevice` builds when the `async` feature was enabled.
 
 ## [v0.1.0] - 2023-12-28
 

embedded-hal-bus/Cargo.toml

@@ -20,10 +20,11 @@ async = ["dep:embedded-hal-async"]
 defmt-03 = ["dep:defmt-03", "embedded-hal/defmt-03", "embedded-hal-async?/defmt-03"]
 
 [dependencies]
-embedded-hal = { version = "1.0.0", path = "../embedded-hal" }
+embedded-hal = { version = "1.0.0" }
 embedded-hal-async = { version = "1.0.0", path = "../embedded-hal-async", optional = true }
 critical-section = { version = "1.0" }
 defmt-03 = { package = "defmt", version = "0.3", optional = true }
+portable-atomic = {version = "1", default-features = false}
 
 [package.metadata.docs.rs]
 features = ["std", "async"]

embedded-hal-bus/src/i2c/atomic.rs

@@ -0,0 +1,170 @@
+use core::cell::UnsafeCell;
+use embedded_hal::i2c::{Error, ErrorKind, ErrorType, I2c};
+
+/// `UnsafeCell`-based shared bus [`I2c`] implementation.
+///
+/// Sharing is implemented with a `UnsafeCell`. This means it has low overhead, similar to [`crate::i2c::RefCellDevice`] instances, but they are `Send`.
+/// so it only allows sharing across multiple threads (interrupt priority levels). When attempting
+/// to preempt usage of the bus, a `AtomicError::Busy` error is returned.
+///
+/// This primitive is particularly well-suited for applications that have external arbitration
+/// rules, such as the RTIC framework.
+///
+/// # Examples
+///
+/// Assuming there is a pressure sensor with address `0x42` on the same bus as a temperature sensor
+/// with address `0x20`; [`AtomicDevice`] can be used to give access to both of these sensors
+/// from a single `i2c` instance.
+///
+/// ```
+/// use embedded_hal_bus::i2c;
+/// use core::cell::UnsafeCell;
+/// # use embedded_hal::i2c::{self as hali2c, SevenBitAddress, TenBitAddress, I2c, Operation, ErrorKind};
+/// # pub struct Sensor<I2C> {
+/// #     i2c: I2C,
+/// #     address: u8,
+/// # }
+/// # impl<I2C: I2c> Sensor<I2C> {
+/// #     pub fn new(i2c: I2C, address: u8) -> Self {
+/// #         Self { i2c, address }
+/// #     }
+/// # }
+/// # type PressureSensor<I2C> = Sensor<I2C>;
+/// # type TemperatureSensor<I2C> = Sensor<I2C>;
+/// # pub struct I2c0;
+/// # #[derive(Debug, Copy, Clone, Eq, PartialEq)]
+/// # pub enum Error { }
+/// # impl hali2c::Error for Error {
+/// #     fn kind(&self) -> hali2c::ErrorKind {
+/// #         ErrorKind::Other
+/// #     }
+/// # }
+/// # impl hali2c::ErrorType for I2c0 {
+/// #     type Error = Error;
+/// # }
+/// # impl I2c<SevenBitAddress> for I2c0 {
+/// #     fn transaction(&mut self, address: u8, operations: &mut [Operation<'_>]) -> Result<(), Self::Error> {
+/// #       Ok(())
+/// #     }
+/// # }
+/// # struct Hal;
+/// # impl Hal {
+/// #   fn i2c(&self) -> I2c0 {
+/// #     I2c0
+/// #   }
+/// # }
+/// # let hal = Hal;
+///
+/// let i2c = hal.i2c();
+/// let i2c_unsafe_cell = UnsafeCell::new(i2c);
+/// let mut temperature_sensor = TemperatureSensor::new(
+///   i2c::AtomicDevice::new(&i2c_unsafe_cell),
+///   0x20,
+/// );
+/// let mut pressure_sensor = PressureSensor::new(
+///   i2c::AtomicDevice::new(&i2c_unsafe_cell),
+///   0x42,
+/// );
+/// ```
+pub struct AtomicDevice<'a, T> {
+    bus: &'a UnsafeCell<T>,
+    busy: portable_atomic::AtomicBool,
+}
+
+#[derive(Debug, Copy, Clone)]
+/// Wrapper type for errors originating from the atomically-checked I2C bus manager.
+pub enum AtomicError<T: Error> {
+    /// This error is returned if the I2C bus was already in use when an operation was attempted,
+    /// which indicates that the driver requirements are not being met with regard to
+    /// synchronization.
+    Busy,
+
+    /// An I2C-related error occurred, and the internal error should be inspected.
+    Other(T),
+}
+
+impl<T: Error> Error for AtomicError<T> {
+    fn kind(&self) -> ErrorKind {
+        match self {
+            AtomicError::Other(e) => e.kind(),
+            _ => ErrorKind::Other,
+        }
+    }
+}
+
+unsafe impl<'a, T> Send for AtomicDevice<'a, T> {}
+
+impl<'a, T> AtomicDevice<'a, T>
+where
+    T: I2c,
+{
+    /// Create a new `AtomicDevice`.
+    #[inline]
+    pub fn new(bus: &'a UnsafeCell<T>) -> Self {
+        Self {
+            bus,
+            busy: portable_atomic::AtomicBool::from(false),
+        }
+    }
+
+    fn lock<R, F>(&self, f: F) -> Result<R, AtomicError<T::Error>>
+    where
+        F: FnOnce(&mut T) -> Result<R, <T as ErrorType>::Error>,
+    {
+        self.busy
+            .compare_exchange(
+                false,
+                true,
+                core::sync::atomic::Ordering::SeqCst,
+                core::sync::atomic::Ordering::SeqCst,
+            )
+            .map_err(|_| AtomicError::<T::Error>::Busy)?;
+
+        let result = f(unsafe { &mut *self.bus.get() });
+
+        self.busy.store(false, core::sync::atomic::Ordering::SeqCst);
+
+        result.map_err(AtomicError::Other)
+    }
+}
+
+impl<'a, T> ErrorType for AtomicDevice<'a, T>
+where
+    T: I2c,
+{
+    type Error = AtomicError<T::Error>;
+}
+
+impl<'a, T> I2c for AtomicDevice<'a, T>
+where
+    T: I2c,
+{
+    #[inline]
+    fn read(&mut self, address: u8, read: &mut [u8]) -> Result<(), Self::Error> {
+        self.lock(|bus| bus.read(address, read))
+    }
+
+    #[inline]
+    fn write(&mut self, address: u8, write: &[u8]) -> Result<(), Self::Error> {
+        self.lock(|bus| bus.write(address, write))
+    }
+
+    #[inline]
+    fn write_read(
+        &mut self,
+        address: u8,
+        write: &[u8],
+        read: &mut [u8],
+    ) -> Result<(), Self::Error> {
+        self.lock(|bus| bus.write_read(address, write, read))
+    }
+
+    #[inline]
+    fn transaction(
+        &mut self,
+        address: u8,
+        operations: &mut [embedded_hal::i2c::Operation<'_>],
+    ) -> Result<(), Self::Error> {
+        self.lock(|bus| bus.transaction(address, operations))
+    }
+}

embedded-hal-bus/src/i2c/mod.rs

@@ -8,3 +8,5 @@ mod mutex;
 pub use mutex::*;
 mod critical_section;
 pub use self::critical_section::*;
+mod atomic;
+pub use atomic::*;

embedded-hal-bus/src/spi/atomic.rs

@@ -0,0 +1,129 @@
+use core::cell::UnsafeCell;
+use embedded_hal::delay::DelayNs;
+use embedded_hal::digital::OutputPin;
+use embedded_hal::spi::{Error, ErrorKind, ErrorType, Operation, SpiBus, SpiDevice};
+
+use super::DeviceError;
+use crate::spi::shared::transaction;
+
+/// `UnsafeCell`-based shared bus [`SpiDevice`] implementation.
+///
+/// This allows for sharing an [`SpiBus`], obtaining multiple [`SpiDevice`] instances,
+/// each with its own `CS` pin.
+///
+/// Sharing is implemented with a `UnsafeCell`. This means it has low overhead, and, unlike [`crate::spi::RefCellDevice`], instances are `Send`,
+/// so it only allows sharing across multiple threads (interrupt priority level). When attempting
+/// to preempt usage of the bus, a `AtomicError::Busy` error is returned.
+///
+/// This primitive is particularly well-suited for applications that have external arbitration
+/// rules, such as the RTIC framework.
+///
+pub struct AtomicDevice<'a, BUS, CS, D> {
+    bus: &'a UnsafeCell<BUS>,
+    cs: CS,
+    delay: D,
+    busy: portable_atomic::AtomicBool,
+}
+
+#[derive(Debug, Copy, Clone)]
+/// Wrapper type for errors originating from the atomically-checked SPI bus manager.
+pub enum AtomicError<T: Error> {
+    /// This error is returned if the SPI bus was already in use when an operation was attempted,
+    /// which indicates that the driver requirements are not being met with regard to
+    /// synchronization.
+    Busy,
+
+    /// An SPI-related error occurred, and the internal error should be inspected.
+    Other(T),
+}
+
+impl<'a, BUS, CS, D> AtomicDevice<'a, BUS, CS, D> {
+    /// Create a new [`AtomicDevice`].
+    #[inline]
+    pub fn new(bus: &'a UnsafeCell<BUS>, cs: CS, delay: D) -> Self {
+        Self {
+            bus,
+            cs,
+            delay,
+            busy: portable_atomic::AtomicBool::from(false),
+        }
+    }
+}
+
+impl<'a, BUS, CS> AtomicDevice<'a, BUS, CS, super::NoDelay>
+where
+    BUS: ErrorType,
+    CS: OutputPin,
+{
+    /// Create a new [`AtomicDevice`] without support for in-transaction delays.
+    ///
+    /// **Warning**: The returned instance *technically* doesn't comply with the `SpiDevice`
+    /// contract, which mandates delay support. It is relatively rare for drivers to use
+    /// in-transaction delays, so you might still want to use this method because it's more practical.
+    ///
+    /// Note that a future version of the driver might start using delays, causing your
+    /// code to panic. This wouldn't be considered a breaking change from the driver side, because
+    /// drivers are allowed to assume `SpiDevice` implementations comply with the contract.
+    /// If you feel this risk outweighs the convenience of having `cargo` automatically upgrade
+    /// the driver crate, you might want to pin the driver's version.
+    ///
+    /// # Panics
+    ///
+    /// The returned device will panic if you try to execute a transaction
+    /// that contains any operations of type [`Operation::DelayNs`].
+    #[inline]
+    pub fn new_no_delay(bus: &'a UnsafeCell<BUS>, cs: CS) -> Self {
+        Self {
+            bus,
+            cs,
+            delay: super::NoDelay,
+            busy: portable_atomic::AtomicBool::from(false),
+        }
+    }
+}
+
+unsafe impl<'a, BUS, CS, D> Send for AtomicDevice<'a, BUS, CS, D> {}
+
+impl<T: Error> Error for AtomicError<T> {
+    fn kind(&self) -> ErrorKind {
+        match self {
+            AtomicError::Other(e) => e.kind(),
+            _ => ErrorKind::Other,
+        }
+    }
+}
+
+impl<'a, BUS, CS, D> ErrorType for AtomicDevice<'a, BUS, CS, D>
+where
+    BUS: ErrorType,
+    CS: OutputPin,
+{
+    type Error = AtomicError<DeviceError<BUS::Error, CS::Error>>;
+}
+
+impl<'a, Word: Copy + 'static, BUS, CS, D> SpiDevice<Word> for AtomicDevice<'a, BUS, CS, D>
+where
+    BUS: SpiBus<Word>,
+    CS: OutputPin,
+    D: DelayNs,
+{
+    #[inline]
+    fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
+        self.busy
+            .compare_exchange(
+                false,
+                true,
+                core::sync::atomic::Ordering::SeqCst,
+                core::sync::atomic::Ordering::SeqCst,
+            )
+            .map_err(|_| AtomicError::Busy)?;
+
+        let bus = unsafe { &mut *self.bus.get() };
+
+        let result = transaction(operations, bus, &mut self.delay, &mut self.cs);
+
+        self.busy.store(false, core::sync::atomic::Ordering::SeqCst);
+
+        result.map_err(AtomicError::Other)
+    }
+}

embedded-hal-bus/src/spi/exclusive.rs

@@ -6,7 +6,10 @@ use embedded_hal::spi::{ErrorType, Operation, SpiBus, SpiDevice};
 #[cfg(feature = "async")]
 use embedded_hal_async::{
     delay::DelayNs as AsyncDelayNs,
-    spi::{SpiBus as AsyncSpiBus, SpiDevice as AsyncSpiDevice},
+    spi::{
+        ErrorType as AsyncErrorType, Operation as AsyncOperation, SpiBus as AsyncSpiBus,
+        SpiDevice as AsyncSpiDevice,
+    },
 };
 
 use super::shared::transaction;
@@ -89,6 +92,15 @@ where
     }
 }
 
+#[cfg(feature = "async")]
+impl<BUS, CS, D> AsyncErrorType for ExclusiveDevice<BUS, CS, D>
+where
+    BUS: AsyncErrorType,
+    CS: OutputPin,
+{
+    type Error = DeviceError<BUS::Error, CS::Error>;
+}
+
 #[cfg(feature = "async")]
 #[cfg_attr(docsrs, doc(cfg(feature = "async")))]
 impl<Word: Copy + 'static, BUS, CS, D> AsyncSpiDevice<Word> for ExclusiveDevice<BUS, CS, D>
@@ -100,18 +112,18 @@ where
     #[inline]
     async fn transaction(
         &mut self,
-        operations: &mut [Operation<'_, Word>],
+        operations: &mut [AsyncOperation<'_, Word>],
     ) -> Result<(), Self::Error> {
         self.cs.set_low().map_err(DeviceError::Cs)?;
 
         let op_res = 'ops: {
             for op in operations {
                 let res = match op {
-                    Operation::Read(buf) => self.bus.read(buf).await,
-                    Operation::Write(buf) => self.bus.write(buf).await,
-                    Operation::Transfer(read, write) => self.bus.transfer(read, write).await,
-                    Operation::TransferInPlace(buf) => self.bus.transfer_in_place(buf).await,
-                    Operation::DelayNs(ns) => match self.bus.flush().await {
+                    AsyncOperation::Read(buf) => self.bus.read(buf).await,
+                    AsyncOperation::Write(buf) => self.bus.write(buf).await,
+                    AsyncOperation::Transfer(read, write) => self.bus.transfer(read, write).await,
+                    AsyncOperation::TransferInPlace(buf) => self.bus.transfer_in_place(buf).await,
+                    AsyncOperation::DelayNs(ns) => match self.bus.flush().await {
                         Err(e) => Err(e),
                         Ok(()) => {
                             self.delay.delay_ns(*ns).await;