Consolidate identical SPI transaction impl

Consolidate three identical implementations of the `SpiDevice::transaction`. Fewer lines of code, faster compilation, fewer bugs...
rust-embedded · Nov 22, 2023 · cb22321 · cb22321
1 parent 6df95fd
commit cb22321
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 70 deletions.
diff --git a/embedded-hal-bus/src/spi/critical_section.rs b/embedded-hal-bus/src/spi/critical_section.rs
@@ -5,6 +5,7 @@ use embedded_hal::digital::OutputPin;
 use embedded_hal::spi::{ErrorType, Operation, SpiBus, SpiDevice};
 
 use super::DeviceError;
+use crate::spi::shared::transaction;
 
 /// `critical-section`-based shared bus [`SpiDevice`] implementation.
 ///
@@ -66,29 +67,7 @@ where
         critical_section::with(|cs| {
             let bus = &mut *self.bus.borrow_ref_mut(cs);
 
-            self.cs.set_low().map_err(DeviceError::Cs)?;
-
-            let op_res = operations.iter_mut().try_for_each(|op| match op {
-                Operation::Read(buf) => bus.read(buf),
-                Operation::Write(buf) => bus.write(buf),
-                Operation::Transfer(read, write) => bus.transfer(read, write),
-                Operation::TransferInPlace(buf) => bus.transfer_in_place(buf),
-                Operation::DelayUs(us) => {
-                    bus.flush()?;
-                    self.delay.delay_us(*us);
-                    Ok(())
-                }
-            });
-
-            // On failure, it's important to still flush and deassert CS.
-            let flush_res = bus.flush();
-            let cs_res = self.cs.set_high();
-
-            op_res.map_err(DeviceError::Spi)?;
-            flush_res.map_err(DeviceError::Spi)?;
-            cs_res.map_err(DeviceError::Cs)?;
-
-            Ok(())
+            transaction(operations, bus, &mut self.delay, &mut self.cs)
         })
     }
 }
diff --git a/embedded-hal-bus/src/spi/mod.rs b/embedded-hal-bus/src/spi/mod.rs
@@ -12,6 +12,8 @@ mod mutex;
 #[cfg(feature = "std")]
 pub use mutex::*;
 mod critical_section;
+mod shared;
+
 pub use self::critical_section::*;
 
 #[cfg(feature = "defmt-03")]

diff --git a/embedded-hal-bus/src/spi/mutex.rs b/embedded-hal-bus/src/spi/mutex.rs
@@ -4,6 +4,7 @@ use embedded_hal::spi::{ErrorType, Operation, SpiBus, SpiDevice};
 use std::sync::Mutex;
 
 use super::DeviceError;
+use crate::spi::shared::transaction;
 
 /// `std` `Mutex`-based shared bus [`SpiDevice`] implementation.
 ///
@@ -63,28 +64,6 @@ where
     fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
         let bus = &mut *self.bus.lock().unwrap();
 
-        self.cs.set_low().map_err(DeviceError::Cs)?;
-
-        let op_res = operations.iter_mut().try_for_each(|op| match op {
-            Operation::Read(buf) => bus.read(buf),
-            Operation::Write(buf) => bus.write(buf),
-            Operation::Transfer(read, write) => bus.transfer(read, write),
-            Operation::TransferInPlace(buf) => bus.transfer_in_place(buf),
-            Operation::DelayUs(us) => {
-                bus.flush()?;
-                self.delay.delay_us(*us);
-                Ok(())
-            }
-        });
-
-        // On failure, it's important to still flush and deassert CS.
-        let flush_res = bus.flush();
-        let cs_res = self.cs.set_high();
-
-        op_res.map_err(DeviceError::Spi)?;
-        flush_res.map_err(DeviceError::Spi)?;
-        cs_res.map_err(DeviceError::Cs)?;
-
-        Ok(())
+        transaction(operations, bus, &mut self.delay, &mut self.cs)
     }
 }
diff --git a/embedded-hal-bus/src/spi/refcell.rs b/embedded-hal-bus/src/spi/refcell.rs
@@ -4,6 +4,7 @@ use embedded_hal::digital::OutputPin;
 use embedded_hal::spi::{ErrorType, Operation, SpiBus, SpiDevice};
 
 use super::DeviceError;
+use crate::spi::shared::transaction;
 
 /// `RefCell`-based shared bus [`SpiDevice`] implementation.
 ///
@@ -62,28 +63,6 @@ where
     fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
         let bus = &mut *self.bus.borrow_mut();
 
-        self.cs.set_low().map_err(DeviceError::Cs)?;
-
-        let op_res = operations.iter_mut().try_for_each(|op| match op {
-            Operation::Read(buf) => bus.read(buf),
-            Operation::Write(buf) => bus.write(buf),
-            Operation::Transfer(read, write) => bus.transfer(read, write),
-            Operation::TransferInPlace(buf) => bus.transfer_in_place(buf),
-            Operation::DelayUs(us) => {
-                bus.flush()?;
-                self.delay.delay_us(*us);
-                Ok(())
-            }
-        });
-
-        // On failure, it's important to still flush and deassert CS.
-        let flush_res = bus.flush();
-        let cs_res = self.cs.set_high();
-
-        op_res.map_err(DeviceError::Spi)?;
-        flush_res.map_err(DeviceError::Spi)?;
-        cs_res.map_err(DeviceError::Cs)?;
-
-        Ok(())
+        transaction(operations, bus, &mut self.delay, &mut self.cs)
     }
 }
diff --git a/embedded-hal-bus/src/spi/shared.rs b/embedded-hal-bus/src/spi/shared.rs
@@ -0,0 +1,43 @@
+use crate::spi::DeviceError;
+use embedded_hal::delay::DelayUs;
+use embedded_hal::digital::OutputPin;
+use embedded_hal::spi::{ErrorType, Operation, SpiBus};
+
+/// Common implementation to perform a transaction against the device.
+#[inline]
+pub fn transaction<Word, BUS, CS, D>(
+    operations: &mut [Operation<Word>],
+    bus: &mut BUS,
+    delay: &mut D,
+    cs: &mut CS,
+) -> Result<(), DeviceError<BUS::Error, CS::Error>>
+where
+    BUS: SpiBus<Word> + ErrorType,
+    CS: OutputPin,
+    D: DelayUs,
+    Word: Copy,
+{
+    cs.set_low().map_err(DeviceError::Cs)?;
+
+    let op_res = operations.iter_mut().try_for_each(|op| match op {
+        Operation::Read(buf) => bus.read(buf),
+        Operation::Write(buf) => bus.write(buf),
+        Operation::Transfer(read, write) => bus.transfer(read, write),
+        Operation::TransferInPlace(buf) => bus.transfer_in_place(buf),
+        Operation::DelayUs(us) => {
+            bus.flush()?;
+            delay.delay_us(*us);
+            Ok(())
+        }
+    });
+
+    // On failure, it's important to still flush and deassert CS.
+    let flush_res = bus.flush();
+    let cs_res = cs.set_high();
+
+    op_res.map_err(DeviceError::Spi)?;
+    flush_res.map_err(DeviceError::Spi)?;
+    cs_res.map_err(DeviceError::Cs)?;
+
+    Ok(())
+}
diff --git a/embedded-io-adapters/src/std.rs b/embedded-io-adapters/src/std.rs
@@ -1,6 +1,6 @@
 //! Adapters to/from `std::io` traits.
 
-use embedded_io::Error;
+use embedded_io::Error as _;
 
 /// Adapter from `std::io` traits.
 #[derive(Clone)]