[Discussion] Non-blocking I2C Trait

[austinglaser]

It's unfortunate that there's no nonblocking variant of the i2c HAL traits -- but that could easily change.

My first impression is that it should look something like:

//! Inter-integrated Circuit Interface

use nb;

/// Possible I2C errors
pub enum Error<E> {
    /// An implementation-specific error ocurred
    Other(E),

    /// Another master owns the bus
    ArbitrationLost,

    /// The slave did not acknowledge an address or data byte
    ByteNacked,

    /// An operation was attempted at an invalid time
    ///
    /// For example, an attempt to `send()` a byte before `send_address()` is called will fail with
    /// this error
    InvalidOperation,

    #[doc(hidden)]
    _Extensible,
}

/// Read or write access to the peripheral
pub enum AccessType {
    Read,
    Write,
}

pub trait Master {
    /// An enumeration of implementation-specific I2C errors
    type ImplError;

    /// Drive a start condition on the bus
    ///
    /// Returns an `Error` if the system is in an invalid state.
    ///
    /// May be used to generate a repeated start condition -- however, at minimum `send_address()`
    /// must be called between the start conditions.
    ///
    /// TODO: Does I2C require at least byte transferred before the repeated start? This is by far
    /// the most common use-case
    fn send_start(&mut self) -> nb::Result<(), Error<Self::ImplError>>;

    /// Drive a start condition on the bus
    ///
    /// Returns an `Error` if the system is in an invalid state
    fn send_stop(&mut self) -> nb::Result<(), Error<Self::ImplError>>;

    /// Send a peripheral address
    ///
    /// Must be the first operation after generating a start condition.
    fn send_address(
        &mut self,
        address: u8,
        access: AccessType,
    ) -> nb::Result<(), Error<Self::ImplError>>;

    /// Send a byte to the peripheral
    ///
    /// May be called several times after sending an address with `AccessType::Write`. May not be
    /// called after sending an address with `AccessType::Read`
    fn send(&mut self, byte: u8) -> nb::Result<(), Error<Self::ImplError>>;

    /// Send a byte to the peripheral
    ///
    /// May be called several times after sending an address with `AccessType::Read`. May not be
    /// called after sending an address with `AccessType::Write`
    fn read(&mut self, send_ack: bool) -> nb::Result<u8, Error<Self::ImplError>>;
}

If this is stabilized, there could be default implementations of blocking::i2c in terms of it, simplifying driver writers' lives.

Alternatives

