Repository: esp-rs/esp-hal (rust)

Issue:
C-VALIDATE
`write`, `read`, `write_read` and `transaction` do not validate the address

Pull request:
Validate 7-bit I2C addresses
## Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

### Submission Checklist 📝
- [ ] I have updated existing examples or added new ones (if applicable).
- [x] I have used `cargo xtask fmt-packages` command to ensure that all changed code is formatted correctly.
- [ ] My changes were added to the [`CHANGELOG.md`](https://github.com/esp-rs/esp-hal/blob/main/esp-hal/CHANGELOG.md) in the **_proper_** section.
- [ ] I have added necessary changes to user code to the [Migration Guide](https://github.com/esp-rs/esp-hal/blob/main/esp-hal/MIGRATING-0.21.md).
- [x] My changes are in accordance to the [esp-rs developer guidelines](https://github.com/esp-rs/esp-hal/blob/main/documentation/DEVELOPER-GUIDELINES.md)

#### Extra:
- [x] I have read the [CONTRIBUTING.md guide](https://github.com/esp-rs/esp-hal/blob/main/documentation/CONTRIBUTING.md) and followed its instructions.

### Pull Request Details 📖

#### Description
Closes #2787 

#### Testing
CI


Patch:
diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md
index 6cdb3660..29c37deb 100644
--- a/esp-hal/CHANGELOG.md
+++ b/esp-hal/CHANGELOG.md
@@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix PCNT counter not keeping the peripheral enabled (#3334)
 - Fixed an issue where inverting a pin via the interconnect matrix was ineffective (#3312)
 - The half-duplex SPI APIs should accept more valid line width combinations (#3325)
+- Passing an invalid seven bit I2C address is now rejected (#3343)
 - PARL_IO: Use correct max transfer size (#3346)
 
 ### Removed
diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs
index bae8a930..05357a27 100644
--- a/esp-hal/src/i2c/master/mod.rs
+++ b/esp-hal/src/i2c/master/mod.rs
@@ -79,17 +79,33 @@ const MAX_ITERATIONS: u32 = 1_000_000;
 pub enum I2cAddress {
     /// 7-bit address mode type.
     ///
-    /// Note that 7-bit addresses defined by drivers should be specified in
-    /// **right-aligned** form, e.g. in the range `0x00..=0x7F`.
+    /// Note that 7-bit addresses are specified in **right-aligned** form, e.g.
+    /// in the range `0x00..=0x7F`.
     ///
     /// For example, a device that has the seven bit address of `0b011_0010`,
     /// and therefore is addressed on the wire using:
     ///
     /// * `0b0110010_0` or `0x64` for *writes*
     /// * `0b0110010_1` or `0x65` for *reads*
+    ///
+    /// The above address is specified as 0b0011_0010 or 0x32, NOT 0x64 or 0x65.
     SevenBit(u8),
 }
 
+impl I2cAddress {
+    fn validate(&self) -> Result<(), Error> {
+        match self {
+            I2cAddress::SevenBit(addr) => {
+                if *addr > 0x7F {
+                    return Err(Error::AddressInvalid(*self));
+                }
+            }
+        }
+
+        Ok(())
+    }
+}
+
 impl From<u8> for I2cAddress {
     fn from(value: u8) -> Self {
         I2cAddress::SevenBit(value)
@@ -168,6 +184,8 @@ pub enum Error {
     CommandNumberExceeded,
     /// Zero length read or write operation.
     ZeroLengthInvalid,
+    /// The given address is invalid.
+    AddressInvalid(I2cAddress),
 }
 
 /// I2C no acknowledge error reason.
@@ -231,6 +249,9 @@ impl core::fmt::Display for Error {
                 write!(f, "The number of commands issued exceeded the limit")
             }
             Error::ZeroLengthInvalid => write!(f, "Zero length read or write operation"),
+            Error::AddressInvalid(address) => {
+                write!(f, "The given address ({:?}) is invalid", address)
+            }
         }
     }
 }
@@ -794,6 +815,8 @@ impl<'d> I2c<'d, Async> {
         address: I2cAddress,
         operations: impl Iterator<Item = Operation<'a>>,
     ) -> Result<(), Error> {
+        address.validate()?;
+
         let mut last_op: Option<OpKind> = None;
         // filter out 0 length read operations
         let mut op_iter = operations
@@ -866,6 +889,8 @@ where
         address: I2cAddress,
         operations: impl Iterator<Item = Operation<'a>>,
     ) -> Result<(), Error> {
+        address.validate()?;
+
         let mut last_op: Option<OpKind> = None;
         // filter out 0 length read operations
         let mut op_iter = operations
@@ -2211,6 +2236,7 @@ impl Driver<'_> {
         bytes: &[u8],
         start: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.reset_fifo();
         self.reset_command_list();
         let cmd_iterator = &mut self.regs().comd_iter();
@@ -2244,6 +2270,7 @@ impl Driver<'_> {
         start: bool,
         will_continue: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.reset_fifo();
         self.reset_command_list();
 
@@ -2275,6 +2302,7 @@ impl Driver<'_> {
         start: bool,
         stop: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.clear_all_interrupts();
 
         // Short circuit for zero length writes without start or end as that would be an
@@ -2311,6 +2339,7 @@ impl Driver<'_> {
         stop: bool,
         will_continue: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.clear_all_interrupts();
 
         // Short circuit for zero length reads as that would be an invalid operation
@@ -2364,6 +2393,7 @@ impl Driver<'_> {
         start: bool,
         stop: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.clear_all_interrupts();
 
         // Short circuit for zero length writes without start or end as that would be an
@@ -2400,6 +2430,7 @@ impl Driver<'_> {
         stop: bool,
         will_continue: bool,
     ) -> Result<(), Error> {
+        address.validate()?;
         self.clear_all_interrupts();
 
         // Short circuit for zero length reads as that would be an invalid operation


Test patch:
diff --git a/hil-test/tests/i2c.rs b/hil-test/tests/i2c.rs
index 032ef041..c6fc32fa 100644
--- a/hil-test/tests/i2c.rs
+++ b/hil-test/tests/i2c.rs
@@ -7,7 +7,7 @@
 #![no_main]
 
 use esp_hal::{
-    i2c::master::{AcknowledgeCheckFailedReason, Config, Error, I2c, Operation},
+    i2c::master::{AcknowledgeCheckFailedReason, Config, Error, I2c, I2cAddress, Operation},
     Async,
     Blocking,
 };
@@ -49,6 +49,22 @@ mod tests {
         Context { i2c }
     }
 
+    #[test]
+    fn invalid_address_returns_error(mut ctx: Context) {
+        assert_eq!(
+            ctx.i2c.write(0x80, &[]),
+            Err(Error::AddressInvalid(I2cAddress::SevenBit(0x80)))
+        );
+        assert_eq!(
+            ctx.i2c.read(0x80, &mut [0; 1]),
+            Err(Error::AddressInvalid(I2cAddress::SevenBit(0x80)))
+        );
+        assert_eq!(
+            ctx.i2c.write_read(0x80, &[0x77], &mut [0; 1]),
+            Err(Error::AddressInvalid(I2cAddress::SevenBit(0x80)))
+        );
+    }
+
     #[test]
     fn empty_write_returns_ack_error_for_unknown_address(mut ctx: Context) {
         // on some chips we can determine the ack-check-failed reason but not on all