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

Add custom exception classes instead of using plain error types. #9

Open
wants to merge 1 commit into
base: master
from

Conversation

@Ferroin
Copy link

Ferroin commented Oct 8, 2019

This adds custom exception classes so that users do not have to match on generic exception types in code that uses this module.

The new custom classes are structured so that existing code that is matching on the generic exception types will still work, and so that there's a primary exception type that can be matched on to catch all
errors specific to this module.


Fixes: #8

This is minimally tested right now (using flake8 to check that there are no syntax errors or other obvious problems with the changes). I intend to test it further, but won't be able to until at least this evening, or possibly tomorrow evening. I'll comment here again when I've finished proper testing.

This adds custom exception classes so that users do not have to match on
generic exception types in code that uses this module.

The new custom classes are structured so that existing code that is
matching on the generic exception types will still work, and so that
there's a primary exception type that can be matched on to catch all
errors specific to this module.
@ladyada ladyada requested a review from adafruit/circuitpythonlibrarians Oct 8, 2019
@ladyada

This comment has been minimized.

Copy link
Member

ladyada commented Oct 8, 2019

looks good! please let us know when you've tested this with hardware :)

@Ferroin

This comment has been minimized.

Copy link
Author

Ferroin commented Oct 11, 2019

looks good! please let us know when you've tested this with hardware :)

I don't have actual hardware to test with, as mentioned in the issue this was prompted by review of a plugin for Netdata that uses this module to collect data from the hardware (relevant discussion that prompted this starts at netdata/netdata#7024 (comment)).

However, I have now (now that I finally got time to do so) tested it using Linux's i2c-stub driver to simulate an AM2320 (configured to just return a static value for the temperature and humidity), and nothing appears to behave any different with or without the change (except of course for the type of the exceptions that get raised, and the memory usage (marginally higher, empty classes aren't free, but they're not particularly expensive either)). I can't really confirm the I2C read error behavior this way though (at least, I'm pretty sure I can't, I don't think I can get the i2c-stub driver to simulate a read error), but the CRC mismatch and connection failure errors work correctly.

@ladyada

This comment has been minimized.

Copy link
Member

ladyada commented Oct 11, 2019

ooh ok - we'll get to reviewing this later, hardware's tricky so we like to always test with physsical hardware :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.