Skip to content

Add transactional traits for SPI and I2C #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ryankurte
Copy link
Contributor

@ryankurte ryankurte commented Jan 9, 2020

@rust-highfive
Copy link

r? @thejpster

(rust_highfive has picked a reviewer for you, use r? to override)

@ryankurte ryankurte changed the title [WIP] add transactional traits for SPI and I2C Add transactional traits for SPI and I2C Jan 21, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
@ryankurte
Copy link
Contributor Author

Closing out in favour of #191 for now

@ryankurte ryankurte closed this Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
@eldruin eldruin mentioned this pull request Jun 17, 2020
1 task
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 24, 2020
bors bot added a commit that referenced this pull request Oct 28, 2020
191: Added transactional SPI interface r=therealprof a=ryankurte

This PR adds a transactional interface for SPI devices (#94), compatible with linux spidev.

Split from #178 as I believe this is complete and useful, but that there is more experimentation required before (if?) the I2C component is landed, check there for previous reviews / discussion.

**Demonstrated in:**
- Linux embedded hal: rust-embedded/linux-embedded-hal#35
- STM32F4xx-hal: stm32-rs/stm32f4xx-hal#167
- embedded-spi driver abstraction (previously provided a polyfill for equivalent transactional functionality) https://github.com/ryankurte/rust-embedded-spi/pull/4/files#diff-74eea42f4e5e15399ac9184c8f2727a9R344
- sx128x radio driver: rust-iot/rust-radio-sx128x#5


**Notes:**
- `Operation::Transfer` uses one buffer to allow polyfill using the existing `Transfer` trait (with the convenient side effect of reducing memory requirements)
- `W` has a static bound as it _should_ only ever be a type with static lifetime (u8, u16 etc., not a reference), and to differentiate this from `'a` which is the lifetime of the data in the object and only bound to the function
- `exec(.., &mut [Operation])` is chosen over `exec<O: AsMut<[Operation]>(..)` as the latter imposes limits on generic types using this trait (which i ran into, see [E0038](https://doc.rust-lang.org/error-index.html#E0038))

cc. @rust-embedded/hal folks, @eldruin, @RandomInsano, @Rahix, @austinglaser for opinions / review

Co-authored-by: Ryan Kurte <[email protected]>
Co-authored-by: ryan <[email protected]>
bors bot added a commit that referenced this pull request Oct 30, 2020
223: Add transactional I2C interface r=therealprof a=eldruin

An example where a transactional I2C interface would be an improvement is when facing the problem of sending an array of data where the first item is the destination address.
At the moment this requires copying the data into a bigger array and assigning the first item to the address. With a transactional I2C two `write` operations could be chained into one transaction so that it is possible to send the address and data without an extra copy.
This is specially problematic if the data length is unknown e.g. because it comes from the user.

You can see the problem [here in the eeprom24x driver](https://github.com/eldruin/eeprom24x-rs/blob/f75770c6fc6dd8d591e3695908d5ef4ce8642566/src/eeprom24x.rs#L220). In this case I am lucky enough to have the page upper limit so that the copy workaround is possible.

With this PR a bunch of code and macros could be replaced by doing something similar to this:
```rust
let user_data = [0x12, 0x34, ...]; // defined somewhere else. length may be unknown.
let target_address = 0xAB;
let mut ops = [
  Operation::Write(&[target_address]),
  Operation::Write(&user_data),
];
i2cdev.try_exec(DEV_ADDR, &ops)
```

I added a PoC [here in linux-embedded-hal](https://github.com/eldruin/linux-embedded-hal/blob/7512dbcc09faa58344a5058baab8826a0e0d0160/src/lib.rs#L211) including [an example](https://github.com/eldruin/linux-embedded-hal/blob/transactional-i2c/examples/transactional-i2c.rs) of a driver where a classical combined write/read is performed through the transactional interface.

Note: A default implementation of the `Transactional` trait like in #191 is not possible because STOPs would always be sent after each operation. What is possible is to do is the other way around. This includes an implementation of the `Write`, `Read` and `WriteRead` traits for `Transactional` implementers.

This is based on previous work from #178 by @ryankurte and it is similar to #191 

TODO:
- [x] Add changelog entry

Co-authored-by: Diego Barrios Romero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants