-
-
Notifications
You must be signed in to change notification settings - Fork 221
Improve smartprotocol error handling and retries #578
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
273d8a2
to
1a57e63
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #578 +/- ##
==========================================
- Coverage 79.59% 79.26% -0.34%
==========================================
Files 37 37
Lines 3088 3178 +90
Branches 806 818 +12
==========================================
+ Hits 2458 2519 +61
- Misses 547 574 +27
- Partials 83 85 +2 ☔ View full report in Codecov by Sentry. |
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.
Looking good! 👍 Just some brief feedback off the bat, I'll check out this in more detail later.
Most of the comments addressed. I think the one you may come back on from the open conversations is the |
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.
Looking really good, just a couple of very minor nits that may be worth considering.
ea2d9fd
to
e4b7b76
Compare
All addressed and rebased to master so hopefully ready to go |
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.
Let's get it merged then, thanks! 👍
This PR introduces the SMART error codes and has handling to interpret them and avoid retries for authentication errors.
I haven't been able to test it with an actual AES device but it works with TAPO/KLAP.
I've rebased #569 on top of this PR to help with testing.