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

Changing adafruit_bus_device to duck typing #3936

Merged
merged 6 commits into from
Jan 25, 2021

Conversation

gamblor21
Copy link
Member

The first attempt at moving bus device to the core assumed the bus was the busio I2C when it could be bitbangio or others. The new methods use duck typing to call the function names regardless of the underlying bus.

@gamblor21
Copy link
Member Author

gamblor21 commented Jan 5, 2021

Created as a draft for a code review while I continue to test it. I may require help to make sure the changes solve some of the reported issues.

I also wanted to check/ask if anyone sees anything to be done with the SPI portion of the code or is it safer to assume that always will use the SPI bus (in which case I need to check for it and raise an error if not).

I have not yet re-enabled the flag to build it.

@tannewt tannewt requested a review from jepler January 6, 2021 00:08
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! No testing performed, but I read through the changes and found a couple of items that can use attention.

shared-bindings/adafruit_bus_device/I2CDevice.c Outdated Show resolved Hide resolved
shared-bindings/adafruit_bus_device/I2CDevice.c Outdated Show resolved Hide resolved
shared-module/adafruit_bus_device/I2CDevice.c Outdated Show resolved Hide resolved
shared-module/adafruit_bus_device/I2CDevice.c Outdated Show resolved Hide resolved
@gamblor21 gamblor21 marked this pull request as ready for review January 17, 2021 17:58
@gamblor21
Copy link
Member Author

Ready for review. This was done to fix the root cause for these issues:
#3856 - With duck typing this should no longer be an issue. I tested it using busio, bitbangio and a test class that just implemented the basic functions expected from the duck typing (and otherwise did nothing)
#3821 - I do not have a SHTC3 sensor to test this with. But the new code exactly copies the python code

I only have a couple I2C devices I can test with. So far most of my testing was with the BME280. I will dig out the others I have to keep going.

There is one build also failing and I am unsure why.

@tannewt tannewt requested a review from jepler January 20, 2021 02:42
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very much like I expect. No testing performed.

@jepler
Copy link
Member

jepler commented Jan 20, 2021

The lone build failure was an occasional CI weirdness.

@gamblor21
Copy link
Member Author

In addition to the BME280 tested with PCT2075, BH1750 and BNO-055 with busio.I2C and bitbangio.I2C. All work except the BNO-055 with bitbangio, but testing it with the python busdevice library the issue persisted so seems to be an issue with that library or bitbangio and not with this build.

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

Successfully merging this pull request may close these issues.

3 participants