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

Pass a partially received response in the ValueError exception. #113

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

Conversation

acolomb
Copy link
Contributor

@acolomb acolomb commented Feb 3, 2021

Allow better debugging when an incomplete response was received or
timed out. The already received part (empty on timeout) is passed as
the first and only argument in the ValueError exception, accessible as
e.args[0] in a handler.

I find the choice of ValueError exception kind of confusing for this type of error, but anything else would break compatibility. So I opted to just add the (partial) response verbatim as an argument. If it is empty, a TimeoutError or similar might be easier to understand?

Allow better debugging when an incomplete response was received or
timed out.  The already received part (empty on timeout) is passed as
the first and only argument in the ValueError exception, accessible as
e.args[0] in a handler.
@coveralls
Copy link

coveralls commented Feb 3, 2021

Coverage Status

Coverage decreased (-0.1%) to 96.158% when pulling 7522a4f on acolomb:pass-partial-response-in-exception into f1128a7 on AdvancedClimateSystems:master.

When a serial CRC validation fails, the exception is constructed with
a descriptive text as its only argument.  To aid in debugging, pass in
the original failing message instead.  Make sure the string
representation of the exception stays the same, by adding a __str__()
method like in the library's other exceptions.
@acolomb
Copy link
Contributor Author

acolomb commented Mar 3, 2021

Applied the same treatment to the CRCError class, which is also useful in debugging.

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