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

Pin constants (P0, P1, P2 & P3) are common to both the ADCs, and should be extracted into a common module #100

Open
pantheraleo-7 opened this issue Dec 6, 2024 · 3 comments

Comments

@pantheraleo-7
Copy link

pantheraleo-7 commented Dec 6, 2024

Currently, these constants are repeated in both the ads1015 and ads1115 modules. This leads to not only duplication but also weird import statements.

To access these constants the ads1015/ads1115 module is imported as ADS (because we want to name the object of the ADS1015/ADS1115 class as ads). This is in violation of PEP8 as module names are not written in all caps, and also leads to inconsistent import patterns since the AnalogIn class is directly imported from its module (from .. import ..) whereas the ADS1015 or ADS1115 classes are not (import .. as ADS; ADS.ADS1015).

So I propose that these four constants should be copied over to ads1x15 module.
I see that various "enum-like" classes are already defined in the common ads1x15 module. So we could also do this:

class Pin:
    ZERO = 0
    ONE = 1
    TWO = 2
    THREE = 3

After this [backwards-compatible (as we are not removing these constants from their original locations)] implementation, I see the following improvements in the import/usage pattern:

import board
import busio
from adafruit_ads1x15 import Pin # or ads1x15
from adafruit_ads1x15.ads1115 import ADS1115
from adafruit_ads1x15.analog_in import AnalogIn

i2c = busio.I2C(board.SCL, board.SDA)
ads = ADS1115(i2c)
chan = AnalogIn(ads, Pin.ZERO) # or ads1x15.P0

as opposed to the current usage style.

Currently, the best workaround is from adafruit_ads1x15.ads1115 import ADS1115, P0 but I haven't seen any precedence for this nor does it seem like a good idea to import a constant directly.

@pantheraleo-7
Copy link
Author

pantheraleo-7 commented Dec 7, 2024

Why not re-export the classes ADS1015, ADS1115 & AnalogIn to __init__.py? That way, we would have:

import board
import busio
from adafruit_ads1x15 import ADS1115, AnalogIn, ads1x15 as adsx

i2c = busio.I2C(board.SCL, board.SDA)
ads = ADS1115(i2c)
chan = AnalogIn(ads, adsx.P0) # or adsx.Pin.ZERO

Everything in one beautiful import statement. This pattern would also make accessing the other "enum-like" classes easier and readable.

@pantheraleo-7
Copy link
Author

At the very least, the pin constants should be re-exported to ads1x15 either as constants or an "enum-like" class.

@pantheraleo-7
Copy link
Author

Also, I would argue that we should rename the constants as PINx instead of Px. This is more readable.

@pantheraleo-7 pantheraleo-7 changed the title Pin constants (P0, P1, P2 & P4) are common to both the ADCs, and should be extracted into a common module Pin constants (P0, P1, P2 & P3) are common to both the ADCs, and should be extracted into a common module Dec 7, 2024
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

1 participant