Merge pull request #748 from embassy-rs/nrf-i2c-timeout

Misc HAL improvements.

embassy-nrf/Cargo.toml

@@ -17,6 +17,8 @@ flavors = [
 
 [features]
 
+time = ["embassy/time"]
+
 defmt = ["dep:defmt", "embassy/defmt", "embassy-usb?/defmt"]
 
 # Enable nightly-only features
@@ -56,7 +58,7 @@ _nrf5340-net = ["_nrf5340", "nrf5340-net-pac"]
 _nrf5340 = ["_gpio-p1", "_dppi"]
 _nrf9160 = ["nrf9160-pac", "_dppi"]
 
-_time-driver = ["embassy/time-tick-32768hz"]
+_time-driver = ["embassy/time-tick-32768hz", "time"]
 
 _ppi = []
 _dppi = []

embassy-nrf/src/gpio.rs

@@ -7,10 +7,10 @@ use core::marker::PhantomData;
 use cfg_if::cfg_if;
 use embassy::util::Unborrow;
 use embassy_hal_common::{unborrow, unsafe_impl_unborrow};
-use gpio::pin_cnf::DRIVE_A;
 
 use crate::pac;
 use crate::pac::p0 as gpio;
+use crate::pac::p0::pin_cnf::{DRIVE_A, PULL_A};
 
 use self::sealed::Pin as _;
 
@@ -129,9 +129,30 @@ impl<'d, T: Pin> Output<'d, T> {
     }
 }
 
+fn convert_drive(drive: OutputDrive) -> DRIVE_A {
+    match drive {
+        OutputDrive::Standard => DRIVE_A::S0S1,
+        OutputDrive::HighDrive0Standard1 => DRIVE_A::H0S1,
+        OutputDrive::Standard0HighDrive1 => DRIVE_A::S0H1,
+        OutputDrive::HighDrive => DRIVE_A::H0H1,
+        OutputDrive::Disconnect0Standard1 => DRIVE_A::D0S1,
+        OutputDrive::Disconnect0HighDrive1 => DRIVE_A::D0H1,
+        OutputDrive::Standard0Disconnect1 => DRIVE_A::S0D1,
+        OutputDrive::HighDrive0Disconnect1 => DRIVE_A::H0D1,
+    }
+}
+
+fn convert_pull(pull: Pull) -> PULL_A {
+    match pull {
+        Pull::None => PULL_A::DISABLED,
+        Pull::Up => PULL_A::PULLUP,
+        Pull::Down => PULL_A::PULLDOWN,
+    }
+}
+
 /// GPIO flexible pin.
 ///
-/// This pin can either be a disconnected, input, or output pin. The level register bit will remain
+/// This pin can either be a disconnected, input, or output pin, or both. The level register bit will remain
 /// set while not in output mode, so the pin's level will be 'remembered' when it is not in output
 /// mode.
 pub struct Flex<'d, T: Pin> {
