async/spi: Add a transaction helper macro

embedded-hal-async/CHANGELOG.md

@@ -9,10 +9,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [v0.1.0-alpha.1] - 2022-05-24
 
+### Added
+
+- spi: added a transaction helper macro as a workaround for the raw pointer workaround.
+
 ### Changed
 
 - spi: device helper methods (`read`, `write`, `transfer`...) are now default methods in `SpiDevice` instead of an `SpiDeviceExt` extension trait.
 - spi: the `SpiDevice::transaction` closure now gets a raw pointer to the `SpiBus` to work around Rust borrow checker limitations.
+- spi: the `SpiDevice` trait is now unsafe to implement due to the raw pointer workaround.
 
 
 ## [v0.1.0-alpha.0] - 2022-04-17

embedded-hal-async/src/lib.rs

@@ -9,7 +9,6 @@
 
 #![warn(missing_docs)]
 #![no_std]
-#![feature(generic_associated_types)]
 #![feature(type_alias_impl_trait)]
 
 pub mod delay;

embedded-hal-async/src/spi.rs

@@ -35,13 +35,118 @@ where
     Word: Copy + 'static,
 = impl Future<Output = Result<(), T::Error>>;
 
+#[macro_export]
+/// Do an SPI transaction on a bus.
+/// This is a safe wrapper for [SpiDevice::transaction], which handles dereferencing the raw pointer for you.
+///
+/// # Examples
+///
+/// ```
+/// use embedded_hal_async::spi::{transaction, SpiBus, SpiBusRead, SpiBusWrite, SpiDevice};
+///
+/// pub async fn transaction_example<SPI>(mut device: SPI) -> Result<u32, SPI::Error>
+/// where
+///     SPI: SpiDevice,
+///     SPI::Bus: SpiBus,
+/// {
+///     transaction!(&mut device, move |bus| async move {
+///         // Unlike `SpiDevice::transaction`, we don't need to
+///         // manually dereference a pointer in order to use the bus.
+///         bus.write(&[42]).await?;
+///         let mut data = [0; 4];
+///         bus.read(&mut data).await?;
+///         Ok(u32::from_be_bytes(data))
+///     })
+///     .await
+/// }
+/// ```
+///
+/// Note that the compiler will prevent you from moving the bus reference outside of the closure
+/// ```compile_fail
+/// # use embedded_hal_async::spi::{transaction, SpiBus, SpiBusRead, SpiBusWrite, SpiDevice};
+/// #
+/// # pub async fn smuggle_test<SPI>(mut device: SPI) -> Result<(), SPI::Error>
+/// # where
+/// #     SPI: SpiDevice,
+/// #     SPI::Bus: SpiBus,
+/// # {
+///     let mut bus_smuggler: Option<&mut SPI::Bus> = None;
+///     transaction!(&mut device, move |bus| async move {
+///         bus_smuggler = Some(bus);
+///         Ok(())
+///     })
+///     .await
+/// # }
+/// ```
+macro_rules! spi_transaction {
+    ($device:expr, move |$bus:ident| async move $closure_body:expr) => {
+        $crate::spi::SpiDevice::transaction($device, move |$bus| {
+            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
+            // valid and dereferencable for the entire duration of the closure.
+            let $bus = unsafe { &mut *$bus };
+            async move {
+                let result = $closure_body;
+                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
+                let _ = $bus; // Silence the "unused variable" warning from prevous line
+                result
+            }
+        })
+    };
+    ($device:expr, move |$bus:ident| async $closure_body:expr) => {
+        $crate::spi::SpiDevice::transaction($device, move |$bus| {
+            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
+            // valid and dereferencable for the entire duration of the closure.
+            let $bus = unsafe { &mut *$bus };
+            async {
+                let result = $closure_body;
+                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
+                let _ = $bus; // Silence the "unused variable" warning from prevous line
+                result
+            }
+        })
+    };
+    ($device:expr, |$bus:ident| async move $closure_body:expr) => {
+        $crate::spi::SpiDevice::transaction($device, |$bus| {
+            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
+            // valid and dereferencable for the entire duration of the closure.
+            let $bus = unsafe { &mut *$bus };
+            async move {
+                let result = $closure_body;
+                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
+                let _ = $bus; // Silence the "unused variable" warning from prevous line
+                result
+            }
+        })
+    };
+    ($device:expr, |$bus:ident| async $closure_body:expr) => {
+        $crate::spi::SpiDevice::transaction($device, |$bus| {
+            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
+            // valid and dereferencable for the entire duration of the closure.
+            let $bus = unsafe { &mut *$bus };
+            async {
+                let result = $closure_body;
+                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
+                let _ = $bus; // Silence the "unused variable" warning from prevous line
+                result
+            }
+        })
+    };
+}
+
+#[doc(inline)]
+pub use spi_transaction as transaction;
+
 /// SPI device trait
 ///
 /// `SpiDevice` represents ownership over a single SPI device on a (possibly shared) bus, selected
 /// with a CS (Chip Select) pin.
 ///
 /// See (the docs on embedded-hal)[embedded_hal::spi::blocking] for important information on SPI Bus vs Device traits.
