From ef7842fab4236b6824fef47f30fe8ef75f7588ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 14 Oct 2024 14:50:37 +0200 Subject: [PATCH] Reimplement blocking trait for async i2c (#2343) --- esp-hal/CHANGELOG.md | 2 + esp-hal/src/i2c.rs | 157 ++++++++++++++++++++++-------------------- hil-test/tests/i2c.rs | 10 +++ 3 files changed, 95 insertions(+), 74 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 3fc3ade03..10b7f43e4 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Restored blocking `embedded_hal` compatibility for async I2C driver (#2343) + ### Removed ## [0.21.0] diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index f6ed59cc8..87d15c9b6 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -63,7 +63,10 @@ use crate::{ peripheral::{Peripheral, PeripheralRef}, peripherals::i2c0::{RegisterBlock, COMD}, system::PeripheralClockControl, + Async, + Blocking, InterruptConfigurable, + Mode, }; cfg_if::cfg_if! { @@ -229,14 +232,77 @@ impl From for u32 { } /// I2C driver -pub struct I2c<'d, T, DM: crate::Mode> { +pub struct I2c<'d, T, DM: Mode> { peripheral: PeripheralRef<'d, T>, phantom: PhantomData, frequency: HertzU32, timeout: Option, } -impl I2c<'_, T, crate::Blocking> +impl I2c<'_, T, DM> +where + T: Instance, + DM: Mode, +{ + fn transaction_impl<'a>( + &mut self, + address: u8, + operations: impl Iterator>, + ) -> Result<(), Error> { + let mut last_op: Option = None; + // filter out 0 length read operations + let mut op_iter = operations + .filter(|op| op.is_write() || !op.is_empty()) + .peekable(); + + while let Some(op) = op_iter.next() { + let next_op = op_iter.peek().map(|v| v.kind()); + let kind = op.kind(); + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + + let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + match op { + Operation::Write(buffer) => { + // execute a write operation: + // - issue START/RSTART if op is different from previous + // - issue STOP if op is the last one + self.peripheral + .write_operation( + address, + buffer, + !matches!(last_op, Some(OpKind::Write)), + next_op.is_none(), + cmd_iterator, + ) + .inspect_err(|_| self.internal_recover())?; + } + Operation::Read(buffer) => { + // execute a read operation: + // - issue START/RSTART if op is different from previous + // - issue STOP if op is the last one + // - will_continue is true if there is another read operation next + self.peripheral + .read_operation( + address, + buffer, + !matches!(last_op, Some(OpKind::Read)), + next_op.is_none(), + matches!(next_op, Some(OpKind::Read)), + cmd_iterator, + ) + .inspect_err(|_| self.internal_recover())?; + } + } + + last_op = Some(kind); + } + + Ok(()) + } +} + +impl I2c<'_, T, Blocking> where T: Instance, { @@ -361,66 +427,9 @@ where ) -> Result<(), Error> { self.transaction_impl(address, operations.into_iter().map(Operation::from)) } - - fn transaction_impl<'a>( - &mut self, - address: u8, - operations: impl Iterator>, - ) -> Result<(), Error> { - let mut last_op: Option = None; - // filter out 0 length read operations - let mut op_iter = operations - .filter(|op| op.is_write() || !op.is_empty()) - .peekable(); - - while let Some(op) = op_iter.next() { - let next_op = op_iter.peek().map(|v| v.kind()); - let kind = op.kind(); - // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); - - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); - match op { - Operation::Write(buffer) => { - // execute a write operation: - // - issue START/RSTART if op is different from previous - // - issue STOP if op is the last one - self.peripheral - .write_operation( - address, - buffer, - !matches!(last_op, Some(OpKind::Write)), - next_op.is_none(), - cmd_iterator, - ) - .inspect_err(|_| self.internal_recover())?; - } - Operation::Read(buffer) => { - // execute a read operation: - // - issue START/RSTART if op is different from previous - // - issue STOP if op is the last one - // - will_continue is true if there is another read operation next - self.peripheral - .read_operation( - address, - buffer, - !matches!(last_op, Some(OpKind::Read)), - next_op.is_none(), - matches!(next_op, Some(OpKind::Read)), - cmd_iterator, - ) - .inspect_err(|_| self.internal_recover())?; - } - } - - last_op = Some(kind); - } - - Ok(()) - } } -impl embedded_hal_02::blocking::i2c::Read for I2c<'_, T, crate::Blocking> +impl embedded_hal_02::blocking::i2c::Read for I2c<'_, T, Blocking> where T: Instance, { @@ -431,7 +440,7 @@ where } } -impl embedded_hal_02::blocking::i2c::Write for I2c<'_, T, crate::Blocking> +impl embedded_hal_02::blocking::i2c::Write for I2c<'_, T, Blocking> where T: Instance, { @@ -442,7 +451,7 @@ where } } -impl embedded_hal_02::blocking::i2c::WriteRead for I2c<'_, T, crate::Blocking> +impl embedded_hal_02::blocking::i2c::WriteRead for I2c<'_, T, Blocking> where T: Instance, { @@ -458,11 +467,11 @@ where } } -impl embedded_hal::i2c::ErrorType for I2c<'_, T, DM> { +impl embedded_hal::i2c::ErrorType for I2c<'_, T, DM> { type Error = Error; } -impl embedded_hal::i2c::I2c for I2c<'_, T, crate::Blocking> +impl embedded_hal::i2c::I2c for I2c<'_, T, DM> where T: Instance, { @@ -475,7 +484,7 @@ where } } -impl<'d, T, DM: crate::Mode> I2c<'d, T, DM> +impl<'d, T, DM: Mode> I2c<'d, T, DM> where T: Instance, { @@ -548,7 +557,7 @@ where } } -impl<'d, T> I2c<'d, T, crate::Blocking> +impl<'d, T> I2c<'d, T, Blocking> where T: Instance, { @@ -581,9 +590,9 @@ where } } -impl<'d, T> crate::private::Sealed for I2c<'d, T, crate::Blocking> where T: Instance {} +impl<'d, T> crate::private::Sealed for I2c<'d, T, Blocking> where T: Instance {} -impl<'d, T> InterruptConfigurable for I2c<'d, T, crate::Blocking> +impl<'d, T> InterruptConfigurable for I2c<'d, T, Blocking> where T: Instance, { @@ -592,7 +601,7 @@ where } } -impl<'d, T> I2c<'d, T, crate::Async> +impl<'d, T> I2c<'d, T, Async> where T: Instance, { @@ -779,7 +788,7 @@ mod asynch { } } - impl I2c<'_, T, crate::Async> + impl I2c<'_, T, Async> where T: Instance, { @@ -1110,11 +1119,11 @@ mod asynch { address: u8, operations: impl IntoIterator>, ) -> Result<(), Error> { - self.transaction_impl(address, operations.into_iter().map(Operation::from)) + self.transaction_impl_async(address, operations.into_iter().map(Operation::from)) .await } - async fn transaction_impl<'a>( + async fn transaction_impl_async<'a>( &mut self, address: u8, operations: impl Iterator>, @@ -1170,7 +1179,7 @@ mod asynch { } } - impl<'d, T> embedded_hal_async::i2c::I2c for I2c<'d, T, crate::Async> + impl<'d, T> embedded_hal_async::i2c::I2c for I2c<'d, T, Async> where T: Instance, { @@ -1179,7 +1188,7 @@ mod asynch { address: u8, operations: &mut [EhalOperation<'_>], ) -> Result<(), Self::Error> { - self.transaction_impl(address, operations.iter_mut().map(Operation::from)) + self.transaction_impl_async(address, operations.iter_mut().map(Operation::from)) .await } } diff --git a/hil-test/tests/i2c.rs b/hil-test/tests/i2c.rs index 7b2367e99..c8784abca 100644 --- a/hil-test/tests/i2c.rs +++ b/hil-test/tests/i2c.rs @@ -10,6 +10,7 @@ use esp_hal::{ i2c::{I2c, Operation}, peripherals::I2C0, prelude::*, + Async, Blocking, }; use hil_test as _; @@ -17,6 +18,15 @@ use hil_test as _; struct Context { i2c: I2c<'static, I2C0, Blocking>, } + +fn _async_driver_is_compatible_with_blocking_ehal() { + fn _with_driver(driver: I2c<'static, I2C0, Async>) { + _with_ehal(driver); + } + + fn _with_ehal(_: impl embedded_hal::i2c::I2c) {} +} + #[cfg(test)] #[embedded_test::tests] mod tests {