@@ -158,17 +179,7 @@ impl<'d, T: Pin> Flex<'d, T> {
         self.pin.conf().write(|w| {
             w.dir().input();
             w.input().connect();
-            match pull {
+            w.pull().variant(convert_pull(pull));
-                Pull::None => {
-                    w.pull().disabled();
-                }
-                Pull::Up => {
-                    w.pull().pullup();
-                }
-                Pull::Down => {
-                    w.pull().pulldown();
-                }
-            }
             w.drive().s0s1();
             w.sense().disabled();
             w
@@ -180,22 +191,31 @@ impl<'d, T: Pin> Flex<'d, T> {
     /// The pin level will be whatever was set before (or low by default). If you want it to begin
     /// at a specific level, call `set_high`/`set_low` on the pin first.
     pub fn set_as_output(&mut self, drive: OutputDrive) {
-        let drive = match drive {
-            OutputDrive::Standard => DRIVE_A::S0S1,
-            OutputDrive::HighDrive0Standard1 => DRIVE_A::H0S1,
-            OutputDrive::Standard0HighDrive1 => DRIVE_A::S0H1,
-            OutputDrive::HighDrive => DRIVE_A::H0H1,
-            OutputDrive::Disconnect0Standard1 => DRIVE_A::D0S1,
-            OutputDrive::Disconnect0HighDrive1 => DRIVE_A::D0H1,
-            OutputDrive::Standard0Disconnect1 => DRIVE_A::S0D1,
-            OutputDrive::HighDrive0Disconnect1 => DRIVE_A::H0D1,
-        };
-
         self.pin.conf().write(|w| {
             w.dir().output();
             w.input().disconnect();
             w.pull().disabled();
-            w.drive().variant(drive);
+            w.drive().variant(convert_drive(drive));
+            w.sense().disabled();
+            w
+        });
+    }
+
+    /// Put the pin into input + output mode.
+    ///
+    /// This is commonly used for "open drain" mode. If you set `drive = Standard0Disconnect1`,
+    /// the hardware will drive the line low if you set it to low, and will leave it floating if you set
+    /// it to high, in which case you can read the input to figure out whether another device
+    /// is driving the line low.
+    ///
+    /// The pin level will be whatever was set before (or low by default). If you want it to begin
+    /// at a specific level, call `set_high`/`set_low` on the pin first.
+    pub fn set_as_input_output(&mut self, pull: Pull, drive: OutputDrive) {
+        self.pin.conf().write(|w| {
+            w.dir().output();
+            w.input().connect();
+            w.pull().variant(convert_pull(pull));
+            w.drive().variant(convert_drive(drive));
             w.sense().disabled();
             w
         });

embassy-nrf/src/twim.rs

@@ -11,6 +11,8 @@ use core::marker::PhantomData;
 use core::sync::atomic::{compiler_fence, Ordering::SeqCst};
 use core::task::Poll;
 use embassy::interrupt::{Interrupt, InterruptExt};
+#[cfg(feature = "time")]
+use embassy::time::{Duration, Instant};
 use embassy::util::Unborrow;
 use embassy::waitqueue::AtomicWaker;
 use embassy_hal_common::unborrow;
@@ -34,7 +36,9 @@ pub enum Frequency {
 #[non_exhaustive]
 pub struct Config {
     pub frequency: Frequency,
+    pub sda_high_drive: bool,
     pub sda_pullup: bool,
+    pub scl_high_drive: bool,
     pub scl_pullup: bool,
 }
 
@@ -42,7 +46,9 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             frequency: Frequency::K100,
+            scl_high_drive: false,
             sda_pullup: false,
+            sda_high_drive: false,
             scl_pullup: false,
         }
     }
@@ -62,6 +68,7 @@ pub enum Error {
     AddressNack,
     DataNack,
     Overrun,
+    Timeout,
 }
 
 /// Interface to a TWIM instance using EasyDMA to offload the transmission and reception workload.
@@ -87,7 +94,11 @@ impl<'d, T: Instance> Twim<'d, T> {
         sda.conf().write(|w| {
             w.dir().input();
             w.input().connect();
-            w.drive().s0d1();
+            if config.sda_high_drive {
+                w.drive().h0d1();
+            } else {
+                w.drive().s0d1();
+            }
             if config.sda_pullup {
                 w.pull().pullup();
             }
@@ -96,7 +107,11 @@ impl<'d, T: Instance> Twim<'d, T> {
         scl.conf().write(|w| {
             w.dir().input();
             w.input().connect();
-            w.drive().s0d1();
+            if config.scl_high_drive {
+                w.drive().h0d1();
+            } else {
+                w.drive().s0d1();
+            }
             if config.scl_pullup {
                 w.pull().pullup();
             }
@@ -266,6 +281,29 @@ impl<'d, T: Instance> Twim<'d, T> {
         }
     }
 
+    /// Wait for stop or error
+    #[cfg(feature = "time")]
+    fn blocking_wait_timeout(&mut self, timeout: Duration) -> Result<(), Error> {
+        let r = T::regs();
+        let deadline = Instant::now() + timeout;
+        loop {
+            if r.events_stopped.read().bits() != 0 {
+                r.events_stopped.reset();
+                break;
+            }
+            if r.events_error.read().bits() != 0 {
+                r.events_error.reset();
+                r.tasks_stop.write(|w| unsafe { w.bits(1) });
+            }
+            if Instant::now() > deadline {
+                r.tasks_stop.write(|w| unsafe { w.bits(1) });
+                return Err(Error::Timeout);
+            }
+        }
+
+        Ok(())
+    }
+
     /// Wait for stop or error
     fn async_wait(&mut self) -> impl Future<Output = ()> {
         poll_fn(move |cx| {
@@ -493,6 +531,103 @@ impl<'d, T: Instance> Twim<'d, T> {
         Ok(())
     }
 
+    // ===========================================
+
+    /// Write to an I2C slave with timeout.
+    ///
+    /// See [`blocking_write`].
+    #[cfg(feature = "time")]
+    pub fn blocking_write_timeout(
+        &mut self,
+        address: u8,
+        buffer: &[u8],
+        timeout: Duration,
+    ) -> Result<(), Error> {
+        self.setup_write(address, buffer, false)?;
+        self.blocking_wait_timeout(timeout)?;
+        compiler_fence(SeqCst);
+        self.check_errorsrc()?;
+        self.check_tx(buffer.len())?;
+        Ok(())
+    }
+
+    /// Same as [`blocking_write`](Twim::blocking_write) but will fail instead of copying data into RAM. Consult the module level documentation to learn more.
+    #[cfg(feature = "time")]
+    pub fn blocking_write_from_ram_timeout(
+        &mut self,
+        address: u8,
+        buffer: &[u8],
+        timeout: Duration,
+    ) -> Result<(), Error> {
+        self.setup_write_from_ram(address, buffer, false)?;
+        self.blocking_wait_timeout(timeout)?;
+        compiler_fence(SeqCst);
+        self.check_errorsrc()?;
+        self.check_tx(buffer.len())?;
+        Ok(())
+    }
+
+    /// Read from an I2C slave.
+    ///
+    /// The buffer must have a length of at most 255 bytes on the nRF52832
+    /// and at most 65535 bytes on the nRF52840.
+    #[cfg(feature = "time")]
+    pub fn blocking_read_timeout(
+        &mut self,
+        address: u8,
+        buffer: &mut [u8],
+        timeout: Duration,
+    ) -> Result<(), Error> {
+        self.setup_read(address, buffer, false)?;
+        self.blocking_wait_timeout(timeout)?;
+        compiler_fence(SeqCst);
+        self.check_errorsrc()?;
+        self.check_rx(buffer.len())?;
+        Ok(())
+    }
+
+    /// Write data to an I2C slave, then read data from the slave without
+    /// triggering a stop condition between the two.
+    ///
+    /// The buffers must have a length of at most 255 bytes on the nRF52832
+    /// and at most 65535 bytes on the nRF52840.
+    #[cfg(feature = "time")]
+    pub fn blocking_write_read_timeout(
+        &mut self,
+        address: u8,
+        wr_buffer: &[u8],
+        rd_buffer: &mut [u8],
+        timeout: Duration,
+    ) -> Result<(), Error> {
+        self.setup_write_read(address, wr_buffer, rd_buffer, false)?;
+        self.blocking_wait_timeout(timeout)?;
+        compiler_fence(SeqCst);
+        self.check_errorsrc()?;
+        self.check_tx(wr_buffer.len())?;
+        self.check_rx(rd_buffer.len())?;
+        Ok(())
+    }
+
+    /// Same as [`blocking_write_read`](Twim::blocking_write_read) but will fail instead of copying data into RAM. Consult the module level documentation to learn more.
+    #[cfg(feature = "time")]
+    pub fn blocking_write_read_from_ram_timeout(
+        &mut self,
+        address: u8,
+        wr_buffer: &[u8],
+        rd_buffer: &mut [u8],
+        timeout: Duration,
+    ) -> Result<(), Error> {
+        self.setup_write_read_from_ram(address, wr_buffer, rd_buffer, false)?;
+        self.blocking_wait_timeout(timeout)?;
+        compiler_fence(SeqCst);
+        self.check_errorsrc()?;
+        self.check_tx(wr_buffer.len())?;
+        self.check_rx(rd_buffer.len())?;
+        Ok(())
+    }
+
+    // ===========================================
+
     pub async fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Error> {
         self.setup_read(address, buffer, true)?;
         self.async_wait().await;
@@ -677,6 +812,7 @@ mod eh1 {
                     embedded_hal_1::i2c::NoAcknowledgeSource::Data,
                 ),
                 Self::Overrun => embedded_hal_1::i2c::ErrorKind::Overrun,
+                Self::Timeout => embedded_hal_1::i2c::ErrorKind::Other,
             }
         }
     }

embassy-stm32/src/gpio.rs

@@ -3,7 +3,6 @@ use core::convert::Infallible;
 use core::marker::PhantomData;
 use embassy::util::Unborrow;
 use embassy_hal_common::{unborrow, unsafe_impl_unborrow};
-use embedded_hal_02::digital::v2::{InputPin, OutputPin, StatefulOutputPin, ToggleableOutputPin};
 
 use crate::pac;
 use crate::pac::gpio::{self, vals};
@@ -605,6 +604,9 @@ pub(crate) unsafe fn init() {
 
 mod eh02 {
     use super::*;
+    use embedded_hal_02::digital::v2::{
+        InputPin, OutputPin, StatefulOutputPin, ToggleableOutputPin,
+    };
 
     impl<'d, T: Pin> InputPin for Input<'d, T> {
         type Error = Infallible;
@@ -691,6 +693,103 @@ mod eh02 {
     }
 }
 
+#[cfg(feature = "unstable-traits")]
+mod eh1 {
+    use super::*;
+    use embedded_hal_1::digital::blocking::{
+        InputPin, OutputPin, StatefulOutputPin, ToggleableOutputPin,
+    };
+    use embedded_hal_1::digital::ErrorType;
+
+    impl<'d, T: Pin> ErrorType for Input<'d, T> {
+        type Error = Infallible;
+    }
+
+    impl<'d, T: Pin> InputPin for Input<'d, T> {
+        #[inline]
+        fn is_high(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_high())
+        }
+
+        #[inline]
+        fn is_low(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_low())
+        }
+    }
+
+    impl<'d, T: Pin> ErrorType for Output<'d, T> {
+        type Error = Infallible;
+    }
+
+    impl<'d, T: Pin> OutputPin for Output<'d, T> {
+        #[inline]
+        fn set_high(&mut self) -> Result<(), Self::Error> {
+            Ok(self.set_high())
+        }
+
+        #[inline]
+        fn set_low(&mut self) -> Result<(), Self::Error> {
+            Ok(self.set_low())
+        }
+    }
+
+    impl<'d, T: Pin> StatefulOutputPin for Output<'d, T> {
+        #[inline]
+        fn is_set_high(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_set_high())
+        }
+
+        /// Is the output pin set as low?
+        #[inline]
+        fn is_set_low(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_set_low())
+        }
+    }
+
+    impl<'d, T: Pin> ToggleableOutputPin for Output<'d, T> {
+        #[inline]
+        fn toggle(&mut self) -> Result<(), Self::Error> {
+            Ok(self.toggle())
+        }
+    }
+
+    impl<'d, T: Pin> ErrorType for OutputOpenDrain<'d, T> {
+        type Error = Infallible;
+    }
+
+    impl<'d, T: Pin> OutputPin for OutputOpenDrain<'d, T> {
+        #[inline]
+        fn set_high(&mut self) -> Result<(), Self::Error> {
+            Ok(self.set_high())
+        }
+
+        #[inline]
+        fn set_low(&mut self) -> Result<(), Self::Error> {
+            Ok(self.set_low())
+        }
+    }
+
+    impl<'d, T: Pin> StatefulOutputPin for OutputOpenDrain<'d, T> {
+        #[inline]
+        fn is_set_high(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_set_high())
+        }
+
+        /// Is the output pin set as low?
+        #[inline]
+        fn is_set_low(&self) -> Result<bool, Self::Error> {
+            Ok(self.is_set_low())
+        }
+    }
+
+    impl<'d, T: Pin> ToggleableOutputPin for OutputOpenDrain<'d, T> {
+        #[inline]
+        fn toggle(&mut self) -> Result<(), Self::Error> {
+            Ok(self.toggle())
+        }
+    }
+}
+
 #[cfg(feature = "unstable-pac")]
 pub mod low_level {
     pub use super::sealed::*;

embassy/src/time/timer.rs

@@ -7,6 +7,8 @@ use crate::executor::raw;
 use crate::time::{Duration, Instant};
 
 /// Error returned by [`with_timeout`] on timeout.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct TimeoutError;
 
 /// Runs a given future with a timeout.