Make I2C compatible with multiple address sizes

[eldruin]

This adds I2C 7-bit and 10-bit address mode compatibility as roughly described here.
Discussion issue: #147

I have also added the SevenBitAddress as the default address mode to the traits so this is not even a breaking change.

Usage broken down per use case:

• Device driver which only supports 7-bit addressing mode:
  The driver looks exactly the same as now since the default address mode is 7-bit.

 impl<I2C, E> MyDriver<I2C>
 where I2C: i2c::Write<Error = E> {
   pub fn do_cool_stuff(&mut self) // ...
 }

• Device driver which only supports 10-bit addressing mode:
  The only difference to a 7-bit-address-only driver is one additional parameter in the I2C trait bound.

 impl<I2C, E> MyDriver<I2C>
 where I2C: i2c::Write<TenBitAddress, Error = E> {
   pub fn do_cool_stuff(&mut self) // ...
 }

• Driver for device supporting both addressing modes:
  Complexity can be abstracted away into additional internal traits which can handle the addressing stuff. Driver code stays clean.
  This is nothing new. We already do this on drivers for devices compatible with both I2C and SPI. No need for duplicated code.
  Here a real example: usage, traits

impl<DI, E> MyDriver<DI>
where DI: WriteData<Error = E> {
  pub fn do_cool_stuff(&mut self) {} // ...
}

pub trait WriteData {
// ...
}

// it is also possible to just leave the `SevenBitAddress` type out here,
// since it is the default.
impl<I2C, E> WriteData for I2cInterface<I2C, SevenBitAddress>
where
    I2C: i2c::Write<SevenBitAddress, Error = E>,
{
  // ...
}

impl<I2C, E> WriteData for I2cInterface<I2C, TenBitAddress>
where
    I2C: i2c::Write<TenBitAddress, Error = E>,
{
  // ...
}

• Bus controller impl supporting only 7-bit addressing mode:
  Code stays almost the same, just adding one addressing mode parameter. Additionally, if desired:

  • 10-bit addressing can be software-emulated:
    Emulate by extending and copying payload in separate TenBitAddress implementation. Total flexibility to do whatever is necessary in this case since the code is independent.
  • 10-bit addressing cannot be software-emulated:
    Implementation does not offer implementation for TenBitAddress variant. The user gets a compilation error and everything is clear.

• Bus controller impl supporting both addressing modes:
  No problem. Two separate implementations guarantee as much flexibility as necessary. At the same time, sharing generic code is possible.

Additional benefits:

• No runtime performance cost
• No runtime switching, duplicated code or panics for unsupported modes.
• Consistent with what we do for code paths that can be determined statically by the compiler.
• To my taste elegant, simple and very descriptive.

See here for a comparison to other alternatives.

I have also sealed the trait.

Proof

• A HAL implementation of both modes: bitbang-hal. code changes
• Drivers supporting only 7-bit addresses need no changes.
  For demonstration purposes, explicitly including the SevenBitAddress would look like this: OPT300x. code changes.
  This would be similar to the case of a 10-bit-only device driver.

[rust-highfive]

r? @ryankurte

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

[eldruin]

I have now also added the SevenBitAddress as the default address mode to the traits so this is not even a breaking change.

[ryankurte]

seems generally good to me! the trait indirection approach is clever, though, feels a touch complex (and i have been bitten by trying to do overly clever things with the type system 🙃)

does this give us a strong benefit over providing a couple of public type aliases and documentation of the convention (pub type I2cAddrSevenBit = u8;, pub type I2cAddrTenBit = u16;)? i suppose the perk is you cannot implement the I2C interfaces over external types, with the disadvantage it makes bounds moderately more complex, and either way this must be communicated by documentation.

(i'm not opposed to this approach, but, would like to know we have considered all possibilities)

[eldruin]

does this give us a strong benefit over providing a couple of public type aliases and documentation of the convention (pub type I2cAddrSevenBit = u8;, pub type I2cAddrTenBit = u16;)?
i suppose the perk is you cannot implement the I2C interfaces over external types, with the disadvantage it makes bounds moderately more complex, and either way this must be communicated by documentation.

Yes, technically without a trait bound it would be possible to misuse the I2C traits and pass some random object through the address parameter if both HAL implementation and driver agree on a complex "address" type.

It would also be possible to do this simplification:

pub trait AddressMode: private::Sealed {}

pub type SevenBitAddress = u8;
pub type TenBitAddress = u16;

impl AddressMode for SevenBitAddress {}
impl AddressMode for TenBitAddress {}

I think this would be fine, since we probably will not need any additional type other than Address in the AddressMode trait.

Would you prefer that?

I have added some documentation now on how this would work for a HAL implementation and device drivers. I could also write some documentation about how to write a driver for a device compatible with both modes but that might be too much. Opinions?

[burrbull]

Looks good to me. 👍

[ryankurte]

It would also be possible to do this simplification:

ooh, i was under the impression you can't implement traits on type aliases? though, this may be mitigated by the trait being defined in the same place.

Would you prefer that?

hmm i think so? it seems a bit simpler (which is excellent imo), neat that the Address type is actually the type of the address (rather than a marker), and i think this will simplify/clarify the docs too. so, weak preference unless there's a good reason for the more complex approach.

[eldruin]

Would you prefer that?

hmm i think so? it seems a bit simpler (which is excellent imo), neat that the Address type is actually the type of the address (rather than a marker), and i think this will simplify/clarify the docs too. so, weak preference unless there's a good reason for the more complex approach.

Done!

[ryankurte]

i think this looks good to me, @therealprof any thoughts?

[therealprof]

Haven't had time to play with it in person but looks and sounds great.

[ryankurte]

well, let's get it out and in the alpha eh? i guess we should look to another (alpha) release shortly

bors r+

[bors]

Build succeeded:

• ci-linux (1.35.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, thumbv6m-none-eabi)