WIP: Convert tplink integration to use a new asyncio-enabled backend library #30719
Conversation
02a1dd5 to
8041bff
Compare
|
You're removing the retry logic for the light update. Why is that? The new kasa api doesn't seem to support it. The retry is there to avoid common issues with the bulb throwing an error instead of providing data when it gets busy. The new API will cause more calls to the bulb as it refreshes sysinfo on every update call. |
|
I removed it for the time being, as simply doing a single retry by calling another method does not necessitate the extra code branching (it just made it more difficult to follow what is going on); if retries are wanted, they should be done either by a loop or some other structure. The main point for doing this lib cleanup is to fix the underlying issues causing the need for so many requests, so my hoping is that in the end there will be no need for such retry behavior. And if it is still needed, I'll simply add it to the backend library, it should be invisible to homeassistant what is happening underneath the hood. |
|
Adding retries to the back end library is the best approach. However removing it entirely will result in failed lighting commands. This isn't a problem with the tplink plugs. I suspect the web server on the lights can only handle serial requests with spacing between them. |
|
I had some bulbs for testing last year and that's when I implemented caching in the backend library. The problem was way too many requests on them in a short period of time, mostly accessing the information that's already available after receiving the initial system information from them. But I'll keep that in mind when I get so far, in its current state this should already work just fine with the bulbs though. |
|
I have a couple of HS300s at home that have this issue which I have just been ignoring. I'll spin up this change in my deployment later on and see if it resolves my communication errors and drop some feedback. |
|
If you want to give it a try, you should use the code from this PR or stay tuned until it gets merged. This integration PR in its current state should work well enough for testing, but some changes are needed to adapt the "subdevices" to be non-polling. |
|
Any updates on this PR? After needing to perform a clean install, i'm unable to use my TP-Link devices because they can't be discovered. |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
8041bff to
624adb0
Compare
The only purpose of my HS300 is to monitor energy consumption, so I would strongly disagree. I look forward to to the new python-kasa as well, but would appreciate it not being merged before that is fixed. |
|
I’d personally rather have this integration working with a single feature missing rather then having the Kasa integration not working at all. |
|
Agreed, this integration is unusable for me right now. If you don't want to
change, don't upgrade. Just my opinion
…On Sun, Aug 9, 2020, 3:55 PM Alec Dusheck ***@***.***> wrote:
I’d personally rather have this integration working with a single feature
missing rather then having the Kasa integration not working at all.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#30719 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACGYSVMKHGNUIMYIQQJOE5TR74SORANCNFSM4KF2RNBQ>
.
|
|
Sure, easy for you to say, when you don't use that feature. For the current integration there's at least a workaround available that doesn't take away any functionality. It might be dirty, but it's been proven to work for myself, as well as others, for over 6 months. |
|
There a new feature (HS220 transition support) in python-kasa that I would love to get integrated with home assistant, but that can't happen until this PR is merged. |
|
@rytilahti Do you still need access to an HS300 strip? |
Agreed - While we all are excited about this major new update, this needs to work 100% and have ample testing. There has been a work-around for people with tplink issues for over 6 months. I have been running the modified tplink work-around code ever since December of 2019 and it works 100%. with all my tplink devices including the KP400 with multiple outlets. I would have liked to have the work-around merged, but it was never approved. |
|
Sorry for the late response, so first of all, this is not going to be merged before it has the feature parity to the current implementation (HS300 emeter readings), so no worries about that. I was previously against merging any "hacks" to fix some cases to avoid making it harder to keep this PR in sync, but considering it remains unclear when proper HS300 support lands to python-kasa, I think the way to go is to merge the fixes as long as someone takes over the ownership (or acts as second codeowner for tplink) as I don't currently have interest nor time to keep fixing it in case something breaks. Considering this is almost a rewrite of the integration, I think the safest approach is to take the current state of this PR and incorporate it inside python-kasa as When the time comes that the new integration is mature enough to be merged back to the mainline, I'll create a PR to replace the old implementation. As there is no ETA, who knows when that's going to happen, but I think two versions living side-by-side for now would be a workable compromise to fix the current unfortunate situation. What do you think about such a solution? I proposed @vangorra that he'd take up the maintainership (or at least share it with me) for the time being, but haven't heard back from him yet. If someone else wants to step up to maintain the integration, I'm open for suggestions. @loaxley that would be great as I haven't unfortunately been able to obtain access so far. The promised access didn't pan out (yet), life happens :-) |
|
@rytilahti I think what you are suggesting is the best approach. @TheGardenMonkey gets the credit for the work-around that I started using back in December 2019. I had recently updated his modification to include the recent updates since that date - here: #35460 I have a bunch of different tplink devices, and lots of them - all working perfectly using this work-around. Still running this over here currently with no issues. |
|
I am still down for assisting with this integration. I have a large number of the tp-link / kasa devices (added even more since I wrote the other PR). |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
|
This is still relevant. My tplink plugs require me to restart multiple times before they all get picked up |
|
I have given up on the integration for now, it's too unstable. Luckily I can integrate my plugs with smartthings and use them that way. Looking forward to a more stable release so I can switch back. I'd prefer to avoid using st for things that don't need them |
Can you try out the one from my PR which is still using the old library until the new one is ready...#39762 | https://github.com/TheGardenMonkey/core/tree/dev/homeassistant/components/tplink. I have been using this logic since last year without issues but would like to get some more field testing. |
|
I'll close this now, see #30719 (comment) for the future plans of this PR. As soon as the conversion into a custom component is ready, I'll post a link here for those who want to give it a shot. |
|
@rytilahti did you end up with any luck in the creation of custom component? I just picked up some KL125P2's that don't work with the existing component, but do work with |
|
Hi, unfortunately I haven't had time to clean up the code base properly and I'm currently occupied with more important projects, so this is on hold :-( |
No worries. My main goal is getting those bulbs working now, and it looks like my other PR will be accepted and we're in good shape. Whenever you get cycles to come back, happy to help with it. |
|
From looking around online, people in the HASS forums have reported success by having a script run on a PC that pings all their kasa devices. Apparently they require a ping every so often or they will fail to respond to the ARP requests HASS sends out when checking their availability. Would this be a relatively simple fix to implement? |
|
@mike391 can you link to one of those posts? |
Description:
This PR ports the tplink integration to use a maintainer-made asyncio-fork of pyhs100 called python-kasa.
The major changes which should help with the linked issues are:
update().This PR has been tested to work with the following devices so far:
It would be great to get feedback from users with other devices!
Testing this requires installing python-kasa from git: https://github.com/python-kasa/python-kasa/ .
Related issue (if applicable): Related to / potentially closes: #30357, closes #30693, closes #28912, closes #23045, closes #28590, closes #29169, obsoletes #35628, obsoletes #35460, obsoletes #34465, obsoletes #29941
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Example entry for
configuration.yaml(if applicable):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.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: