From d8c9a3b159b3c2ae2ad60b5105d16f4c42c63b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Mon, 23 Dec 2024 11:39:07 +0100 Subject: [PATCH 1/3] Add `AcknowledgeCheckFailedReason` --- esp-hal/MIGRATING-0.22.md | 7 ++++ esp-hal/src/i2c/master/mod.rs | 63 ++++++++++++++++++++++++++++++----- hil-test/tests/i2c.rs | 7 ++-- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 6e2d8a256ff..297a562694a 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -317,6 +317,13 @@ To avoid abbreviations and contractions (as per the esp-hal guidelines), some er + Error::ZeroLengthInvalid ``` +The `AckCheckFailed` variant changed to `AcknowledgeCheckFailed(AcknowledgeCheckFailedReason)` + +```diff +- Err(Error::AckCheckFailed) ++ Err(Error::AcknowledgeCheckFailed(reason)) +``` + ## The crate prelude has been removed The reexports that were previously part of the prelude are available through other paths: diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs index d317c9384c6..5cc46f7443b 100644 --- a/esp-hal/src/i2c/master/mod.rs +++ b/esp-hal/src/i2c/master/mod.rs @@ -93,7 +93,7 @@ pub enum Error { /// The transmission exceeded the FIFO size. FifoExceeded, /// The acknowledgment check failed. - AckCheckFailed, + AcknowledgeCheckFailed(AcknowledgeCheckFailedReason), /// A timeout occurred during transmission. Timeout, /// The arbitration for the bus was lost. @@ -106,13 +106,54 @@ pub enum Error { ZeroLengthInvalid, } +/// I2C no acknowledge error reason. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[non_exhaustive] +pub enum AcknowledgeCheckFailedReason { + /// The device did not acknowledge its address. The device may be missing. + Address, + /// The device did not acknowledge the data. It may not be ready to process + /// requests at the moment. + Data, + /// Either the device did not acknowledge its address or the data, but it is + /// unknown which. + Unknown, +} + +impl core::fmt::Display for AcknowledgeCheckFailedReason { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + AcknowledgeCheckFailedReason::Address => write!(f, "Address"), + AcknowledgeCheckFailedReason::Data => write!(f, "Data"), + AcknowledgeCheckFailedReason::Unknown => write!(f, "Unknown"), + } + } +} + +impl From<&AcknowledgeCheckFailedReason> for embedded_hal::i2c::NoAcknowledgeSource { + fn from(value: &AcknowledgeCheckFailedReason) -> Self { + match value { + AcknowledgeCheckFailedReason::Address => { + embedded_hal::i2c::NoAcknowledgeSource::Address + } + AcknowledgeCheckFailedReason::Data => embedded_hal::i2c::NoAcknowledgeSource::Data, + AcknowledgeCheckFailedReason::Unknown => { + embedded_hal::i2c::NoAcknowledgeSource::Unknown + } + } + } +} + impl core::error::Error for Error {} impl core::fmt::Display for Error { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { Error::FifoExceeded => write!(f, "The transmission exceeded the FIFO size"), - Error::AckCheckFailed => write!(f, "The acknowledgment check failed"), + Error::AcknowledgeCheckFailed(reason) => { + write!(f, "The acknowledgment check failed. Reason: {}", reason) + } Error::Timeout => write!(f, "A timeout occurred during transmission"), Error::ArbitrationLost => write!(f, "The arbitration for the bus was lost"), Error::ExecutionIncomplete => { @@ -211,12 +252,12 @@ impl Operation<'_> { impl embedded_hal::i2c::Error for Error { fn kind(&self) -> embedded_hal::i2c::ErrorKind { - use embedded_hal::i2c::{ErrorKind, NoAcknowledgeSource}; + use embedded_hal::i2c::ErrorKind; match self { Self::FifoExceeded => ErrorKind::Overrun, Self::ArbitrationLost => ErrorKind::ArbitrationLoss, - Self::AckCheckFailed => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Unknown), + Self::AcknowledgeCheckFailed(reason) => ErrorKind::NoAcknowledge(reason.into()), _ => ErrorKind::Other, } } @@ -657,7 +698,9 @@ impl<'a> I2cFuture<'a> { } if r.nack().bit_is_set() { - return Err(Error::AckCheckFailed); + return Err(Error::AcknowledgeCheckFailed( + AcknowledgeCheckFailedReason::Unknown, + )); } #[cfg(not(esp32))] @@ -670,7 +713,9 @@ impl<'a> I2cFuture<'a> { .resp_rec() .bit_is_clear() { - return Err(Error::AckCheckFailed); + return Err(Error::AcknowledgeCheckFailed( + AcknowledgeCheckFailedReason::Unknown, + )); } Ok(()) @@ -1685,7 +1730,7 @@ impl Driver<'_> { let retval = if interrupts.time_out().bit_is_set() { Err(Error::Timeout) } else if interrupts.nack().bit_is_set() { - Err(Error::AckCheckFailed) + Err(Error::AcknowledgeCheckFailed) } else if interrupts.arbitration_lost().bit_is_set() { Err(Error::ArbitrationLost) } else { @@ -1696,11 +1741,11 @@ impl Driver<'_> { let retval = if interrupts.time_out().bit_is_set() { Err(Error::Timeout) } else if interrupts.nack().bit_is_set() { - Err(Error::AckCheckFailed) + Err(Error::AcknowledgeCheckFailed(AcknowledgeCheckFailedReason::Unknown)) } else if interrupts.arbitration_lost().bit_is_set() { Err(Error::ArbitrationLost) } else if interrupts.trans_complete().bit_is_set() && self.register_block().sr().read().resp_rec().bit_is_clear() { - Err(Error::AckCheckFailed) + Err(Error::AcknowledgeCheckFailed(AcknowledgeCheckFailedReason::Unknown)) } else { Ok(()) }; diff --git a/hil-test/tests/i2c.rs b/hil-test/tests/i2c.rs index 14c1620478c..514e25b674d 100644 --- a/hil-test/tests/i2c.rs +++ b/hil-test/tests/i2c.rs @@ -6,7 +6,7 @@ #![no_main] use esp_hal::{ - i2c::master::{Config, Error, I2c, Operation}, + i2c::master::{AcknowledgeCheckFailedReason, Config, Error, I2c, Operation}, Async, Blocking, }; @@ -50,9 +50,12 @@ mod tests { #[test] fn empty_write_returns_ack_error_for_unknown_address(mut ctx: Context) { + // we don't specify the exact reason yet assert_eq!( ctx.i2c.write(NON_EXISTENT_ADDRESS, &[]), - Err(Error::AckCheckFailed) + Err(Error::AcknowledgeCheckFailed( + AcknowledgeCheckFailedReason::Unknown + )) ); assert_eq!(ctx.i2c.write(DUT_ADDRESS, &[]), Ok(())); } From 8ebb1f97d6b89fc04cf9a7b369979a8278094dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Mon, 23 Dec 2024 11:55:49 +0100 Subject: [PATCH 2/3] Fix --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/i2c/master/mod.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 36a6e86aed5..9d681f1230a 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -92,6 +92,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851) - UART: Make `AtCmdConfig` use builder-lite pattern (#2851) - UART: Fix naming violations for `DataBits`, `Parity`, and `StopBits` enum variants (#2893) +- Renamed / changed some I2C error variants (#2844, #2862) ### Fixed diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs index 5cc46f7443b..2bae6d95450 100644 --- a/esp-hal/src/i2c/master/mod.rs +++ b/esp-hal/src/i2c/master/mod.rs @@ -1730,7 +1730,7 @@ impl Driver<'_> { let retval = if interrupts.time_out().bit_is_set() { Err(Error::Timeout) } else if interrupts.nack().bit_is_set() { - Err(Error::AcknowledgeCheckFailed) + Err(Error::AcknowledgeCheckFailed(AcknowledgeCheckFailedReason::Unknown)) } else if interrupts.arbitration_lost().bit_is_set() { Err(Error::ArbitrationLost) } else { From 28e162fa03ee2b4e58aee3c48edae6d9818917b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Mon, 6 Jan 2025 13:53:44 +0100 Subject: [PATCH 3/3] Remove unused AcknowledgeCheckFailedReason variants --- esp-hal/src/i2c/master/mod.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs index 2bae6d95450..76275d11c97 100644 --- a/esp-hal/src/i2c/master/mod.rs +++ b/esp-hal/src/i2c/master/mod.rs @@ -111,11 +111,6 @@ pub enum Error { #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[non_exhaustive] pub enum AcknowledgeCheckFailedReason { - /// The device did not acknowledge its address. The device may be missing. - Address, - /// The device did not acknowledge the data. It may not be ready to process - /// requests at the moment. - Data, /// Either the device did not acknowledge its address or the data, but it is /// unknown which. Unknown, @@ -124,8 +119,6 @@ pub enum AcknowledgeCheckFailedReason { impl core::fmt::Display for AcknowledgeCheckFailedReason { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - AcknowledgeCheckFailedReason::Address => write!(f, "Address"), - AcknowledgeCheckFailedReason::Data => write!(f, "Data"), AcknowledgeCheckFailedReason::Unknown => write!(f, "Unknown"), } } @@ -134,10 +127,6 @@ impl core::fmt::Display for AcknowledgeCheckFailedReason { impl From<&AcknowledgeCheckFailedReason> for embedded_hal::i2c::NoAcknowledgeSource { fn from(value: &AcknowledgeCheckFailedReason) -> Self { match value { - AcknowledgeCheckFailedReason::Address => { - embedded_hal::i2c::NoAcknowledgeSource::Address - } - AcknowledgeCheckFailedReason::Data => embedded_hal::i2c::NoAcknowledgeSource::Data, AcknowledgeCheckFailedReason::Unknown => { embedded_hal::i2c::NoAcknowledgeSource::Unknown }