-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Make tplink light more responsive #28652
Conversation
Hey there @rytilahti, mind taking a look at this pull request as its been labeled with a integration ( |
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.
I don't have generally speaking nothing against this, but it'd be great to have this done simultaneously for all platforms to keep them in sync.
I haven't noticed any issues with the switch platform so I didn't think there was a need to change anything. |
Both of the platforms share most of the code and should probably be derived from a common base class to keep them in sync without extra effort :-) The same "problem" should also be relevant to plugs as they are polled just like the lights (or maybe the lights are slower at responding?) |
The lights tend to make 4 or 5 duplicate calls to get state information where the switch make only 1 call. This appears to be the cause. |
Yes, that's the reason why I added caching – I had a couple of bulbs earlier for testing, and noticed how homeassistant spams the bulbs :-) |
Tests updated to mock the tplink module and not the API. |
As there has been no objections, let's get this merged. Thanks for the PR @vangorra! |
This PR has caused flaky tests like this |
I will have to revert this PR before the next release to make sure that we don't have flaky tests anymore in |
Ping @vangorra – any ideas what could be wrong? |
Breaking Change:
None
Description:
Tplink light bulbs take upwards of 6 seconds to update their state after taking action. This makes for a bad user experience. This change makes the entity optimistically update the state, run the actual update in the background and throttles the number of update calls.
Related issue (if applicable): fixes None
Pull request with documentation for home-assistant.io (if applicable): None
Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices: