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

Convert unknown error codes to a useful exception class. #112

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

acolomb
Copy link
Contributor

@acolomb acolomb commented Feb 3, 2021

Introduce an UndefinedModbusException class and use it when the lookup
in error_code_to_exception_map fails. Previously that would just
throw a KeyError with the contained value, so this change makes it
more obvious what actually happened.

Introduce an UndefinedModbusException class and use it when the lookup
in error_code_to_exception_map fails.  Previously that would just
throw a KeyError with the contained value, so this change makes it
more obvious what actually happened.
@coveralls
Copy link

coveralls commented Feb 3, 2021

Coverage Status

Coverage decreased (-0.3%) to 95.969% when pulling c3212d6 on acolomb:handle-undefined-error-codes into f1128a7 on AdvancedClimateSystems:master.

@acolomb
Copy link
Contributor Author

acolomb commented Feb 3, 2021

The docstrings for all these exception classes are somewhat inconsistent. If you like, I will include a commit making them PEP-257 compliant.

The __repr__() special method is usually called in place of __str__(),
but only if the latter is not defined.  However the BaseException
class already has a __str__() method, so we need to override that
specifically.  Note that this change doesn't touch the other cases
below where the same problem exists.

Initialize the error_code field explicitly using the KeyError argument
instead of passing in the whole exception object.
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.

None yet

2 participants