I2cBus

[krwq]

Some sensors use multiple I2C devices. Example of that is SenseHat. Currently in order to swap I2C implementation with different one (i.e. Ft4222) it's a bit of a hassle: device need to take 6 I2C Devices as inputs and construct all of them. If we had I2cBus this could be solved much simpler:

public abstract class I2cBus
{
  public abstract I2cDevice CreateDevice(int deviceAddress);
  public static I2cBus Create(int bus)
  {
    return new DefaultI2cBus(bus);
  }
}

internal class DefaultI2cBus : I2cBus
{
  private int _bus;

  public DefaultI2cBus(int bus)
  {
    _bus = bus;
  }

  public override I2cDevice CreateDevice(int deviceAddress)
  {
    return I2cDevice.Create(new I2cConnectionSettings(bus, deviceAddress));
  }
}

in that case sense hat could take bus as an input (and null as default which would use the correct bus). The Ft4222 could implement a bus and could be provided directly to SenseHat.

• open question: should bus manage lifetime of devices or just be a factory (factory would be fine for most of the cases)

[Ellerbach]

This is similar but kind of reverse way of the GpioController and GpioPin discussion :-) In the I2C case, we have the equivalent of the GpioPin and we are missing the equivalent of the GpioController. In all cases, we need both, they have their usage. So I'm very much in favor of having an I2cBus as described. You'll most likely need a bit more:

public override void RemoveDevice(int deviceAddress)

[shaggygi]

Somewhat related #214

[krwq]

@shaggygi actually wouldn't be really bad idea to add Write/Read/WriteRead to I2cBus with the device address to allow for direct writing to the device. For things like I2C scanner this would remove overhead of having to have create instances and also make it easier to implement those few weird devices. It would also be on pair with GpioController and potential GpioPin we add later

[Ellerbach]

+1

[pgrawehr]

I've also thought of this before, but IIRC it was considered to much overhead. But I agree that an i2cbus instance would be helpful. It could provide functions such as bus scanning or also bus reset (send a 0x6 to addres 0x0 I think that was).

[joperezr]

[Triage] This is approved and we are ready for a PR. We will want the bus to handle device management.

[pgrawehr]

@joperezr Should - not considering possible breaking changes - devices only be created over the bus, or should there stay both variants?

[Ellerbach]

@pgrawehr I think we should keep the I2cDevice as the one to pass to a binding. No change from this point of view. I2cBus is extra where you can create I2cDevice from it. That will allow something like the FT4222 and other elements like this to be able to easily create multiple I2cDevice in a consistent way. And for couple of complicated scenarios to have multiple devices at the same time. But only for complicated scenarios that can't be addressed by I2cDevice only.

[pgrawehr]

@Ellerbach That part seems clear and I understand. I was merely asking about the creation step. Should one need to always create a I2cBus instance first and a device from that? Or would we keep Board.CreateI2cDevice or the static I2cDevice.Create methods or deprecate them?

[Ellerbach]

Or would we keep Board.CreateI2cDevice or the static I2cDevice.Create methods or deprecate them?

The impact of changing is not on the binding side as no binding is creating the I2cDevice. At least I haven't seen any doing that. So if we deprecate for the board abstraction or the I2cBus or anything like this impact will be very minimal. And on the end user applicative code, will be a matter of 2 or 3 lines maximum. So for me, all options are open!

[krwq]

@pgrawehr for devices needing just a single I2cDevice I'd suggest taking just I2cDevice, I2cBus can be considered as a convenience so that address doesn't have to provided (perhaps even auto-creating bus if null is provided) but the main scenario should remain providing I2cDevice. For devices having multiple devices we should consider recommend API taking multiple I2cDevices (but not required; i.e. for SenseHat it doesn't add much value since you can't change address) and I2cBus overload should be a must have in such cases.

[krwq]

Would we want public void RemoveDevice(int deviceAddress) or public void RemoveDevice(I2cDevice i2cDevice) ?

I'm personally in favor of the latter although first option is on pair with GpioController so perhaps we could do it this way just because of that.

[krwq]

also I'm not sure that doing validation is such a good idea. I just found out that i.e. SenseHat is using same address for matrix and joystick... arguably you could share same device there but I'm not 100% that's a good idea (i.e. led matrix can also be controlled by native driver using sysfs while joystick still needs I2C to be constructed)... perhaps when creating a device there should be a bool flag telling it if it should be not tracked by the bus?

[pgrawehr]

@krwq I added a proposal to #1128. It does not do any validation on the device level, just on the bus level (so you cannot use the same pins for I2c and Gpio for instance).

If we go this way, I suppose we should also have an SpiBus?

[pgrawehr]

Oh, I guess we just had similar ideas... But they're not so much interfering: Yours is a concrete example, mine the base infrastructure.

[Ellerbach]

public void RemoveDevice(int deviceAddress)

This one. As you create it with the deviceAddress, it's more logical in this point of view.

[krwq]

@pgrawehr yes, I2cDeviceManager seems the same as I2cBus. I2cBus we already had a general agreement so let's go with this design. I'll see if I have time to write this PR (might need to wait until next week as I'll likely be busy this week). The PR showcases it in Ft4222 but not using the proposed class yet. There will be a bit more plumbing in the System.Device to get that to work fully so I prefer to do it separately

[pgrawehr]

@krwq I've implemented basically that design, except not (yet) in System.Device. Ideally, your FT4222 showcase would, in the end, simply be changed to inherit from my base class with almost no further impact. We can hopefully talk this over directly in the triage call later this week.