diff --git a/embedded-hal-async/CHANGELOG.md b/embedded-hal-async/CHANGELOG.md index 1622d0e1d..469b7f1c5 100644 --- a/embedded-hal-async/CHANGELOG.md +++ b/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 diff --git a/embedded-hal-async/src/lib.rs b/embedded-hal-async/src/lib.rs index 1d7e0177c..07b3b966a 100644 --- a/embedded-hal-async/src/lib.rs +++ b/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; diff --git a/embedded-hal-async/src/spi.rs b/embedded-hal-async/src/spi.rs index 562d0ee40..da4f9cc7c 100644 --- a/embedded-hal-async/src/spi.rs +++ b/embedded-hal-async/src/spi.rs @@ -35,13 +35,118 @@ where Word: Copy + 'static, = impl Future>; +#[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(mut device: SPI) -> Result +/// 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(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: 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: 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: 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: 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 SpiDevice for &mut T { +unsafe impl 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 SpiDevice for ExclusiveDevice +unsafe impl SpiDevice for ExclusiveDevice where BUS: SpiBusFlush, CS: OutputPin,