pi_ina219
pi_ina219 copied to clipboard
Abstract i2c driver
@chrisb2 here is the suggested abstraction of the I2C drivers.
The PR targets master
, though it builds up on my other branch add-type-hints
(https://github.com/chrisb2/pi_ina219/pull/29). Not sure how to change this on GitHub.
This change would make your branch convert-to-smbus2
obsolete.
I'm also removing the install_requires
from setup.py
, because testing packages aren't needed for installation.
To be discussed would be, what the best location for the example/reference implementations of I2C devices (AdafruitI2cDevice
, SmbusI2cDevice
, and SmbusI2cDevice
) could be. For now, I just put them into the same module. But the import
statements within the constructors are a bit debatable - not a problem for example code I'd say, but not so nice in the delivered module. Speaking of which - perhaps examples.py
wouldn't be such a bad place for those.
Codecov Report
Merging #30 (4282d43) into master (1f5da6b) will decrease coverage by
7.14%
. The diff coverage is78.49%
.
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
- Coverage 99.14% 92.00% -7.15%
==========================================
Files 1 1
Lines 234 275 +41
Branches 18 26 +8
==========================================
+ Hits 232 253 +21
- Misses 1 21 +20
Partials 1 1
Impacted Files | Coverage Δ | |
---|---|---|
ina219.py | 92.00% <78.49%> (-7.15%) |
:arrow_down: |
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 1f5da6b...4282d43. Read the comment docs.
I think this is looking good, may take me a bit of time to review (I am learning a few things in Python from what you have done!).
This will be a breaking change for people who upgrade, but I guess we can just give instructions in Readme on exactly what to do?
I had a detailed look at this, pretty happy.
Would just like to discuss how we can make this library as easy as possible to use particularly for those who have only minimal experience with Python.
- imports in constructors - you indicated this is not ideal, can you explain? I did not find anything googling. Seems like not a problem if it makes it easier for newbies?
- I2C lib dependency - we could add smbus2 to requirements in setup.py and fix example.py to match this, this would mean newbies would get working code out of the box after installing ina219. The readme could then separately describe how to use the other I2C libs. We would provide upgrade instructions in readme for those upgrading from 1.x explaining code change required. Maybe you can think of something else from your Python knowledge?
Chris