Merge #316

316: I2C NACK error nesting r=therealprof a=yodaldevoid

This idea was first proposed [here](#296 (comment)), but broken out to keep things moving.

This merges `ErrorKind::NoAcknowledgeData` and `ErrorKind::NoAcknowledgeAddress` into one variant with a source field to indicate what the NACK was in response to. Some drivers may not be able to differentiate between NACKs on address or data bytes so they should use `NoAcknowledgeSource::Unknown`.

Feel free to bikeshed names and documentation. I used the most obvious names to me, but naming is one of the hardest problems in programming.

Co-authored-by: Gabriel Smith <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   implement these `Error` traits, which implies providing a conversion to a common
   set of error kinds. Generic drivers using these interfaces can then convert the errors
   to this common set to act upon them.
+- The `NoAcknowledgeAddress` and `NoAcknowledgeData` variants of the I2C `Error`
+  trait have been merged into `NoAcknowledge` with a `NoAcknowledgeSource` field
+  to differentiate between the two events. An additional `Unknown` variant is
+  provided in `NoAcknowledgeSource` for devices that can't differentiate between
+  the two events.
 
 ### Removed
 - Removed `DelayMs` in favor of `DelayUs` with `u32` as type for clarity.

src/i2c.rs

@@ -119,20 +119,38 @@ pub trait Error: core::fmt::Debug {
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[non_exhaustive]
 pub enum ErrorKind {
-    /// Bus error occurred. e.g. A START or a STOP condition is detected and is not located after a multiple of 9 SCL clock pulses.
+    /// Bus error occurred. e.g. A START or a STOP condition is detected and is not
+    /// located after a multiple of 9 SCL clock pulses.
     Bus,
     /// The arbitration was lost, e.g. electrical problems with the clock signal
     ArbitrationLoss,
-    /// The device did not acknowledge its address. The device may be missing.
-    NoAcknowledgeAddress,
-    /// The device did not acknowled the data. It may not be ready to process requests at the moment.
-    NoAcknowledgeData,
+    /// A bus operation was not acknowledged, e.g. due to the addressed device not
+    /// being available on the bus or the device not being ready to process requests
+    /// at the moment
+    NoAcknowledge(NoAcknowledgeSource),
     /// The peripheral receive buffer was overrun
     Overrun,
     /// A different error occurred. The original error may contain more information.
     Other,
 }
 
+/// I2C no acknowledge error source
+///
+/// In cases where it is possible, a device should indicate if a no acknowledge
+/// response was received to an address versus a no acknowledge to a data byte.
+/// Where it is not possible to differentiate, `Unknown` should be indicated.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub enum NoAcknowledgeSource {
+    /// 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 Error for ErrorKind {
     fn kind(&self) -> ErrorKind {
         *self
@@ -144,8 +162,7 @@ impl core::fmt::Display for ErrorKind {
         match self {
             Self::Bus => write!(f, "Bus error occurred"),
             Self::ArbitrationLoss => write!(f, "The arbitration was lost"),
-            Self::NoAcknowledgeAddress => write!(f, "The device did not acknowledge its address"),
-            Self::NoAcknowledgeData => write!(f, "The device did not acknowledge the data"),
+            Self::NoAcknowledge(s) => s.fmt(f),
             Self::Overrun => write!(f, "The peripheral receive buffer was overrun"),
             Self::Other => write!(
                 f,
@@ -155,6 +172,16 @@ impl core::fmt::Display for ErrorKind {
     }
 }
 
+impl core::fmt::Display for NoAcknowledgeSource {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Self::Address => write!(f, "The device did not acknowledge its address"),
+            Self::Data => write!(f, "The device did not acknowledge the data"),
+            Self::Unknown => write!(f, "The device did not acknowledge its address or the data"),
+        }
+    }
+}
+
 /// Address mode (7-bit / 10-bit)
 ///
 /// Note: This trait is sealed and should not be implemented outside of this crate.