wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 611 forks source link

Allow I2C Address To Be Changed After Creation #2297

Open carlosgj opened 4 years ago

carlosgj commented 4 years ago

Currently, the address of an I2C object must be passed at creation, and there's no way to change the address afterwards (at least in wpilibj). This makes dependency injection difficult in some libraries I'm writing to support several TCS34725 I2C color sensors attached to a TCA9548A I2C multiplexer.

I've created a class for the mux itself: public TCA9548A(Port port, int address)

and a class for a device attached to the mux:

public class TCA9548ADevice extends I2C {
    private int muxChannel; 
    private TCA9548A mux;

    public TCA9548ADevice(TCA9548A mux, int deviceAddress, int muxChannel)

(the TCA9548ADevice basically just overrides the I2C methods to first set up the mux to the desired channel, and then call the actual method.)

I'd like to make my color sensor class take an I2C object in the constructor, so that the robot code can opt to pass a TCA9548ADevice if it needs to use several color sensors behind a multiplexer. However, that's not possible if the address (which is specific to the color sensor) has to be known when the TCA9548ADevice is created. That is, I want to be able to inject the TCA9548ADevice into the color sensor, but the TCA9548ADevice needs an address at creation, and the address of the color sensor should be contained in the color sensor library.

I could get around it by allowing the creation of an "empty" color sensor object, then creating the mux device, and finally setting it to the sensor:

TCS34725 sensor = new TCS34725();
TCA9548ADevice sensorI2CDevice = new TCA9548ADevice(mux, sensor.getAddress(), muxChannel);
sensor.setI2CDevice(sensorI2CDevice);

but that doesn't seem very elegant. It would be a lot easier if I could create the I2C object with no address (or a default one), pass it to the sensor object, and then let the sensor object set the address.

PeterJohnson commented 4 years ago

This would introduce the possibility of an I2C device object being at least partially invalid. The current design philosophy across all of WPILib is for the constructor to allocate a fully-defined resource. Why can't you just create a new I2C object for the new address? E.g. use composition instead of inheritance?

carlosgj commented 4 years ago

Are you suggesting that the mux-channel object own an I2C object, rather than extending it? I could do that, but then any classes that represent I2C devices would need different methods to handle mux channels vs. direct I2C connections.

The reason I want the mux channel to inherit from I2C is so that I can create device drivers with constructors like

public TCS34725(I2C i2c)

and then allow the robot code to do either

I2C i2c = new I2C(Port.kOnboard, 0x29);
TCS34725 colorSensor = new TCS34725(i2c);

if it's a direct I2C connection, or

//Create object to represent mux
TCA9548A mux = new TCA9548A(Port.kOnboard, 0x70);

//Create object to represent device behind mux
TCA9548ADevice muxedi2c = new TCA9548ADevice(mux, 0x29, 1);
TCS34725 colorSensor = new TCS34725(muxedi2c);

if the sensor is behind a mux.

(But as mentioned above, the robot code shouldn't have to know that the address of the color sensor is 0x29; that information should be supplied by the TCS34725 driver, so that it's easy to replace the TCS34725 with a different I2C color sensor.)

In other words, the I2C sensor driver shouldn't need to know whether it's talking directly to the device, or via a mux.