Created a first draft of the I3C trait.#724
Created a first draft of the I3C trait.#724Tremoneck wants to merge 5 commits intorust-embedded:masterfrom
Conversation
There are still things missing in order of priority: Common Command Codes Dynamic Address Assignment Interupts Second Master Hot Join
|
Ping @felipebalbi for discussion |
| type Error = T::Error; | ||
| } | ||
|
|
||
| pub trait I3c<S: SpeedMode = Sdr>: ErrorType { |
There was a problem hiding this comment.
Not sure I agree with the Speed mode typestate here. We can switch between DDR and SDR in runtime, right? Will you require folks to consume and recreate the I3C (presumably with helpers into_sdr() and into_ddr())?
There was a problem hiding this comment.
I thought about the controller implementing all supported speeds for the same object. The client can then use the different supported speeds depending on which trait is used to call transaction. In this design the whole transaction has the same speed. The switch from i2c to i3c would happen after the first address byte has been sent and arbitration is over.
There was a problem hiding this comment.
Apparently my idea on how I would implement this didn't work out or I'm missing on how to constrain the types properly. As can be seen in the test. Ideas on another Idea. Different traits for the different speeds could work, but they'd be unwieldy.
There was a problem hiding this comment.
Yeah, it'll be hard to come up with something sensible. But I think a single trait that lets you do a transaction and the transaction knows if it's supposed to be I2c, I3cSdr or I3cDdr would be simpler. Then, anything implementing I3c gets an I2c implementation for free by calling I3c methods with all transactions set to I2c.
There was a problem hiding this comment.
My idea behin the typestates was to ensure that a user is never able to combine a slave and controller, where the speed is not suported by both.
This enables us to explicitly specify the point at which High Data Rate is enabled and disabled.
Added documentation with an example about how the trait should be properly used.
|
Please follow https://github.com/rust-embedded/embedded-hal/blob/master/docs/how-to-add-a-new-trait.md before undrafting this for review. |
3 participants
Idea to close #586
There are still things missing in order of priority, though we could probably close at least the features in additional PRs
Common Command Codes
Dynamic Address Assignment
Interrupts
A lot of documentation
The other two high speed modes
Second Master
Hot Join