TcMenu / IoAbstraction

Rotary encoders, fully debounced switches, EEPROM support on Arduino and mbed - direct and over I2C
Apache License 2.0
162 stars 32 forks source link

PCF8574 / PCF8575 Support for inverted logic #158

Closed vzahradnik closed 2 years ago

vzahradnik commented 2 years ago

Hello Dave,

While working with the PCF8575 expander, I found out that I need to invert the bit logic, and I think such a support could be easy to add into the library.

As a reference, I found another library which inverts the whole bit array. They have a flag for the inversion: https://github.com/4-20ma/I2cDiscreteIoExpander/blob/master/src/I2cDiscreteIoExpander.cpp

I'm thinking to either allow to configure the inversion per port (no bit inversion would be the default) or to invert the whole bit array.

So far I came up with this hacky implementation:

bool PCF8574IoAbstraction::runLoop(){
    bool writeOk = true;
    size_t bytesToTransfer = bitRead(flags, PCF8575_16BIT_FLAG) ? 2 : 1;
    if (bitRead(flags, NEEDS_WRITE_FLAG)) {
        bitWrite(flags, NEEDS_WRITE_FLAG, false);
        toWrite[0] = ~toWrite[0];
        toWrite[1] = ~toWrite[1];
        writeOk = ioaWireWriteWithRetry(wireImpl, address, toWrite, bytesToTransfer);
    }

    if(bitRead(flags, PINS_CONFIGURED_READ_FLAG)) {
        writeOk = writeOk && ioaWireRead(wireImpl, address, lastRead, bytesToTransfer);
        lastRead[0] = ~lastRead[0];
        lastRead[1] = ~lastRead[1];
    }
    return writeOk;
}

Thanks for the feedback!

davetcc commented 2 years ago

Sorry, been a really busy week this week, not had a chance to look at this before now. Did you take a look at https://github.com/davetcc/IoAbstraction/blob/master/src/NegatingIoAbstraction.h it was written a long time ago and actually well tested by the unit tests, and maybe is not that well documented but it may do what you want already, I see you've raised a PR for this as well, I'll try and take a look through this over the weekend.

vzahradnik commented 2 years ago

Hi Dave,

No I didn't know about this. But I already can tell it wouldn't work. Simply negating the logic wasn't enough in my case.

In order to read from the expander, the value of the input pin needs to be always set to HIGH (1), even in cases of inverted logic. I spent quite some time figuring out why the buttons don't work, and this was the reason.

As soon as I wrote 1's to the input pins, buttons started to work.

For the output pins, your abstraction would work fine. At the moment, I use your library from my custom branch with all the changes. This gives me time to continue with my project. So take your time to review the PR.

vzahradnik commented 2 years ago

I decided to test NegatingIoAbstraction anyway. It's better to have a confirmation than to guess. But as expected, it didn't work.

davetcc commented 2 years ago

This is implemented by PR #160 and just needs smoke testing on a few different boards to ensure all is good.

davetcc commented 2 years ago

Sorry this got backed up behind a few other issues. I’ll be doing a release in a couple of days for IoAbstraction and hopefully not too long after for tcMenu.

davetcc commented 2 years ago

All tested and ready to go!