I2c: attempt empty writes (#2506)

* Attempt 0-length writes

* Deduplicate

* Changelog

* Test existing address

* Name addresses

* Fix formatting
This commit is contained in:
Dániel Buga 2024-11-13 10:57:30 +01:00 committed by GitHub
parent 30276e1609
commit 8cbc249e2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 96 additions and 67 deletions

View File

@ -199,6 +199,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- TWAI should no longer panic when receiving a non-compliant frame (#2255)
- OneShotTimer: fixed `delay_nanos` behaviour (#2256)
- Fixed unsoundness around `Efuse` (#2259)
- Empty I2C writes to unknown addresses now correctly fail with `AckCheckFailed`. (#2506)
### Removed

View File

@ -1855,6 +1855,70 @@ impl Driver<'_> {
.write(|w| w.rxfifo_full().clear_bit_by_one());
}
fn start_write_operation(
&self,
address: u8,
bytes: &[u8],
start: bool,
stop: bool,
) -> Result<usize, Error> {
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_write(address, bytes, start, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
let index = self.fill_tx_fifo(bytes);
self.start_transmission();
Ok(index)
}
/// Executes an I2C read operation.
/// - `addr` is the address of the slave device.
/// - `buffer` is the buffer to store the read data.
/// - `start` indicates whether the operation should start by a START
/// condition and sending the address.
/// - `stop` indicates whether the operation should end with a STOP
/// condition.
/// - `will_continue` indicates whether there is another read operation
/// following this one and we should not nack the last byte.
/// - `cmd_iterator` is an iterator over the command registers.
fn start_read_operation(
&self,
address: u8,
buffer: &mut [u8],
start: bool,
stop: bool,
will_continue: bool,
) -> Result<(), Error> {
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_read(address, buffer, start, will_continue, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
self.start_transmission();
Ok(())
}
/// Executes an I2C write operation.
/// - `addr` is the address of the slave device.
/// - `bytes` is the data two be sent.
@ -1878,23 +1942,7 @@ impl Driver<'_> {
return Ok(());
}
// Reset FIFO and command list
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_write(address, bytes, start, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
let index = self.fill_tx_fifo(bytes);
self.start_transmission();
let index = self.start_write_operation(address, bytes, start, stop)?;
// Fill the FIFO with the remaining bytes:
self.write_remaining_tx_fifo_blocking(index, bytes)?;
self.wait_for_completion_blocking(!stop)?;
@ -1927,23 +1975,7 @@ impl Driver<'_> {
return Ok(());
}
// Reset FIFO and command list
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_read(address, buffer, start, will_continue, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
self.start_transmission();
self.start_read_operation(address, buffer, start, stop, will_continue)?;
self.read_all_from_fifo_blocking(buffer)?;
self.wait_for_completion_blocking(!stop)?;
Ok(())
@ -1972,22 +2004,7 @@ impl Driver<'_> {
return Ok(());
}
// Reset FIFO and command list
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_write(address, bytes, start, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
let index = self.fill_tx_fifo(bytes);
self.start_transmission();
let index = self.start_write_operation(address, bytes, start, stop)?;
// Fill the FIFO with the remaining bytes:
self.write_remaining_tx_fifo(index, bytes).await?;
self.wait_for_completion(!stop).await?;
@ -2020,19 +2037,7 @@ impl Driver<'_> {
return Ok(());
}
// Reset FIFO and command list
self.reset_fifo();
self.reset_command_list();
let cmd_iterator = &mut self.register_block().comd_iter();
if start {
add_cmd(cmd_iterator, Command::Start)?;
}
self.setup_read(address, buffer, start, will_continue, cmd_iterator)?;
add_cmd(
cmd_iterator,
if stop { Command::Stop } else { Command::End },
)?;
self.start_transmission();
self.start_read_operation(address, buffer, start, stop, will_continue)?;
self.read_all_from_fifo(buffer).await?;
self.wait_for_completion(!stop).await?;
Ok(())
@ -2067,6 +2072,9 @@ impl Driver<'_> {
start: bool,
stop: bool,
) -> Result<(), Error> {
if buffer.is_empty() {
return self.write_operation_blocking(address, &[], start, stop);
}
let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE);
for (idx, chunk) in buffer.chunks(I2C_CHUNK_SIZE).enumerate() {
self.write_operation_blocking(
@ -2110,6 +2118,9 @@ impl Driver<'_> {
start: bool,
stop: bool,
) -> Result<(), Error> {
if buffer.is_empty() {
return self.write_operation(address, &[], start, stop).await;
}
let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE);
for (idx, chunk) in buffer.chunks(I2C_CHUNK_SIZE).enumerate() {
self.write_operation(

View File

@ -6,7 +6,7 @@
#![no_main]
use esp_hal::{
i2c::master::{Config, I2c, Operation},
i2c::master::{Config, Error, I2c, Operation},
Async,
Blocking,
};
@ -24,6 +24,9 @@ fn _async_driver_is_compatible_with_blocking_ehal() {
fn _with_ehal(_: impl embedded_hal::i2c::I2c) {}
}
const DUT_ADDRESS: u8 = 0x77;
const NON_EXISTENT_ADDRESS: u8 = 0x6b;
#[cfg(test)]
#[embedded_test::tests]
mod tests {
@ -44,6 +47,16 @@ mod tests {
Context { i2c }
}
#[test]
#[timeout(3)]
fn empty_write_returns_ack_error_for_unknown_address(mut ctx: Context) {
assert_eq!(
ctx.i2c.write(NON_EXISTENT_ADDRESS, &[]),
Err(Error::AckCheckFailed)
);
assert_eq!(ctx.i2c.write(DUT_ADDRESS, &[]), Ok(()));
}
#[test]
#[timeout(3)]
fn test_read_cali(mut ctx: Context) {
@ -51,10 +64,14 @@ mod tests {
// have a failing read which might could leave the peripheral in an undesirable
// state
ctx.i2c.write_read(0x55, &[0xaa], &mut read_data).ok();
ctx.i2c
.write_read(NON_EXISTENT_ADDRESS, &[0xaa], &mut read_data)
.ok();
// do the real read which should succeed
ctx.i2c.write_read(0x77, &[0xaa], &mut read_data).ok();
ctx.i2c
.write_read(DUT_ADDRESS, &[0xaa], &mut read_data)
.ok();
assert_ne!(read_data, [0u8; 22])
}
@ -67,7 +84,7 @@ mod tests {
// do the real read which should succeed
ctx.i2c
.transaction(
0x77,
DUT_ADDRESS,
&mut [Operation::Write(&[0xaa]), Operation::Read(&mut read_data)],
)
.ok();