mattjlewis / diozero

Java Device I/O library that is portable across Single Board Computers and microcontrollers. Tested with Raspberry Pi, Odroid C2, BeagleBone Black, Next Thing CHIP, Asus Tinker Board and Arduinos / Pico. Supports GPIO, I2C, SPI as well as Serial communication. Also known to work with Udoo Quad.
https://www.diozero.com
MIT License
261 stars 59 forks source link

There is a bug in PCA9685 implementation #196

Closed dafik closed 3 months ago

dafik commented 5 months ago

When you call "on" with float 1

public void setValue(int channel, float value) throws RuntimeIOException {
        if (value < 0 || value > 1) {
            throw new IllegalArgumentException("PWM value must 0..1, you requested " + value);
        }
        int off = Math.round(value * RANGE);
        setPwm(channel, 0, off);
    }

off is assigned with 4096, beacause of RANGE constant

next in setPwm method, validateOnOff(on, off)is called

which causes IllegalArgumentException to be thrown

if (off < 0 || off >= RANGE) {
    throw new IllegalArgumentException(String.format("Error: off (" + off + ") must be 0.." + (RANGE - 1)));
}

I think, as pca is 12bit resolution, max value should be 4095

4095 = 1111 1111 1111
4096 = 1 0000 0000 0000

and validation should be open set (> not >=)

    private static void validateOnOff(int on, int off) {
        if (on < 0 || on > RANGE) {
            throw new IllegalArgumentException(String.format("Error: on (" + on + ") must be 0.." + (RANGE - 1)));
        }
        if (off < 0 || off > RANGE) {
            throw new IllegalArgumentException(String.format("Error: off (" + off + ") must be 0.." + (RANGE - 1)));
        }
        // Off must be after on
        if (off < on) {
            throw new IllegalArgumentException("Off value (" + off + ") must be > on value (" + on + ")");
        }
        // Total must be < 4096
        if (on + off > RANGE) {
            throw new IllegalArgumentException(String.format("Error: on (%d) + off (%d) must be < %d", on, off, RANGE));
        }
    }
mattjlewis commented 4 months ago

Thank you - will fix