Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine with adafruit_pcf8574 into a single implementation #5

Open
rgov opened this issue Oct 31, 2023 · 2 comments
Open

Combine with adafruit_pcf8574 into a single implementation #5

rgov opened this issue Oct 31, 2023 · 2 comments

Comments

@rgov
Copy link
Contributor

rgov commented Oct 31, 2023

In #4 I showed that it's a very simple code change to support the PCF8574 and other 8-bit expanders in the same module.

Therefore, it doesn't seem necessary to maintain two separate repos when the functionality is effectively identical. Fixes and improvements need to be submitted to both, for example #3 also applies to the Adafruit_CircuitPython_PCF8574 repo.

I can see from comparing the two that there are a lot of (mostly minor) differences between them, including inconsistent copyright/license, and at least one bug:

-    def pull(self, val: digitalio.Pull.UP) -> None:
+    def pull(self, val: digitalio.Pull) -> None:

Unifying these into adafruit_pcf857x module with backwards-compatible stubs for adafruit_pcf8574 and adafruit_pcf8575 would reduce the maintenance burden and likely result in a minor decrease in size for the CircuitPython library bundle. For example:

# in adafruit_pcf8574.py
class PCF8574(PCF857x):
    def __init__(
        self, i2c_bus: busio.I2C, address: int = PCF8574_I2CADDR_DEFAULT
    ) -> None:
        super().__init__(i2c_bus, address, gpios=8)

# in adafruit_pcf8575.py
class PCF8575(PCF857x):
    def __init__(
        self, i2c_bus: busio.I2C, address: int = PCF8575_I2CADDR_DEFAULT
    ) -> None:
        super().__init__(i2c_bus, address, gpios=16)

# in adafruit_pcf857x.py
class PCF857x:
    # As in #4. Maybe remove default values for constructor args.
    pass  

pcf857x is/was the name used in the Linux kernel to cover a lot of devices, listed in this document.

@tannewt
Copy link
Member

tannewt commented Oct 31, 2023

Generally we have separate libraries for separate chips to make it clear what works with what chip. It is interesting that Linux has about a dozen chips under one driver though.

@rgov
Copy link
Contributor Author

rgov commented Nov 1, 2023

Right, those "frontend" packages could still exist for clarity, but use a common backend module since the wire protocol is the same, and then you spare yourself the code duplication and maintenance overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants