Add transactional I2C interface#223
Merged
Merged
Conversation
|
r? @ryankurte (rust_highfive has picked a reviewer for you, use r? to override) |
ryankurte
requested changes
Jun 18, 2020
f8c1d08 to
b22c88c
Compare
therealprof
reviewed
Jun 20, 2020
Contributor
|
Doesn't the trait need an entry in the prelude? |
Member
Author
Done. |
ryankurte
reviewed
Jun 20, 2020
ryankurte
reviewed
Jun 20, 2020
0de9610 to
bcaa268
Compare
Co-authored-by: Daniel Egger <daniel@eggers-club.de>
bcaa268 to
cfa13de
Compare
Member
Author
|
Is there anything left to do here? Since we have merged transactional SPI already, adding also transactional I2C to the same release would be nice. |
therealprof
approved these changes
Oct 30, 2020
therealprof
left a comment
Contributor
There was a problem hiding this comment.
Excellent, let's get this in then.
bors r+
Contributor
|
👎 Rejected by code reviews |
Contributor
|
... after @ryankurte removed his concern... |
ryankurte
approved these changes
Oct 30, 2020
Contributor
|
bors r+ |
Contributor
bors Bot
added a commit
to rust-embedded/linux-embedded-hal
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 <eldruin@gmail.com>
bors Bot
added a commit
to rust-embedded/linux-embedded-hal
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 <eldruin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
writeoperations 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. 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:
I added a PoC here in linux-embedded-hal including an example of a driver where a classical combined write/read is performed through the transactional interface.
Note: A default implementation of the
Transactionaltrait 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 theWrite,ReadandWriteReadtraits forTransactionalimplementers.This is based on previous work from #178 by @ryankurte and it is similar to #191
TODO: