I2c: Inherent transaction function, lift size limits #2262
Conversation
| #[cfg(any(esp32, esp32s2))] | ||
| const I2C_CHUNK_SIZE: usize = 32; | ||
|
|
||
| #[cfg(not(any(esp32, esp32s2)))] |
There was a problem hiding this comment.
technically not necessary but would be a subtle behavioral change for chips other than ESP32/ESP32-S2 otherwise
esp-hal/src/i2c.rs
Outdated
| // filter out 0 length read operations | ||
| let mut op_iter = operations | ||
| .iter_mut() | ||
| .filter(|op| if op.is_write() { true } else { !op.is_empty() }) |
There was a problem hiding this comment.
| .filter(|op| if op.is_write() { true } else { !op.is_empty() }) | |
| .filter(|op| op.is_write() || !op.is_empty()) |
esp-hal/src/i2c.rs
Outdated
| matches!(self, Operation::Read(_)) | ||
| } | ||
|
|
||
| fn write_buffer(&self) -> Option<&'a [u8]> { |
There was a problem hiding this comment.
See above, I'm not too fond of these. They make sense in public code, but these aren't intended to be public as far as I can tell.
There was a problem hiding this comment.
me too - but see above
esp-hal/src/i2c.rs
Outdated
| pub fn transaction<'a>( | ||
| &mut self, | ||
| address: u8, | ||
| operations: &mut [impl I2cOperation<'a>], |
There was a problem hiding this comment.
Could this just take an IntoIterator? This method doesn't seem to need a slice.
| /// Transaction contract: | ||
| /// - Before executing the first operation an ST is sent automatically. This | ||
| /// is followed by SAD+R/W as appropriate. | ||
| /// - Data from adjacent operations of the same type are sent after each | ||
| /// other without an SP or SR. | ||
| /// - Between adjacent operations of a different type an SR and SAD+R/W is | ||
| /// sent. | ||
| /// - After executing the last operation an SP is sent automatically. | ||
| /// - If the last operation is a `Read` the master does not send an | ||
| /// acknowledge for the last byte. | ||
| /// | ||
| /// - `ST` = start condition | ||
| /// - `SAD+R/W` = slave address followed by bit 1 to indicate reading or 0 | ||
| /// to indicate writing | ||
| /// - `SR` = repeated start condition | ||
| /// - `SP` = stop condition |
There was a problem hiding this comment.
In future, I hope we can make this contract optional as well and allow users to do this themselves.
I personally need a non e-hal conforming contract for the SCCB protocol.
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 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
transactionread,write,write_read(fixes I2C:write,write_readandreadare unnecessarily limiting #2174)Testing
write/write_readfunctions