Skip to content

Conversation

ryankurte
Copy link
Contributor

@ryankurte ryankurte commented Mar 10, 2020

Implementation of transactional SPI

Important: remove patch and bump hal version before merging

@ryankurte ryankurte requested a review from a team as a code owner March 10, 2020 22:18
@rust-highfive
Copy link

r? @nastevens

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

@rust-highfive rust-highfive added S-waiting-on-review Status: Review is incomplete T-embedded-linux labels Mar 10, 2020
@ryankurte ryankurte changed the title Feature/spi transactions [WIP] Feature/spi transactions Mar 10, 2020
@ryankurte ryankurte assigned posborne and unassigned nastevens Mar 10, 2020
@ryankurte ryankurte force-pushed the feature/spi-transactions branch from 04cd071 to 35ff43e Compare March 12, 2020 01:55
Requires embedded-hal patch with feature/spi-transactions branch and transactional-spi feature
@ryankurte ryankurte force-pushed the feature/spi-transactions branch from 35ff43e to 4acac31 Compare June 24, 2020 23:39
bors bot added a commit to rust-embedded/embedded-hal 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]>
@ryankurte ryankurte requested a review from eldruin November 10, 2020 02:42
Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! An update to e-h alpha.4 would be necessary, though.

bors bot added a commit that referenced this pull request Nov 12, 2020
44: Implement transactional I2C interface and add example r=ryankurte a=eldruin

I implemented the transactional I2C interface from rust-embedded/embedded-hal#223 and added an example with a driver which does a combined Write/Read transaction.
This is based on previous work from #33 by @ryankurte, rust-embedded/rust-i2cdev#51 and is similar to #35.

Co-authored-by: Diego Barrios Romero <[email protected]>
eldruin
eldruin previously approved these changes Nov 13, 2020
Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, thanks!

@ryankurte
Copy link
Contributor Author

@posborne are you able to check this plz?

bors bot added a commit that referenced this pull request Nov 14, 2020
44: Implement transactional I2C interface and add example r=ryankurte a=eldruin

I implemented the transactional I2C interface from rust-embedded/embedded-hal#223 and added an example with a driver which does a combined Write/Read transaction.
This is based on previous work from #33 by @ryankurte, rust-embedded/rust-i2cdev#51 and is similar to #35.

Co-authored-by: Diego Barrios Romero <[email protected]>
@eldruin
Copy link
Member

eldruin commented Nov 15, 2020

Ach, the build broke after the merge. Only trivial problems, though.

Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, transactional thanks!

@ryankurte ryankurte changed the title [WIP] Feature/spi transactions Add transactional SPI Nov 16, 2020
@posborne
Copy link
Member

I believe this is ready to go, but I'll let you confirm and give bors a kick @ryankurte

@ryankurte
Copy link
Contributor Author

great, thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 17, 2020

Build succeeded:

@bors bors bot merged commit 1362ed6 into rust-embedded:master Nov 17, 2020
@ryankurte ryankurte deleted the feature/spi-transactions branch November 17, 2020 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Review is incomplete T-embedded-linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants