Implement apply_config, SetConfig

esp-hal/CHANGELOG.md

@@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `{Uart, UartRx, UartTx}` now implement `embassy_embedded_hal::SetConfig` (#2449)
 - GPIO ETM tasks and events now accept `InputSignal` and `OutputSignal` (#2427)
 - `spi::master::Config` and `{Spi, SpiDma, SpiDmaBus}::apply_config` (#2448)
-- `embassy_embedded_hal::SetConfig` is now implemented for `{Spi, SpiDma, SpiDmaBus}` (#2448)
+- `embassy_embedded_hal::SetConfig` is now implemented for `spi::master::{Spi, SpiDma, SpiDmaBus}`, `i2c::master::I2c` (#2448, #?)
 
 ### Changed
 

esp-hal/MIGRATING-0.21.md

@@ -69,11 +69,17 @@ let spi: Spi<'static, FullDuplexMode, SPI2> = Spi::new_typed(peripherals.SPI2, 1
 
 The I2C master driver and related types have been moved to `esp_hal::i2c::master`.
 
-The `with_timeout` constructors have been removed in favour of `set_timeout` or `with_timeout`.
+The `with_timeout` constructors have been removed. `new` and `new_typed` now take a `Config` struct
+with the available configuration options.
 
 ```diff
 -let i2c = I2c::new_with_timeout(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz(), timeout);
-+let i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz()).with_timeout(timeout);
++let i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, {
++    let mut config = Config::default();
++    config.frequency = 100.kHz();
++    config.timeout = timeout;
++    config
++});
 ```
 
 ## Changes to half-duplex SPI

esp-hal/src/i2c/master/mod.rs

@@ -18,7 +18,7 @@
 //!
 //! ```rust, no_run
 #![doc = crate::before_snippet!()]
-//! # use esp_hal::i2c::master::I2c;
+//! # use esp_hal::i2c::master::{Config, I2c};
 //! # use esp_hal::gpio::Io;
 //! let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
 //!
@@ -28,7 +28,7 @@
 //!     peripherals.I2C0,
 //!     io.pins.gpio1,
 //!     io.pins.gpio2,
-//!     100.kHz(),
+//!     Config::default(),
 //! );
 //!
 //! loop {
@@ -46,6 +46,7 @@ use core::{
     task::{Context, Poll},
 };
 
+use embassy_embedded_hal::SetConfig;
 use embassy_sync::waitqueue::AtomicWaker;
 use embedded_hal::i2c::Operation as EhalOperation;
 use fugit::HertzU32;
@@ -107,6 +108,11 @@ pub enum Error {
     InvalidZeroLength,
 }
 
+/// I2C-specific configuration errors
+#[derive(Debug, Clone, Copy, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum ConfigError {}
+
 #[derive(PartialEq)]
 // This enum is used to keep track of the last/next operation that was/will be
 // performed in an embedded-hal(-async) I2c::transaction. It is used to
@@ -231,12 +237,54 @@ impl From<Ack> for u32 {
     }
 }
 
+/// I2C driver configuration
+#[derive(Debug, Clone, Copy)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct Config {
+    /// The I2C clock frequency.
+    pub frequency: HertzU32,
+
+    /// I2C SCL timeout period.
+    ///
+    /// When the level of SCL remains unchanged for more than `timeout` bus
+    /// clock cycles, the bus goes to idle state.
+    ///
+    /// The default value is about 10 bus clock cycles.
+    #[doc = ""]
+    #[cfg_attr(
+        not(esp32),
+        doc = "Note that the effective timeout may be longer than the value configured here."
+    )]
+    #[cfg_attr(not(esp32), doc = "Configuring `None` disables timeout control.")]
+    #[cfg_attr(esp32, doc = "Configuring `None` equals to the maximum timeout value.")]
+    // TODO: when supporting interrupts, document that SCL = high also triggers an interrupt.
+    pub timeout: Option<u32>,
+}
+
+impl Default for Config {
+    fn default() -> Self {
+        use fugit::RateExtU32;
+        Config {
+            frequency: 100.kHz(),
+            timeout: Some(10),
+        }
+    }
+}
+
 /// I2C driver
 pub struct I2c<'d, DM: Mode, T = AnyI2c> {
     i2c: PeripheralRef<'d, T>,
     phantom: PhantomData<DM>,
-    frequency: HertzU32,
-    timeout: Option<u32>,
+    config: Config,
+}
+
+impl<T: Instance, DM: Mode> SetConfig for I2c<'_, DM, T> {
+    type Config = Config;
+    type ConfigError = ConfigError;
+
+    fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> {
+        self.apply_config(config)
+    }
 }
 
 impl<T> embedded_hal_02::blocking::i2c::Read for I2c<'_, Blocking, T>
@@ -302,24 +350,23 @@ where
         i2c: impl Peripheral<P = T> + 'd,
         sda: impl Peripheral<P = impl PeripheralOutput> + 'd,
         scl: impl Peripheral<P = impl PeripheralOutput> + 'd,
-        frequency: HertzU32,
+        config: Config,
     ) -> Self {
         crate::into_ref!(i2c);
         crate::into_mapped_ref!(sda, scl);
 
         let i2c = I2c {
             i2c,
             phantom: PhantomData,
-            frequency,
-            timeout: None,
+            config,
         };
 
         PeripheralClockControl::reset(i2c.i2c.peripheral());
         PeripheralClockControl::enable(i2c.i2c.peripheral());
 
         let i2c = i2c.with_sda(sda).with_scl(scl);
 
-        i2c.info().setup(frequency, None);
+        unwrap!(i2c.info().setup(&i2c.config));
         i2c
     }
 
@@ -331,16 +378,15 @@ where
         PeripheralClockControl::reset(self.i2c.peripheral());
         PeripheralClockControl::enable(self.i2c.peripheral());
 
-        self.info().setup(self.frequency, self.timeout);
+        // We know the configuration is valid, we can ignore the result.
+        _ = self.info().setup(&self.config);
     }
 
-    /// Set the I2C timeout.
-    // TODO: explain this function better - what's the unit, what happens on
-    // timeout, and just what exactly is a timeout in this context?
-    pub fn with_timeout(mut self, timeout: Option<u32>) -> Self {
-        self.timeout = timeout;
-        self.info().setup(self.frequency, self.timeout);
-        self
+    /// Applies a new configuration.
+    pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> {
+        self.info().setup(config)?;
+        self.config = *config;
+        Ok(())
     }
 
     fn transaction_impl<'a>(
@@ -443,9 +489,9 @@ impl<'d> I2c<'d, Blocking> {
         i2c: impl Peripheral<P = impl Instance> + 'd,
         sda: impl Peripheral<P = impl PeripheralOutput> + 'd,
         scl: impl Peripheral<P = impl PeripheralOutput> + 'd,
-        frequency: HertzU32,
+        config: Config,
     ) -> Self {
-        Self::new_typed(i2c.map_into(), sda, scl, frequency)
+        Self::new_typed(i2c.map_into(), sda, scl, config)
     }
 }
 
@@ -460,9 +506,9 @@ where
         i2c: impl Peripheral<P = T> + 'd,
         sda: impl Peripheral<P = impl PeripheralOutput> + 'd,
         scl: impl Peripheral<P = impl PeripheralOutput> + 'd,
-        frequency: HertzU32,
+        config: Config,
     ) -> Self {
-        Self::new_internal(i2c, sda, scl, frequency)
+        Self::new_internal(i2c, sda, scl, config)
     }
 
     // TODO: missing interrupt APIs
@@ -474,8 +520,7 @@ where
         I2c {
             i2c: self.i2c,
             phantom: PhantomData,
-            frequency: self.frequency,
-            timeout: self.timeout,
+            config: self.config,
         }
     }
 
@@ -744,8 +789,7 @@ where
         I2c {
             i2c: self.i2c,
             phantom: PhantomData,
-            frequency: self.frequency,
-            timeout: self.timeout,
+            config: self.config,
         }
     }
 
@@ -1322,7 +1366,7 @@ impl Info {
 
     /// Configures the I2C peripheral with the specified frequency, clocks, and
     /// optional timeout.
-    fn setup(&self, frequency: HertzU32, timeout: Option<u32>) {
+    fn setup(&self, config: &Config) -> Result<(), ConfigError> {
         self.register_block().ctr().write(|w| {
             // Set I2C controller to master mode
             w.ms_mode().set_bit();
@@ -1356,12 +1400,14 @@ impl Info {
                 let clock = clocks.xtal_clock.convert();
             }
         }
-        self.set_frequency(clock, frequency, timeout);
+        self.set_frequency(clock, config.frequency, config.timeout);
 
         self.update_config();
 
         // Reset entire peripheral (also resets fifo)
         self.reset();
+
+        Ok(())
     }
 
     /// Resets the I2C controller (FIFO + FSM + command list)

examples/src/bin/embassy_i2c.rs

@@ -19,7 +19,12 @@
 use embassy_executor::Spawner;
 use embassy_time::{Duration, Timer};
 use esp_backtrace as _;
-use esp_hal::{gpio::Io, i2c::master::I2c, prelude::*, timer::timg::TimerGroup};
+use esp_hal::{
+    gpio::Io,
+    i2c::master::{Config, I2c},
+    prelude::*,
+    timer::timg::TimerGroup,
+};
 use lis3dh_async::{Lis3dh, Range, SlaveAddr};
 
 #[esp_hal_embassy::main]
@@ -31,7 +36,12 @@ async fn main(_spawner: Spawner) {
 
     let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
 
-    let i2c0 = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 400.kHz()).into_async();
+    let i2c0 = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, {
+        let mut config = Config::default();
+        config.frequency = 400.kHz();
+        config
+    })
+    .into_async();
 
     let mut lis3dh = Lis3dh::new_i2c(i2c0, SlaveAddr::Alternate).await.unwrap();
     lis3dh.set_range(Range::G8).await.unwrap();

examples/src/bin/embassy_i2c_bmp180_calibration_data.rs

@@ -20,7 +20,12 @@
 use embassy_executor::Spawner;
 use embassy_time::{Duration, Timer};
 use esp_backtrace as _;
-use esp_hal::{gpio::Io, i2c::master::I2c, prelude::*, timer::timg::TimerGroup};
+use esp_hal::{
+    gpio::Io,
+    i2c::master::{Config, I2c},
+    prelude::*,
+    timer::timg::TimerGroup,
+};
 
 #[esp_hal_embassy::main]
 async fn main(_spawner: Spawner) {
@@ -31,7 +36,12 @@ async fn main(_spawner: Spawner) {
 
     let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
 
-    let mut i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 400.kHz()).into_async();
+    let mut i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, {
+        let mut config = Config::default();
+        config.frequency = 400.kHz();
+        config
+    })
+    .into_async();
 
     loop {
         let mut data = [0u8; 22];

examples/src/bin/i2c_bmp180_calibration_data.rs

@@ -12,7 +12,11 @@
 #![no_main]
 
 use esp_backtrace as _;
-use esp_hal::{gpio::Io, i2c::master::I2c, prelude::*};
+use esp_hal::{
+    gpio::Io,
+    i2c::master::{Config, I2c},
+    prelude::*,
+};
 use esp_println::println;
 
 #[entry]
@@ -23,7 +27,12 @@ fn main() -> ! {
 
     // Create a new peripheral object with the described wiring and standard
     // I2C clock speed:
-    let mut i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz());
+    let mut i2c = I2c::new(
+        peripherals.I2C0,
+        io.pins.gpio4,
+        io.pins.gpio5,
+        Config::default(),
+    );
 
     loop {
         let mut data = [0u8; 22];

examples/src/bin/i2c_display.rs

@@ -22,7 +22,12 @@ use embedded_graphics::{
     text::{Alignment, Text},
 };
 use esp_backtrace as _;
-use esp_hal::{delay::Delay, gpio::Io, i2c::master::I2c, prelude::*};
+use esp_hal::{
+    delay::Delay,
+    gpio::Io,
+    i2c::master::{Config, I2c},
+    prelude::*,
+};
 use ssd1306::{prelude::*, I2CDisplayInterface, Ssd1306};
 
 #[entry]
@@ -34,7 +39,12 @@ fn main() -> ! {
 
     // Create a new peripheral object with the described wiring
     // and standard I2C clock speed
-    let i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz());
+    let i2c = I2c::new(
+        peripherals.I2C0,
+        io.pins.gpio4,
+        io.pins.gpio5,
+        Config::default(),
+    );
 
     // Initialize display
     let interface = I2CDisplayInterface::new(i2c);

examples/src/bin/lcd_cam_ov2640.rs

@@ -29,7 +29,10 @@ use esp_hal::{
     dma::{Dma, DmaPriority},
     dma_rx_stream_buffer,
     gpio::Io,
-    i2c::{self, master::I2c},
+    i2c::{
+        self,
+        master::{Config, I2c},
+    },
     lcd_cam::{
         cam::{Camera, RxEightBits},
         LcdCam,
@@ -79,7 +82,7 @@ fn main() -> ! {
 
     delay.delay_millis(500u32);
 
-    let i2c = I2c::new(peripherals.I2C0, cam_siod, cam_sioc, 100u32.kHz());
+    let i2c = I2c::new(peripherals.I2C0, cam_siod, cam_sioc, Config::default());
 
     let mut sccb = Sccb::new(i2c);
 

hil-test/tests/i2c.rs

@@ -7,8 +7,7 @@
 
 use esp_hal::{
     gpio::Io,
-    i2c::master::{I2c, Operation},
-    prelude::*,
+    i2c::master::{Config, I2c, Operation},
     Async,
     Blocking,
 };
@@ -40,7 +39,7 @@ mod tests {
 
         // Create a new peripheral object with the described wiring and standard
         // I2C clock speed:
-        let i2c = I2c::new(peripherals.I2C0, sda, scl, 100.kHz());
+        let i2c = I2c::new(peripherals.I2C0, sda, scl, Config::default());
 
         Context { i2c }
     }