Merge pull request #2713 from sgoll/i2c-blocking-transaction

stm32/i2c(v1): Implement blocking transactions

embassy-stm32/src/i2c/mod.rs

@@ -311,10 +311,10 @@ impl<'d, T: Instance> embedded_hal_1::i2c::I2c for I2c<'d, T, NoDma, NoDma> {
 
     fn transaction(
         &mut self,
-        _address: u8,
-        _operations: &mut [embedded_hal_1::i2c::Operation<'_>],
+        address: u8,
+        operations: &mut [embedded_hal_1::i2c::Operation<'_>],
     ) -> Result<(), Self::Error> {
-        todo!();
+        self.blocking_transaction(address, operations)
     }
 }
 

embassy-stm32/src/i2c/v1.rs

@@ -10,6 +10,7 @@ use core::task::Poll;
 use embassy_embedded_hal::SetConfig;
 use embassy_futures::select::{select, Either};
 use embassy_hal_internal::drop::OnDrop;
+use embedded_hal_1::i2c::Operation;
 
 use super::*;
 use crate::dma::Transfer;
@@ -41,6 +42,68 @@ pub unsafe fn on_interrupt<T: Instance>() {
     });
 }
 
+/// Frame type in I2C transaction.
+///
+/// This tells each method what kind of framing to use, to generate a (repeated) start condition (ST
+/// or SR), and/or a stop condition (SP). For read operations, this also controls whether to send an
+/// ACK or NACK after the last byte received.
+///
+/// For write operations, the following options are identical because they differ only in the (N)ACK
+/// treatment relevant for read operations:
+///
+/// - `FirstFrame` and `FirstAndNextFrame`
+/// - `NextFrame` and `LastFrameNoStop`
+///
+/// Abbreviations used below:
+///
+/// - `ST` = start condition
+/// - `SR` = repeated start condition
+/// - `SP` = stop condition
+#[derive(Copy, Clone)]
+enum FrameOptions {
+    /// `[ST/SR]+[NACK]+[SP]` First frame (of this type) in operation and last frame overall in this
+    /// transaction.
+    FirstAndLastFrame,
+    /// `[ST/SR]+[NACK]` First frame of this type in transaction, last frame in a read operation but
+    /// not the last frame overall.
+    FirstFrame,
+    /// `[ST/SR]+[ACK]` First frame of this type in transaction, neither last frame overall nor last
+    /// frame in a read operation.
+    FirstAndNextFrame,
+    /// `[ACK]` Middle frame in a read operation (neither first nor last).
+    NextFrame,
+    /// `[NACK]+[SP]` Last frame overall in this transaction but not the first frame.
+    LastFrame,
+    /// `[NACK]` Last frame in a read operation but not last frame overall in this transaction.
+    LastFrameNoStop,
+}
+
+impl FrameOptions {
+    /// Sends start or repeated start condition before transfer.
+    fn send_start(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::FirstFrame | Self::FirstAndNextFrame => true,
+            Self::NextFrame | Self::LastFrame | Self::LastFrameNoStop => false,
+        }
+    }
+
+    /// Sends stop condition after transfer.
+    fn send_stop(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::LastFrame => true,
+            Self::FirstFrame | Self::FirstAndNextFrame | Self::NextFrame | Self::LastFrameNoStop => false,
+        }
+    }
+
+    /// Sends NACK after last byte received, indicating end of read operation.
+    fn send_nack(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::FirstFrame | Self::LastFrame | Self::LastFrameNoStop => true,
+            Self::FirstAndNextFrame | Self::NextFrame => false,
+        }
+    }
+}
+
 impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
     pub(crate) fn init(&mut self, freq: Hertz, _config: Config) {
         T::regs().cr1().modify(|reg| {
@@ -124,46 +187,57 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         Ok(sr1)
     }
 
-    fn write_bytes(&mut self, addr: u8, bytes: &[u8], timeout: Timeout) -> Result<(), Error> {
-        // Send a START condition
+    fn write_bytes(&mut self, addr: u8, bytes: &[u8], timeout: Timeout, frame: FrameOptions) -> Result<(), Error> {
+        if frame.send_start() {
+            // Send a START condition
 
-        T::regs().cr1().modify(|reg| {
-            reg.set_start(true);
-        });
+            T::regs().cr1().modify(|reg| {
+                reg.set_start(true);
+            });
 
-        // Wait until START condition was generated
-        while !Self::check_and_clear_error_flags()?.start() {
-            timeout.check()?;
+            // Wait until START condition was generated
+            while !Self::check_and_clear_error_flags()?.start() {
+                timeout.check()?;
+            }
+
+            // Also wait until signalled we're master and everything is waiting for us
+            while {
+                Self::check_and_clear_error_flags()?;
+
+                let sr2 = T::regs().sr2().read();
+                !sr2.msl() && !sr2.busy()
+            } {
+                timeout.check()?;
+            }
+
+            // Set up current address, we're trying to talk to
+            T::regs().dr().write(|reg| reg.set_dr(addr << 1));
+
+            // Wait until address was sent
+            // Wait for the address to be acknowledged
+            // Check for any I2C errors. If a NACK occurs, the ADDR bit will never be set.
+            while !Self::check_and_clear_error_flags()?.addr() {
+                timeout.check()?;
+            }
+
+            // Clear condition by reading SR2
+            let _ = T::regs().sr2().read();
         }
 
-        // Also wait until signalled we're master and everything is waiting for us
-        while {
-            Self::check_and_clear_error_flags()?;
-
-            let sr2 = T::regs().sr2().read();
-            !sr2.msl() && !sr2.busy()
-        } {
-            timeout.check()?;
-        }
-
-        // Set up current address, we're trying to talk to
-        T::regs().dr().write(|reg| reg.set_dr(addr << 1));
-
-        // Wait until address was sent
-        // Wait for the address to be acknowledged
-        // Check for any I2C errors. If a NACK occurs, the ADDR bit will never be set.
-        while !Self::check_and_clear_error_flags()?.addr() {
-            timeout.check()?;
-        }
-
-        // Clear condition by reading SR2
-        let _ = T::regs().sr2().read();
-
         // Send bytes
         for c in bytes {
             self.send_byte(*c, timeout)?;
         }
 
+        if frame.send_stop() {
+            // Send a STOP condition
+            T::regs().cr1().modify(|reg| reg.set_stop(true));
+            // Wait for STOP condition to transmit.
+            while T::regs().cr1().read().stop() {
+                timeout.check()?;
+            }
+        }
+
         // Fallthrough is success
         Ok(())
     }
@@ -205,8 +279,18 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         Ok(value)
     }
 
-    fn blocking_read_timeout(&mut self, addr: u8, buffer: &mut [u8], timeout: Timeout) -> Result<(), Error> {
-        if let Some((last, buffer)) = buffer.split_last_mut() {
+    fn blocking_read_timeout(
+        &mut self,
+        addr: u8,
+        buffer: &mut [u8],
+        timeout: Timeout,
+        frame: FrameOptions,
+    ) -> Result<(), Error> {
+        let Some((last, buffer)) = buffer.split_last_mut() else {
+            return Err(Error::Overrun);
+        };
+
+        if frame.send_start() {
             // Send a START condition and set ACK bit
             T::regs().cr1().modify(|reg| {
                 reg.set_start(true);
@@ -237,49 +321,45 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
 
             // Clear condition by reading SR2
             let _ = T::regs().sr2().read();
+        }
 
-            // Receive bytes into buffer
-            for c in buffer {
-                *c = self.recv_byte(timeout)?;
-            }
+        // Receive bytes into buffer
+        for c in buffer {
+            *c = self.recv_byte(timeout)?;
+        }
 
-            // Prepare to send NACK then STOP after next byte
-            T::regs().cr1().modify(|reg| {
+        // Prepare to send NACK then STOP after next byte
+        T::regs().cr1().modify(|reg| {
+            if frame.send_nack() {
                 reg.set_ack(false);
+            }
+            if frame.send_stop() {
                 reg.set_stop(true);
-            });
+            }
+        });
 
-            // Receive last byte
-            *last = self.recv_byte(timeout)?;
+        // Receive last byte
+        *last = self.recv_byte(timeout)?;
 
+        if frame.send_stop() {
             // Wait for the STOP to be sent.
             while T::regs().cr1().read().stop() {
                 timeout.check()?;
             }
-
-            // Fallthrough is success
-            Ok(())
-        } else {
-            Err(Error::Overrun)
         }
+
+        // Fallthrough is success
+        Ok(())
     }
 
     /// Blocking read.
     pub fn blocking_read(&mut self, addr: u8, read: &mut [u8]) -> Result<(), Error> {
-        self.blocking_read_timeout(addr, read, self.timeout())
+        self.blocking_read_timeout(addr, read, self.timeout(), FrameOptions::FirstAndLastFrame)
     }
 
     /// Blocking write.
     pub fn blocking_write(&mut self, addr: u8, write: &[u8]) -> Result<(), Error> {
-        let timeout = self.timeout();
-
-        self.write_bytes(addr, write, timeout)?;
-        // Send a STOP condition
-        T::regs().cr1().modify(|reg| reg.set_stop(true));
-        // Wait for STOP condition to transmit.
-        while T::regs().cr1().read().stop() {
-            timeout.check()?;
-        }
+        self.write_bytes(addr, write, self.timeout(), FrameOptions::FirstAndLastFrame)?;
 
         // Fallthrough is success
         Ok(())
@@ -287,10 +367,85 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
 
     /// Blocking write, restart, read.
     pub fn blocking_write_read(&mut self, addr: u8, write: &[u8], read: &mut [u8]) -> Result<(), Error> {
+        // Check empty read buffer before starting transaction. Otherwise, we would not generate the
+        // stop condition below.
+        if read.is_empty() {
+            return Err(Error::Overrun);
+        }
+
         let timeout = self.timeout();
 
-        self.write_bytes(addr, write, timeout)?;
-        self.blocking_read_timeout(addr, read, timeout)?;
+        self.write_bytes(addr, write, timeout, FrameOptions::FirstFrame)?;
+        self.blocking_read_timeout(addr, read, timeout, FrameOptions::FirstAndLastFrame)?;
+
+        Ok(())
+    }
+
+    /// Blocking transaction with operations.
+    ///
+    /// Consecutive operations of same type are merged. See [transaction contract] for details.
+    ///
+    /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
+    pub fn blocking_transaction(&mut self, addr: u8, operations: &mut [Operation<'_>]) -> Result<(), Error> {
+        // Check empty read buffer before starting transaction. Otherwise, we would not generate the
+        // stop condition below.
+        if operations.iter().any(|op| match op {
+            Operation::Read(read) => read.is_empty(),
+            Operation::Write(_) => false,
+        }) {
+            return Err(Error::Overrun);
+        }
+
+        let timeout = self.timeout();
+
+        let mut operations = operations.iter_mut();
+
+        let mut prev_op: Option<&mut Operation<'_>> = None;
+        let mut next_op = operations.next();
+
+        while let Some(op) = next_op {
+            next_op = operations.next();
+
+            // Check if this is the first frame of this type. This is the case for the first overall
+            // frame in the transaction and whenever the type of operation changes.
+            let first_frame =
+                match (prev_op.as_ref(), &op) {
+                    (None, _) => true,
+                    (Some(Operation::Read(_)), Operation::Write(_))
+                    | (Some(Operation::Write(_)), Operation::Read(_)) => true,
+                    (Some(Operation::Read(_)), Operation::Read(_))
+                    | (Some(Operation::Write(_)), Operation::Write(_)) => false,
+                };
+
+            let frame = match (first_frame, next_op.as_ref()) {
+                // If this is the first frame of this type, we generate a (repeated) start condition
+                // but have to consider the next operation: if it is the last, we generate the final
+                // stop condition. Otherwise, we branch on the operation: with read operations, only
+                // the last byte overall (before a write operation or the end of the transaction) is
+                // to be NACK'd, i.e. if another read operation follows, we must ACK this last byte.
+                (true, None) => FrameOptions::FirstAndLastFrame,
+                // Make sure to keep sending ACK for last byte in read operation when it is followed
+                // by another consecutive read operation. If the current operation is write, this is
+                // identical to `FirstFrame`.
+                (true, Some(Operation::Read(_))) => FrameOptions::FirstAndNextFrame,
+                // Otherwise, send NACK for last byte (in read operation). (For write, this does not
+                // matter and could also be `FirstAndNextFrame`.)
+                (true, Some(Operation::Write(_))) => FrameOptions::FirstFrame,
+
+                // If this is not the first frame of its type, we do not generate a (repeated) start
+                // condition. Otherwise, we branch the same way as above.
+                (false, None) => FrameOptions::LastFrame,
+                (false, Some(Operation::Read(_))) => FrameOptions::NextFrame,
+                (false, Some(Operation::Write(_))) => FrameOptions::LastFrameNoStop,
+            };
+
+            match op {
+                Operation::Read(read) => self.blocking_read_timeout(addr, read, timeout, frame)?,
+                Operation::Write(write) => self.write_bytes(addr, write, timeout, frame)?,
+            }
+
+            prev_op = Some(op);
+        }
 
         Ok(())
     }

embassy-stm32/src/i2c/v2.rs

@@ -4,6 +4,7 @@ use core::task::Poll;
 
 use embassy_embedded_hal::SetConfig;
 use embassy_hal_internal::drop::OnDrop;
+use embedded_hal_1::i2c::Operation;
 
 use super::*;
 use crate::dma::Transfer;
@@ -579,6 +580,17 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         // Automatic Stop
     }
 
+    /// Blocking transaction with operations.
+    ///
+    /// Consecutive operations of same type are merged. See [transaction contract] for details.
+    ///
+    /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
+    pub fn blocking_transaction(&mut self, addr: u8, operations: &mut [Operation<'_>]) -> Result<(), Error> {
+        let _ = addr;
+        let _ = operations;
+        todo!()
+    }
+
     /// Blocking write multiple buffers.
     ///
     /// The buffers are concatenated in a single write transaction.