androidthings / contrib-drivers

Open source peripheral drivers
Apache License 2.0
560 stars 174 forks source link

cap12xx - support for setting i2c address #111

Open Gadgetoid opened 5 years ago

Gadgetoid commented 5 years ago

While sitting down with @proppy yesterday and getting something of a crash course in building drivers for Things, we ran into an omission in the cap1xxx library.

The cap1xxx range support at least 5 different i2c address that I'm aware of, via the use of a pull-down address select resistor:

Resistor Address
82k 0x2c
100k 0x2b
120k 0x2a
150k 0x29
VDD 0x28

The hard-coded address needs substituting with an optional constructor for supplying one of the above resistor-selected values:

https://github.com/androidthings/contrib-drivers/blob/69d9317e6222f334afbfc62420dd87e399bb2988/cap12xx/src/main/java/com/google/android/things/contrib/driver/cap12xx/Cap12xx.java#L231

I have local code working with alternate i2c addresses (Pimoroni Drum HAT uses 0x2c) which looks like this:

    public Cap12xx(String i2cName, String alertName, Configuration chip, Handler handler) throws IOException {
        this(i2cName, I2C_ADDRESS, alertName, chip, handler);
    }

    /**
     * Create a new Cap12xx controller.
     *
     * @param i2cName I2C port name where the controller is attached. Cannot be null.
     * @param alertName optional GPIO pin name connected to the controller's
     *                  alert interrupt signal. Can be null.
     * @param chip identifier for the connected controller device chip.
     * @param handler optional {@link Handler} for software polling and callback events.
     * @throws IOException
     */
    public Cap12xx(String i2cName, int i2cAddress, String alertName, Configuration chip, Handler handler)
            throws IOException {
        mChipConfiguration = chip;
        try {
            PeripheralManager manager = PeripheralManager.getInstance();
            I2cDevice device = manager.openI2cDevice(i2cName, i2cAddress);
            Gpio alertPin = null;
            if (alertName != null) {
                alertPin = manager.openGpio(alertName);
            }
            init(device, alertPin, chip, handler);
        } catch (IOException|RuntimeException e) {
            // Close the peripherals if an init error occurs
            try {
                close();
            } catch (IOException|RuntimeException ignored) {
            }
            throw e;
        }
    }

I'm happy to raise a PR, but since this is my first Java rodeo my code may be a little rough around the edges and I'm currently missing tests for this feature.

proppy commented 5 years ago

@Gadgetoid Thanks for the report! PR are always welcome.

Looking at the existing tests: https://github.com/androidthings/contrib-drivers/blob/master/cap12xx/src/test/java/com/google/android/things/contrib/driver/cap12xx/Cap12xxTest.java https://github.com/androidthings/contrib-drivers/blob/master/cap12xx/src/androidTest/java/com/google/android/things/contrib/driver/cap12xx/Cap12xxTestActivity.java It doesn't seems that we have any coverage for the public constructors, so I'm not sure it would be required to add new ones, but @jdkoren is the authority there :)

jdkoren commented 5 years ago

I think for now don't worry about adding tests just for the constructors.