-
Notifications
You must be signed in to change notification settings - Fork 16
Possible fix for #11 #12
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
Conversation
i thought we added it to blinka? |
oh right its on blinka (required) but not busio. |
Yep. And that all worked. New issue is that it can then fail on native CP since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we always want writeto_then_readfrom available so that drivers are written for it going forwards. (Otherwise they won't work with Blinka.)
So, on this PR detect in writeto_then_readfrom or in the constructor whether it is available and provide a substitute. Bonus points for adding writeto_then_readfrom into CircuitPython too. Thanks!
Don't most drivers use And in the future, when |
Yes, most do but this driver shouldn't assume I2CDevice is used (just like busio.I2C shouldn't.) It should always provide writeto_then_readfrom even if the underlying I2C bus doesn't. As is, it only adds it when the underlying implementation has it. Doing it only sometimes risks people writing code that won't work on Blinka. I created a PR to remove stop= from CircuitPython in favor of writeto_then_readfrom here: adafruit/circuitpython#2012 |
Is this PR even needed then? If CP's busio.I2C provides |
It'll be needed for older versions. Want me to wrap it up? |
Sure. Works for me. |
It's the future!
@caternuson I've commited to your branch with what I think will work. Please let me know what you think. Thanks! |
Ah, OK. I think I was trying to avoid duplicating the logic from Not sure what it would be - but can you think of any issues in the future if the code in |
Ah, I think it's ok to duplicated it since it's temporary. There is no guarantee the this is used with I2CDevice so it's better to have this updated now. I can't think of any issues but we can always fix things if it comes up. |
OK, cool with me. I just took care of the minor lints. |
Thanks! I totally missed that it had issues. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_HX8357 to 1.1.0 from 1.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_HX8357#4 from makermelissa/master Updating https://github.com/adafruit/Adafruit_CircuitPython_TCA9548A to 0.2.1 from 0.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_TCA9548A#12 from caternuson/iss11 > Merge pull request adafruit/Adafruit_CircuitPython_TCA9548A#10 from caternuson/iss9 Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Shapes to 1.1.2 from 1.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#6 from makermelissa/master > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#5 from makermelissa/master > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#4 from makermelissa/master > fixed typo in rtd link
Can't just add logic in existing function since it would still
hasattr
. Need to dynamically provide it as needed.Don't have an INA219 to test with original issue, but tested with an I2C FRAM and TCA:
tca_fram_test.py
RPI/Blinka
ItsyM4/CP