Skip to content

I2c API #9

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
japaric opened this issue Jun 9, 2017 · 5 comments
Closed

I2c API #9

japaric opened this issue Jun 9, 2017 · 5 comments
Labels

Comments

@japaric
Copy link
Member

japaric commented Jun 9, 2017

This API has not been designed yet. What methods should it provide?

@japaric japaric added the RFC label Jun 9, 2017
@therealprof
Copy link
Contributor

I2C is interesting. Since we very likely want to retain the async property of the interface, there needs to be a way to model master and slave operation and hence the ability to either initiate and keep a new "session" with a slave having a specific address (in the simple case, there're also variants where group addressing is possible) where the operation can be read, write or combined. Or in the slave case the device only has a specific address and needs to respond to the read or write operations initiated by the master.

@ahixon
Copy link

ahixon commented Jan 9, 2018

Meant to post this a while back, but just got reminded by 5415ee1

Anyway, just some feedback on how I ended up doing I2C. I had something like:

pub trait I2CTrait {
    fn read_from(&self, address: u8, register: Option<u8>, &mut [u8]) -> Result<usize>;
    fn write_to(&self, address: u8, register: Option<u8>, &[u8]) -> Result<usize>;
}

which was only a first pass -- had intended to make it more abstract and post it here, but that didn't end up happening I guess haha. However, it did support one operation that I think most people would need:

Slave register addressing

Selecting a target register on a slave device to read or write from is a very common operation, so it might be worthwhile including some API at the embedded-hal level to do that easily.

Reading or writing from a slave's register is done by first writing the slave's address followed by the register address on the bus, then writing the slave's address again and entering read or write as normal. The sequence of the START and STOP conditions are different, though (see below).

Blocking API

I'll use write_to as an example, but the same applies to read_from.

One option could be to compose a write_to-like function in this crate, using a variant of the read and write traits as building blocks.

However(!), the controller should not send a STOP followed by another START, but should instead send a repeated START on the bus between the initial write describing the register, and the second read/write transaction.

That means the current read and write traits cannot be used as-is, because they currently provide a contract to send a STOP once they've finished.

So, for that option to work, additional traits that don't send the STOP condition or something would need to be introduced. However, I'm not sure if all controllers allow bit-banging the I2C bus in that way, or if we'd even want to expose that?

Alternatively, the device could choose to implement its own write_to.

For example, the controller I was targeting, for example, provided a special mode that would generate a START, write the slave address, followed by the target register, another START, re-send the slave address, and then jump into RX mode automatically. Error handling interrupts for that mode were the same as the normal read and write cases.

Being able to implement write_to may also provide some speed improvements for controllers that support a similar mode.

Target register word size

What type do we make target, then?

From a quick scan, the I2C spec doesn't describe the "target register" word size at all. Though, this makes good sense IMHO because you want a generic specification. Anyway, from "Fig 13. Combined Format" page 15, it looks like it is permitted to be n bytes.

So, then, I guess the most generic write_to trait could take an array of u8 to describe the register.

Most simple slave ICs use 8-bit addressing, though I've seen some use 16-bits, and rarely, 32-bits (e.g. I2C-compatible audio codecs that have large register files). So, maybe a short-hand version taking a single u8 or something might be useful. Don't know.

You'd probably still want to support the n-byte case anyway though, since I'm sure there are devices somewhere out there that use multiple bytes for something.

A personal anecdote: this controller supports up to 24-bit addresses fine, but anything more than that I think I'd have to bit-bang the data bytes myself. Haven't looked at any others, but I would hope that other controllers permit the same. I only mention this so we don't accidentally end up with an API that not all devices can implement!

Anyway, just my 2c. :)

@ahixon
Copy link

ahixon commented Jan 9, 2018

I'm an idiot and can't read, apparently. You've already got a write_read trait that does exactly that.

Oops!

@japaric
Copy link
Member Author

japaric commented Jan 11, 2018

So, for that option to work, additional traits that don't send the STOP condition

I thought about doing something like that but I couldn't think of a way to enforce at compile time that the user won't perform a string of operations that don't contain a stop -- this would result in keeping the bus busy beyond the operations the user requested.

Target register word size

The I2C API in #27 doesn't have a separate address argument. Instead the adress is assumed to be part of the buffer to send to the device. This way the user can enter an address of arbitrary size at the start of the write buffer. The other reason I went with this approach is that (I suspect that) if you use the I2C with the DMA (haven't tried that yet) you'll also have the register address as part of the DMA buffer so a future I2C+DMA API will probably look like the current API (no extra address argument), I expect.

@japaric
Copy link
Member Author

japaric commented Jan 20, 2018

The blocking::i2c::{Read,Write,WriteRead} traits are now part of a crates.io release so I'm going to close this. If you encounter any problem implementing / using those traits feel free to open a new issue!

If you want to propose adding futures or generator based I2C traits you should open a new RFC issue.

@japaric japaric closed this as completed Jan 20, 2018
peckpeck pushed a commit to peckpeck/embedded-hal that referenced this issue Nov 10, 2022
12: Release new Patch on crates.io with InputPin Support r=therealprof a=Caemor

Since rust-embedded#9 still seems to be WIP a small update for crates.io would be nice, so I could move back to the crates.io version of this package.

Co-authored-by: Chris <[email protected]>
Co-authored-by: Chris <[email protected]>
peckpeck pushed a commit to peckpeck/embedded-hal that referenced this issue Nov 10, 2022
9: Add serial::Read/Write implementation r=ryankurte a=rnestler

So this is just a quick PoC to implement rust-embedded#8.
@japaric Do you think it would be OK to implement it in that way? If yes I can finish it up.

Co-authored-by: Raphael Nestler <[email protected]>
Co-authored-by: Raphael Nestler <[email protected]>
Co-authored-by: Danilo Bargen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants