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

MCP23017 - Allow setting of the invert input register (can switch pins be set individually to active-high logic)? #68

Closed psgcooldog closed 5 years ago

psgcooldog commented 5 years ago

I have four inputs coming from a small RF remote decoder module into an MCP23017 , and they are active-high, rather than the usual active-low switch closure to ground. I am trying to use the debounce and "pressed/held" logic provided by the Switch Class.

Is there any way to make that work?

psgcooldog commented 5 years ago

Apparently this can be done with the MCP23017's "Input Polarity Register".

I can confirm that this does the trick. I did a horribly ugly hack on your lib to test that out.

davetcc commented 5 years ago

Agreed the only thing that would need to be done is to deal with the input pull-up in switches. Does active high logic work with input pull-up?

If it does I can just make the invert input register available as a method call.

psgcooldog commented 5 years ago

Yes, it works fine.

davetcc commented 5 years ago

Ok I’ll make a patch release for this soon so that you don’t need the hack. Would you be able to test it with both capabilities?

psgcooldog commented 5 years ago

Yes, the footswitches are normal "pulled to ground for active". They do have external pullups, but I left the internal ones active. The four bits from the RF remote decoder are active-high, and have the internal pullups on as well. I can test any combination you want.

Right now, the only change I made was to toggle the 4 IPOL register bits to 1, and it's working nicely. Bad things happen if you have the sense of the switch input inverted!!! :)

davetcc commented 5 years ago

This will now be available in 1.4.10 due shortly. It will start as a simple way to set the invert register on the MCP23017

davetcc commented 5 years ago

The version on master is now tested with a pull-down and pull-up switch on an MCP23017. This worked fine, along with a rotary encoder and LCD screen attached to the same device. It was tested in both interrupt mode and polling mode. There's a possibility that the issue you saw (see the first commit for the comments) is when you have so many pins used as switches or an unexpected interaction with multi IO.

My strong recommendation for inverting the logic is to use the new additional parameter to addSwitch and not to use the logic invert register on the MCP23017 (which will give unexpected results with INPUT-PULLUP). When you set the invert parameter to true, it will now turn off the PULL-UP's, requiring a PULL-DOWN switch circuit, or as in your case, if you're reading these values from the output of another IC/device, there's probably no need even for pull down's.

I'll hold off releasing 1.4.10 until we sort this out, as a lot of people are using IoAbstraction and I want to be absolutely certain it's working properly. However, there's no rush as nobody's really waiting on this release so whenever you have time.

psgcooldog commented 5 years ago

I have done some more testing. I refactored the RF decoder (the active-high device) stuff to make use of a SwitchListener class. Tested by itself, it worked fine.

When I also initialize the active-low footswitch code, the active-high device no longer gets any callbacks in response to inputs. ---------------------EDIT------------------------------------------------

Egg, meet face ...

I forgot that I have to reset the MAX_KEYS in SwitchInput.h every time I update the lib!

It's working fine. LOL ... that just got me out of bed to try it. Glad it worked :)

davetcc commented 5 years ago

Yep the MAX_KEYS will disappear in the next major release, basically 1.5.0 now. 1.4.10 will have the ability to provide a different wire to most places that take a wire impl.

Thanks for confirming its working.