The nonblocking trait could operate at a higher-level (basically just mirroring the traits exposed by blocking::i2c except with a nb::Result. The above trait has lots of potential for runtime errors, which may be undesirable.

[Kixunil]

I'd prefer statically checking validity of operation. Like this:

pub trait Send {
    type Error;

    fn send(&mut self, byte: u8) -> nb::Result<(), Error<Self::Error>>;
}

pub trait Receive {
    type Error;

    fn receive(&mut self, send_ack: bool) -> nb::Result<u8, Error<Self::Error>>;
}

pub trait Master {
    type Sender: Send;
    type Receiver: Receive;
    type Error;

    fn send_start(&mut self, address: u8) -> nb::Result<&mut Self::Sender, Error<Self::Error>>;
    fn receive_start(&mut self, address: u8) -> nb::Result<&mut Self::Receiver, Error<Self::Error>>;
}

[eisterman]

Are there any developments on the subject or has it been decided not to build a shared public interface for the non-blocking i2c?

[jounathaen]

I'm not very experienced with this, but I'm wondering, why there is such a major distinction between this at all and why blocking is the default here.
I'd assume, that most μCs have special hardware for I2C, which makes non-blocking the more natural choice. I also think, that a non-blocking implementation can easily be changed into a blocking one.

Idea

So my naive idea of an I2C trait would be something like this:
We have a module embedded_hal::i2c which provides the following traits:

• read/write (or send/receive) might block or not, depending on the implementation. The only guarantee is, that the transaction was started. This function might block when there is an incomplete previous transmission "in the pipe".
• blocking_read/blocking_write guarantee that the transaction is complete when the function returns.

A HAL implementation could then for example implement a non-blocking variant and implement the blocking variant by using busy-waiting or - if there can only be a blocking implementation - use the same function for both of it. I seems like this is the way stm32f1xx_hal implements it.

Driver implementations would then be able to choose the according function for their implementation and save some (precious) μC cycles which are currently wasted by busy-waiting.

Problems

This would be a breaking change. But don't we use semantic versioning for that very reason?

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable

[therealprof]

I'm not very experienced with this, but I'm wondering, why there is such a major distinction between this at all and why blocking is the default here.

Because it's much easier to implement.

I'd assume, that most μCs have special hardware for I2C, which makes non-blocking the more natural choice. I also think, that a non-blocking implementation can easily be changed into a blocking one.

There're different kinds of "non-blocking". There's the kind where you basically just setup the data, point the peripheral to it and say do it and then just check whether the operation is done (i.e. poll the status) or receive a notification (i.e. an interrupt) that's how DMA works.

Problem: You may or may not be able to set up multiple operations in flight so you have to be able to reject a new transaction until the previous one has finished. And the caller needs to be able to deal with that situation.

Then there's a "software" implementation of the above. Problem here is: You need to have an internal buffer to handle store the in-flight data and you need to peridiocally poll the implementation (or use interrupts which really can not be done in a generic fashion internally) to make progress by spoon-feeding the next piece of data into the peripheral.

Problems: Buffers are complicated to implement, especially on lowlier MCUs without atomic operations and sizing is always going to be a big fun. Also you usually cannot stretch the clock forever so you need to poll frequently to make progress.

It's tricky to model an interface that works for both cases, and allows for (and actually requires) regular polling to check the status or make progress plus of course the application using that interface needs to be able to handle with errors which happen asynchronously.

[jounathaen]

Thanks for the good summary 👍 .

However, I still think, that a generic read/write trait that doesn't give specific guarantees would be nice and in addition to this a set of specialized traits (blocking_read, read_to_buffer, something with async/await...) for those drivers that require more fine grained control.

At the moment, every driver implementation requires the I2C object to implement embedded_hal::blocking::i2c, even though they might just send a single byte to a sensor.

[therealprof]

At the moment, every driver implementation requires the I2C object to implement embedded_hal::blocking::i2c

Not sure I follow. When you say "driver", are you talking about the HAL impls? We're usally using the word driver for crate that takes an impl of the generic traits and converts them into a an API usable by applications. Of course the HAL impl needs to implement the whole interface at once which is why it has to be simple and generic.

even though they might just send a single byte to a sensor.

This does not really work for I2C because you need to deal with the addressing and the conditions, which the original proposal in this issue did by exposing a low level interface.

[jounathaen]

This does not really work for I2C because you need to deal with the addressing and the conditions,
which the original proposal in this issue did by exposing a low level interface.

I think, we are talking about a different level of asynchronicity here.
I'm only talking about not (busy-)waiting for the acknowledgement so that this time can be used for computation.

Indeed, this is not really nonblocking, because the software would still have to block when there is an ongoing I2C transfer and you want to initialize a second transfer. But it would at least speed up patterns like I2C transfer - computation - I2C transfer - computation, especially if utilizing DMA hardware.

In contrast, I understood the original proposal like you need a FIFO and a distinct thread that starts the next transfer as soon as the previous one has finished.
This sounds a lot like it requires async/await and I think, this a bit too high-level for the embedded-hal and would better fit in a specialized crate. But in that case, do you really need to control the start/stop bits from a driver point of view? I'd have guessed the current abstractions (like write(&mut self, addr: u8, bytes: &[u8])) are sufficient.

Not sure I follow. When you say "driver", are you talking about the HAL impls? We're usally using the word driver for crate that takes an impl of the generic traits and converts them into a an API usable by applications.

That is what I meant by driver, too.

[therealprof]

I think, we are talking about a different level of asynchronicity here.

I don't think so.

I'm only talking about not (busy-)waiting for the acknowledgement so that this time can be used for computation.

Sure. Depending on how you do your I2C communication you will still be able to do other computation.

Indeed, this is not really nonblocking, because the software would still have to block when there is an ongoing I2C transfer and you want to initialize a second transfer. But it would at least speed up patterns like I2C transfer - computation - I2C transfer - computation, especially if utilizing DMA hardware.

It could be truly non-blocking in this case, but somehow you need to manage the data and DMA accesses and that is simply not easy to abstract over generically.

In contrast, I understood the original proposal like you need a FIFO and a distinct thread that starts the next transfer as soon as the previous one has finished.

You absolutely need an software arbiter taking care of the bus accesses and feeding the data into the peripheral, just as I mentioned before. A separate task is a way of doing that and indeed how most embedded OSes do that but it's not really applicable here.

There might easy specialized implementations with a trivial interface for some MCUs but again we're looking for a universally useful abstraction here.

That is what I meant by driver, too.

Please note that there's literally no use in having a super driver that efficiently uses special traits for which there's exactly the two mandatory proving implementations. You might as well create and use a MCU specific optimized implementation of that driver then.

I'm convinced there's a way to provide so an interface which is easy enough to use and can be universally implemented but it will require quite a bit of work and trial-and-error to come up with something that is acceptable for inclusion here.

Please feel free to take on this task and come up a concrete proposal for such an interface. If you need help or testers, feel free to ping us on matrix or here and I'm sure there're plenty of people glad to help and figure this out with you.

[PvdBerg1998]

Would replacing all busy_wait loops with an async suspension point fix the issue? I don't think you need any buffering as the future would require unique (&mut) access to the I2C struct, and the state is encoded by the async/await transform.

[tgross35]

Will this still be relevant with the embedded-hal-async crate? https://docs.rs/embedded-hal-async/0.1.0-alpha.3/embedded_hal_async/i2c/index.html It seems like this removes the need for anything nb-based