Improve the architecture of SpiFuture

hal/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased Changes
 
+- Improve the architecture of `SpiFuture` (#577)
+
 # v0.15.1
 
 - Fix `sercom::uart` pad definitions to reject pads conflicting with XCK.

hal/src/sercom/spi.rs

@@ -312,7 +312,6 @@ let (chan0, _, spi, _) = dma_transfer.wait();
 "
 )]
 
-use core::convert::TryFrom;
 use core::marker::PhantomData;
 
 use bitflags::bitflags;
@@ -417,15 +416,13 @@ bitflags! {
     }
 }
 
-/// Convert [`Status`] flags into the corresponding [`Error`] variants
-impl TryFrom<Status> for () {
-    type Error = Error;
-    #[inline]
-    fn try_from(status: Status) -> Result<(), Error> {
+impl Status {
+    /// Check the [`Status`] flags for [`Error`] conditions
+    pub fn check_errors(&self) -> Result<(), Error> {
         // Buffer overflow has priority
-        if status.contains(Status::BUFOVF) {
+        if self.contains(Status::BUFOVF) {
             Err(Error::Overflow)
-        } else if status.contains(Status::LENERR) {
+        } else if self.contains(Status::LENERR) {
             Err(Error::LengthError)
         } else {
             Ok(())
@@ -564,13 +561,24 @@ seq!(N in 1..=4 {
 // Capability
 //==============================================================================
 
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub enum DynCapability {
+    Rx,
+    Tx,
+    Duplex,
+}
+
+impl Sealed for DynCapability {}
+
 /// Type-level enum representing the simplex or duplex transaction capability
 ///
 /// The available, type-level variants are [`Rx`], [`Tx`] and [`Duplex`]. See
 /// the [type-level enum] documentation for more details.
 ///
 /// [type-level enum]: crate::typelevel#type-level-enums
-pub trait Capability: Sealed + Default {}
+pub trait Capability: Sealed + Default {
+    const DYN: DynCapability;
+}
 
 /// Sub-set of [`Capability`] variants that can receive data, i.e. [`Rx`] and
 /// [`Duplex`]
@@ -597,7 +605,9 @@ pub struct Rx {
 }
 
 impl Sealed for Rx {}
-impl Capability for Rx {}
+impl Capability for Rx {
+    const DYN: DynCapability = DynCapability::Rx;
+}
 impl Receive for Rx {}
 
 /// Type-level variant of the [`Capability`] enum for simplex, [`Transmit`]-only
@@ -609,7 +619,9 @@ impl Receive for Rx {}
 pub struct Tx;
 
 impl Sealed for Tx {}
-impl Capability for Tx {}
+impl Capability for Tx {
+    const DYN: DynCapability = DynCapability::Tx;
+}
 impl Transmit for Tx {}
 
 /// Type-level variant of the [`Capability`] enum for duplex transactions
@@ -621,7 +633,9 @@ impl Transmit for Tx {}
 pub struct Duplex;
 
 impl Sealed for Duplex {}
-impl Capability for Duplex {}
+impl Capability for Duplex {
+    const DYN: DynCapability = DynCapability::Duplex;
+}
 impl Receive for Duplex {}
 impl Transmit for Duplex {}
 
@@ -779,6 +793,31 @@ where
         self.change()
     }
 
+    /// Return the transaction length, in bytes
+    ///
+    /// This function is valid for all chips and SPI configurations. It returns
+    /// the number of bytes in a single SPI transaction.
+    #[cfg(any(feature = "samd11", feature = "samd21"))]
+    #[inline]
+    pub fn transaction_length(&self) -> u8 {
+        Z::BYTES
+    }
+
+    /// Return the transaction length, in bytes
+    ///
+    /// This function is valid for all chips and SPI configurations. It returns
+    /// the number of bytes in a single SPI transaction.
+    #[cfg(feature = "min-samd51g")]
+    #[inline]
+    pub fn transaction_length(&self) -> u8 {
+        use typenum::Unsigned;
+        if Z::U8 == DynLength::U8 {
+            self.regs.get_length()
+        } else {
+            Z::U8
+        }
+    }
+
     /// Get the clock polarity
     #[inline]
     pub fn get_cpol(&self) -> Polarity {
@@ -1016,10 +1055,10 @@ where
 /// [type class]: crate::typelevel#type-classes
 pub trait AnyConfig: Is<Type = SpecificConfig<Self>> {
     type Sercom: Sercom;
-    type Pads: ValidPads<Sercom = Self::Sercom>;
+    type Pads: ValidPads<Sercom = Self::Sercom, Capability = Self::Capability>;
     type Capability: Capability;
     type OpMode: OpMode;
-    type Size: Size;
+    type Size: Size<Word = Self::Word>;
     type Word: 'static;
 }
 
@@ -1178,6 +1217,15 @@ where
         }
     }
 
+    /// Return the transaction length, in bytes
+    ///
+    /// This function is valid for all chips and SPI configurations. It returns
+    /// the number of bytes in a single SPI transaction.
+    #[inline]
+    pub fn transaction_length(&self) -> u8 {
+        self.config.as_ref().transaction_length()
+    }
+
     /// Update the SPI configuration.
     ///
     /// Calling this method will temporarily disable the SERCOM peripheral, as
@@ -1270,19 +1318,17 @@ where
 }
 
 #[cfg(feature = "min-samd51g")]
-impl<P, M, A> Spi<Config<P, M, DynLength>, A>
+impl<C, A> Spi<C, A>
 where
-    P: ValidPads,
-    M: OpMode,
-    Config<P, M, DynLength>: ValidConfig,
+    C: ValidConfig<Size = DynLength>,
     A: Capability,
 {
     /// Return the current transaction length
     ///
     /// Read the LENGTH register to determine the current transaction length
     #[inline]
     pub fn get_dyn_length(&self) -> u8 {
-        self.config.get_dyn_length()
+        self.config.as_ref().get_dyn_length()
     }
 
     /// Set the transaction length
@@ -1296,7 +1342,7 @@ where
     /// `LENGTH`.
     #[inline]
     pub fn set_dyn_length(&mut self, length: u8) {
-        self.config.set_dyn_length(length);
+        self.config.as_mut().set_dyn_length(length);
     }
 }
 
@@ -1324,7 +1370,14 @@ pub trait AnySpi: Is<Type = SpecificSpi<Self>> {
     type OpMode: OpMode;
     type Size: Size;
     type Word: 'static;
-    type Config: ValidConfig<Sercom = Self::Sercom>;
+    type Config: ValidConfig<
+        Sercom = Self::Sercom,
+        Pads = Self::Pads,
+        Capability = Self::Capability,
+        OpMode = Self::OpMode,
+        Size = Self::Size,
+        Word = Self::Word,
+    >;
 }
 
 /// Type alias to recover the specific [`Spi`] type from an implementation of
@@ -1360,14 +1413,10 @@ where
 {
 }
 
-impl<C, A> AnySpi for Spi<C, A>
-where
-    C: ValidConfig,
-    A: Capability,
-{
+impl<C: ValidConfig> AnySpi for Spi<C, C::Capability> {
     type Sercom = C::Sercom;
     type Pads = C::Pads;
-    type Capability = A;
+    type Capability = C::Capability;
     type OpMode = C::OpMode;
     type Size = C::Size;
     type Word = C::Word;

hal/src/sercom/spi/char_size.rs

@@ -28,6 +28,9 @@ pub trait CharSize: Sealed {
 
     /// Register bit pattern for the corresponding `CharSize`
     const BITS: u8;
+
+    /// Number of bytes in an SPI transaction
+    const BYTES: u8;
 }
 
 /// Type alias to recover the [`Word`](CharSize::Word) type from an
@@ -44,10 +47,12 @@ impl Sealed for EightBit {}
 impl CharSize for EightBit {
     type Word = u8;
     const BITS: u8 = 0;
+    const BYTES: u8 = 1;
 }
 
 impl Sealed for NineBit {}
 impl CharSize for NineBit {
     type Word = u16;
     const BITS: u8 = 1;
+    const BYTES: u8 = 2;
 }

hal/src/sercom/spi/impl_ehal_thumbv7em.rs

@@ -336,7 +336,9 @@ where
 
     #[inline]
     fn transfer<'w>(&mut self, buf: &'w mut [u8]) -> Result<&'w [u8], Error> {
-        assert_eq!(buf.len(), L::USIZE);
+        if buf.len() != L::USIZE {
+            panic!("Slice length does not equal SPI transfer length");
+        }
         let sercom = unsafe { self.config.as_ref().sercom() };
         transfer_slice(sercom, buf)
     }
@@ -352,7 +354,7 @@ where
 /// [`Transfer`]: blocking::spi::Transfer
 impl<P, M, A> blocking::spi::Transfer<u8> for Spi<Config<P, M, DynLength>, A>
 where
-    Config<P, M, DynLength>: ValidConfig,
+    Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
     P: ValidPads,
     M: OpMode,
     A: Receive,
@@ -361,7 +363,9 @@ where
 
     #[inline]
     fn transfer<'w>(&mut self, buf: &'w mut [u8]) -> Result<&'w [u8], Error> {
-        assert_eq!(buf.len(), self.get_dyn_length() as usize);
+        if buf.len() != self.get_dyn_length() as usize {
+            panic!("Slice length does not equal SPI transfer length");
+        }
         let sercom = unsafe { self.config.as_ref().sercom() };
         transfer_slice(sercom, buf)
     }
@@ -524,7 +528,7 @@ where
 /// [`Write`]: blocking::spi::Write
 impl<P, M> blocking::spi::Write<u8> for Spi<Config<P, M, DynLength>, Duplex>
 where
-    Config<P, M, DynLength>: ValidConfig,
+    Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
     P: ValidPads,
     M: OpMode,
 {
@@ -553,7 +557,7 @@ where
 /// [`Write`]: blocking::spi::Write
 impl<P, M> blocking::spi::Write<u8> for Spi<Config<P, M, DynLength>, Tx>
 where
-    Config<P, M, DynLength>: ValidConfig,
+    Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
     P: ValidPads,
     M: OpMode,
 {
@@ -723,8 +727,8 @@ fn transfer_slice<'w>(sercom: &RegisterBlock, buf: &'w mut [u8]) -> Result<&'w [
 /// If `duplex` is false, buffer overflow errors are ignored
 fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), Error> {
     let mut to_send = buf.iter();
-    let mut to_recv: usize = to_send.len();
-    while to_recv > 0 {
+    let mut to_recv: usize = if duplex { to_send.len() } else { 0 };
+    loop {
         let errors = sercom.spim().status.read();
         if duplex && errors.bufovf().bit_is_set() {
             return Err(Error::Overflow);
@@ -733,7 +737,6 @@ fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), E
             return Err(Error::LengthError);
         }
         let flags = sercom.spim().intflag.read();
-        // Send the word
         if to_send.len() > 0 && flags.dre().bit_is_set() {
             let mut bytes = [0; 4];
             for byte in &mut bytes {
@@ -745,11 +748,14 @@ fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), E
             let word = u32::from_le_bytes(bytes);
             sercom.spim().data.write(|w| unsafe { w.data().bits(word) });
         }
-        if duplex && to_recv > to_send.len() && flags.rxc().bit_is_set() {
+        if to_recv > to_send.len() && flags.rxc().bit_is_set() {
             sercom.spim().data.read().data().bits();
             let diff = to_recv - to_send.len();
             to_recv -= if diff < 4 { diff } else { 4 };
         }
+        if to_recv == 0 && to_send.len() == 0 {
+            break;
+        }
     }
     Ok(())
 }

hal/src/sercom/spi/reg.rs

@@ -340,7 +340,7 @@ impl<S: Sercom> Registers<S> {
     /// Try to read the interrupt flags, but first check the error status flags.
     #[inline]
     pub fn read_flags_errors(&self) -> Result<Flags, Error> {
-        self.read_status().try_into()?;
+        self.read_status().check_errors()?;
         Ok(self.read_flags())
     }
 }