-pub trait SpiDevice: ErrorType {
+///
+/// # Safety
+///
+/// See [`SpiDevice::transaction`] for details.
+pub unsafe trait SpiDevice: ErrorType {
     /// SPI Bus type for this device.
     type Bus: ErrorType;
 
@@ -55,6 +160,10 @@ pub trait SpiDevice: ErrorType {
 
     /// Perform a transaction against the device.
     ///
+    /// **NOTE:**
+    /// It is not recommended to use this method directly, because it requires `unsafe` code to dereference the raw pointer.
+    /// Instead, the [`transaction!`] macro should be used, which handles this safely inside the macro.
+    ///
     /// - Locks the bus
     /// - Asserts the CS (Chip Select) pin.
     /// - Calls `f` with an exclusive reference to the bus, which can then be used to do transfers against the device.
@@ -69,9 +178,13 @@ pub trait SpiDevice: ErrorType {
     /// On bus errors the implementation should try to deassert CS.
     /// If an error occurs while deasserting CS the bus error should take priority as the return value.
     ///
+    /// # Safety
+    ///
     /// The current state of the Rust typechecker doesn't allow expressing the necessary lifetime constraints, so
-    /// the `f` closure receives a lifetime-less `*mut Bus` raw pointer instead. The pointer is guaranteed
-    /// to be valid for the entire duration the closure is running, so dereferencing it is safe.
+    /// the `f` closure receives a lifetime-less `*mut Bus` raw pointer instead.
+    ///
+    /// Implementers of the `SpiDevice` trait must guarantee that the pointer is valid and dereferencable
+    /// for the entire duration of the closure.
     fn transaction<'a, R, F, Fut>(&'a mut self, f: F) -> Self::TransactionFuture<'a, R, F, Fut>
     where
         F: FnOnce(*mut Self::Bus) -> Fut + 'a,
@@ -87,11 +200,7 @@ pub trait SpiDevice: ErrorType {
         Self::Bus: SpiBusRead<Word>,
         Word: Copy + 'static,
     {
-        self.transaction(move |bus| async move {
-            // safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
-            let bus = unsafe { &mut *bus };
-            bus.read(buf).await
-        })
+        transaction!(self, move |bus| async move { bus.read(buf).await })
     }
 
     /// Do a write within a transaction.
@@ -104,11 +213,7 @@ pub trait SpiDevice: ErrorType {
         Self::Bus: SpiBusWrite<Word>,
         Word: Copy + 'static,
     {
-        self.transaction(move |bus| async move {
-            // safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
-            let bus = unsafe { &mut *bus };
-            bus.write(buf).await
-        })
+        transaction!(self, move |bus| async move { bus.write(buf).await })
     }
 
     /// Do a transfer within a transaction.
@@ -125,11 +230,10 @@ pub trait SpiDevice: ErrorType {
         Self::Bus: SpiBus<Word>,
         Word: Copy + 'static,
     {
-        self.transaction(move |bus| async move {
-            // safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
-            let bus = unsafe { &mut *bus };
-            bus.transfer(read, write).await
-        })
+        transaction!(
+            self,
+            move |bus| async move { bus.transfer(read, write).await }
+        )
     }
 
     /// Do an in-place transfer within a transaction.
@@ -145,15 +249,14 @@ pub trait SpiDevice: ErrorType {
         Self::Bus: SpiBus<Word>,
         Word: Copy + 'static,
     {
-        self.transaction(move |bus| async move {
-            // safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
-            let bus = unsafe { &mut *bus };
-            bus.transfer_in_place(buf).await
-        })
+        transaction!(
+            self,
+            move |bus| async move { bus.transfer_in_place(buf).await }
+        )
     }
 }
 
-impl<T: SpiDevice> SpiDevice for &mut T {
+unsafe impl<T: SpiDevice> SpiDevice for &mut T {
     type Bus = T::Bus;
 
     type TransactionFuture<'a, R, F, Fut> = T::TransactionFuture<'a, R, F, Fut>
@@ -377,7 +480,7 @@ where
     }
 }
 
-impl<BUS, CS> SpiDevice for ExclusiveDevice<BUS, CS>
+unsafe impl<BUS, CS> SpiDevice for ExclusiveDevice<BUS, CS>
 where
     BUS: SpiBusFlush,
     CS: OutputPin,