Skip to content

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

Merged
merged 3 commits into from
Dec 10, 2023

Conversation

sdb9696
Copy link
Collaborator

@sdb9696 sdb9696 commented Dec 8, 2023

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.

@sdb9696 sdb9696 force-pushed the update_smart_protocol branch from 273d8a2 to 1a57e63 Compare December 8, 2023 19:42
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 57 lines in your changes are missing coverage. Please review.

Comparison is base (a77af5f) 79.59% compared to head (e4b7b76) 79.26%.

Files Patch % Lines
kasa/aestransport.py 12.12% 29 Missing ⚠️
kasa/smartprotocol.py 12.50% 25 Missing and 3 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Member

@rytilahti rytilahti left a 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.

@rytilahti rytilahti added the enhancement New feature or request label Dec 8, 2023
@sdb9696
Copy link
Collaborator Author

sdb9696 commented Dec 9, 2023

Most of the comments addressed. I think the one you may come back on from the open conversations is the TimeoutException but hopefully it makes sense.

@sdb9696 sdb9696 requested a review from rytilahti December 9, 2023 10:08
Copy link
Member

@rytilahti rytilahti left a 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.

@sdb9696 sdb9696 force-pushed the update_smart_protocol branch from ea2d9fd to e4b7b76 Compare December 10, 2023 09:29
@sdb9696
Copy link
Collaborator Author

sdb9696 commented Dec 10, 2023

All addressed and rebased to master so hopefully ready to go

@sdb9696 sdb9696 requested a review from rytilahti December 10, 2023 09:38
Copy link
Member

@rytilahti rytilahti left a 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! 👍

@rytilahti rytilahti merged commit 2e6c41d into python-kasa:master Dec 10, 2023
@sdb9696 sdb9696 deleted the update_smart_protocol branch February 20, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants