Add support for MCP23008 and MCP23017 ICs

[pmuller]

Fix #199

[codecov-io]

Codecov Report

Merging #651 into master will increase coverage by 11.69%.
The diff coverage is 95.78%.

@@             Coverage Diff             @@
##           master     #651       +/-   ##
===========================================
+ Coverage   74.84%   86.53%   +11.69%     
===========================================
  Files          22       40       +18     
  Lines        4214     7823     +3609     
  Branches      609        0      -609     
===========================================
+ Hits         3154     6770     +3616     
- Misses        987     1053       +66     
+ Partials       73        0       -73

Impacted Files	Coverage Δ
gpiozero/__init__.py	100% <100%> (ø)	⬆️
tests/test_i2c.py	100% <100%> (ø)
tests/test_mcp230xx.py	100% <100%> (ø)
gpiozero/exc.py	100% <100%> (ø)	⬆️
gpiozero/pins/pi.py	88.88% <100%> (+1.79%)	⬆️
gpiozero/mixins.py	74.71% <65.62%> (-2.69%)	⬇️
gpiozero/pins/mcp230xx.py	95.34% <95.34%> (ø)
gpiozero/i2c.py	96.96% <96.96%> (ø)
gpiozero/compat.py	55.55% <0%> (-15.42%)	⬇️
... and 36 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 995b746...0d8474f. Read the comment docs.

[pmuller]

I did live tests of this implementation with 2 x MCP23017 attached to a Raspberry Pi 3.
However, I couldn't test the code with MCP23008 as I don't have any. Reading the datasheet, I expect it to work as well.

[ffleandro]

Does this implement the interruption pin for using MCP23017 ports as inputs?

[ffleandro]

I don't think that the MCP230xx class belongs to the pins group. This group is for the middleware implementation for acessing pins, for instance, using the RPI.GPIO library or PiGPIO.

I think the correct architecture should be to create an i2c_devices.py similar to the spi_devices.py and include in it all the devices that communicate through i2c.

This approach doesn't tie the MCP230xx implementation to a specific pin access protocol, you could still use your chosen pin library.

[ffleandro]

For instance, I use pigpio for the pin factory because I need a specific method from this library to configure a glitch filter.

If I use the MCP23017Factory I cannot use this function from pigpio.

May I try to refactor your code architecture as explained?
I have successfully tested your code both using the MCP23017 pins as input and output.

[ffleandro]

@pmuller could you enter this conversation?

After studying some of the code and using this pull-request in my application, I've reconsidered my opinion and I think you've used the right approach.

I found only a bug so far, somehow in the creation of a Button(pin, pull_up=True), the pull_up flag gets lost.
If forcing self.pull = 'up' in MCP230xxPin constructor (mcp230xx.py, line 228), everything works as expected, otherwise a floating value is assumed.

[ffleandro]

So, regarding the pull problem, the error occurs because the Button class checks if it's pin already has pull been set and the MCP230xxPin implementation has a default value of floating for it's pin.
Settings the default value to None solves this problem.

Also, I found a really complex bug in the MCP230xxPoller class.

Inside it's loop implementation, everytime the poller checks for a pin state change it calls the factory pin creation method for ALL 16 pins. Every. single. loop.

Also I think this poller is very inneficient since it does lot's of unnecessary gpio reads.

Why is this poller class necessary and not use the default gpiozero behaviour for checking state changes?

[pmuller]

@ffleandro I believe I set the default to "floating" because it is the MCP230xx ICs default (I created another PR to handle this: #658).

I don't know what should be the proper default value in the context of gpiozero. Any opinion @bennuttall ?

Inside it's loop implementation, everytime the poller checks for a pin state change it calls the factory pin creation method for ALL 16 pins. Every. single. loop.

Yes, but MCP230xxPin objects are cached: https://github.com/RPi-Distro/python-gpiozero/pull/651/files#diff-a583da2ae5f3f1c2e67628bf52236b58R566
And we need access to the pin object to update its internal state.

Also I think this poller is very inneficient since it does lot's of unnecessary gpio reads.
Why is this poller class necessary and not use the default gpiozero behaviour for checking state changes?

The default gpiozero behavior is to rely on factories' native method to be notified of state change (i.e. https://sourceforge.net/p/raspberry-gpio-python/wiki/Inputs/). But this only works for the Pi's GPIO.

For MCP230xx ICs, I only about 2 methods:

• Polling the GPIO register (current implementation)
• Using the IC's interruption registers to get notified of state changes; but this approach is much more complex as you can have a lot of different "event driven configurations" based on your registers configuration (see GPINTEN, DEFVAL, INTCON, INTF, INTCAP, IOCON.MIRROR, IOCON.ODR, IOCON.INTPOL)

I fully agree interruptions is the way to go to prevent useless polling (and resources waste), but I am afraid I lack the necessary expertise to design a generic framework that allows interruption usage with all those configuration options.

[ffleandro]

Yes, but MCP230xxPin objects are cached

True, although it's ineficient to check for a state change in an unnused pin.

Also, if you have more than one MCP230xxFactory attached to the same address, the poller will override configurations made in the other factory since it is creating all 16 MCP230xxPin with default values (floating, input, etc).

Here is a proposition for the MCP230xxPoller run loop implementation:

def run(self):
    """Run the polling loop.
    """
    while True:
        if self.stopping.wait(self.interval):
            break
        if not self.subscribers:
            continue

        states = self.factory.gpio.read()
        for number in list(self.factory._pins.keys()):
            self._run_for_pin(number, states)

I removed the line for value in self.factory.gpio.values since it was doing an unnecessary i2c read operation and changed the iteration to only check for a pin state change in the pins that were created by the user application instead of checking all pins.

[thiagohmoreira]

May I ask/suggest something not strictly related to this PR, but on a similar direction? I mean, you adding not only support for the mentioned ICs, but also doing some ground work for the I2C support itself.

I'm very interested in I2C support, and willing to help in its implementation. Although I'm not super experienced with gpiozero nor python, I could lend a hand and hopefully put some other devices in the supported list too.

My proposal would be to have the I2C support into it's own PR and specific devices would follow that. Makes sense?

[iamamused]

Any status on this? I'm looking for MCP23017 support as well.

[pmlk]

Great work @pmuller! I just seamlessly integrated an MCP23017 thanks to you. 🎉

@bennuttall is there anything we as the community can do to help speed things up? Specifically, what is missing in this PR to move this from "Less important / not ready" to at least "Less important / ready" (besides the obvious conflicts)? 😃 I'd be willing to help.

[ffleandro]

+1 on this.
Also, would be great to remote control an MCP the same way we remote control a Raspberry Pi GPIO (using pigpio).

[agruenberger]

I have some questios:

1. Is there some one still working on this issue ?
2. Why is this issue still open
3. Why is CI (travis) not working any more
4. I have made my own solution for MCP23xxx and whant to contribute to your solution, because i have a lot experience with this Chip --> How should i continue (Folk this fork???)

Thanks in advance and sorry for my bad english

PS: I am try to buy this chip in the SPI-variant (MCP23Sxx) to demonstrate the cross implementation of the same chip on different kind of busses (I2C and SPI ... etc.)

Andy

[lurch]

3. Why is CI (travis) not working any more

I did a bit of digging, and it looks like this is why. And it looks like there's a little bit more info here. 👈 @bennuttall

[agruenberger]

Thanks for fast response .... investigating greets andy     Gesendet: Samstag, 26. Februar 2022 um 15:18 Uhr Von: "Andrew Scheller" ***@***.***> An: "gpiozero/gpiozero" ***@***.***> Cc: "agruenberger" ***@***.***>, "Manual" ***@***.***> Betreff: Re: [gpiozero/gpiozero] Add support for MCP23008 and MCP23017 ICs (#651)   Why is CI (travis) not working any more I did a bit of digging, and it looks like this is why. And it looks like there's a little bit more info here. 👈 @bennuttall — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[bennuttall]

Is there some one still working on this issue ?
Why is this issue still open

This is a pull request adding a new set of complex features which function quite differently to the way the rest of the library work. We always wanted to add I2C support for devices such as the expander chips, and had ideas for how best they would fit alongside the rest of the library, but we've since got busy with other projects such as piwheels and since gpiozero works well for its original purpose (making projects with everyday GPIO devices in education) and much more, it's ok to slow/pause development for now.

Why is CI (travis) not working any more

Travis-CI shut down and now we use GitHub Actions: https://github.com/gpiozero/gpiozero/actions

I have made my own solution for MCP23xxx and whant to contribute to your solution, because i have a lot experience with this Chip --> How should i continue (Folk this fork???)

With the above in mind, the best approach would be to implement your solution in a new project which extends gpiozero classes. Import the bits of gpiozero you need, and build on those foundations. If you wanted to, you could publish this as a new module, so that others can use it. It is not necessary for this functionality to be a part of the core gpiozero library for it to be useful to you and others.]

Hope that helps

[agruenberger]

Thanks for your quck response, i'll do it as you have been proposed