I'm using my own system rather than HomeAssistant so my code is going to be more complicated to post, but here's my working function. I interrogate the HS300 strip first and get the plug objects and then get_emeter_realtime() for each of them.
The resulting data is identical to what I would retrieve from pyHS100 (which I'm migrating from for asyncio support):
Hmm, okay, interesting... I'm wondering if it's possible to define multiple children for those requests to get the information from all child sockets. Otherwise this would bump the number of necessary I/O requests by six (which probably is still workable, compared to previous 18 requests).
It also appears that if you do strip.get_emeter_realtime(), it's only returning data for for the first plug (strip.plugs[0]), not the aggregate for the entire strip. Only walking the individual plugs and requesting the emeter and then adding them up seems to work for that.
I did some rethinking on how to approach the per outlet emeter, and it is most likely not possible to request it from all the plugs at once. So the way to go forward will be to do those requests per plug basis, which will increase the requests per cycle to 7. However, as the HS300 support works (at least sometimes) with current 19 requests, I don't think this will be a problem. If that's the case, maybe the emeter information needs to be made configurable [in homeassistant].
there are two potential approaches:
Make it necessary to explicitly use get_emeter_realtime() for SmartStrip devices, or
Execute the fetch on each child device if the device claims to support EME.
The former makes it clear that there'll be I/O operations needed while the second one hides that as an implementation detail.
The former is "ugly" as we are exposing emeter_realtime etc. properties that are delivering incorrect information, while the latter will cause up to 6 more requests per update cycle. So for the sake of consistency I'd propose the latter. This can be made configurable if 7 requests per update cycle is considered problematic, but I think it's a non-issue.
Any progress on this bug? For what it's worth, I vote for suggestion 2. above.
Also hoping you're able to get #35137 integrated into Home Assistant soon. Just getting the switches to work correctly would be a plus, but it would be really nice to have the power readings for all 6 devices on the HS300 power strip too.
No updates on this one so far, but the second way is likely the way to go. I'm waiting for access to a test device before I'll touch that code path. As that PR is going to be almost like a rewrite, I'd prefer to get it right.
I'd vote on option 2 as well, but include emeters in the strip.update() process, rather than making it a separate call -- or at least provide an option to do so. Then, similarly to usage with other aspects of the strip, the values are cached from the last update().
Yes, the option 2 is the way to go, and all subdevice emeter requests should be done automatically inside the update(). I'd guess that simply adding very short sleeps in-between (or at least limiting the number of concurrent requests) would be enough, when even needed (the pyhs100 implementation does 18 queries, this would only be 7 per update cycle and may work even without any sleeps). So if anyone with access to a HS300 wants to create a PR to implement this, I'm more than happy to help by doing code review & giving feedback/information where I can.
Some things to consider:
This all should be transparent to downstream users (maybe with an option to disable the emeter updates), the API should stay as it is.
The return value for SmartStripPlug.has_emeter should be true only when features of the sysinfo query contains EME.
The cache will require some adjustments, the emeter responses could be saved under _sys_info dict keyed with the id.
The emeter API should follow the SmartDevice interface. The emeter_* properties on the main device should return the sum of child devices properties.
Getting access to a test device didn't pan out, nor do I have time to work on this at the moment, so I'm hoping that someone with a device could make this happen :-)
@rytilahti Hey! I'm super-interested in this, as well as getting homeassistant integration working. I have an HS300.
I can't guarantee that I'll get to it, but if I develop this, should I just go ahead and do a PR?
Also, a quick side question -- pyHS100 has some significant issues talking with the HS300 and some other devices -- namely, one in ten queries just fails for reasons unclear. With python-kasa, From a small test locally (while True: await strip.update()) it seems to have been fixed.
Can you confirm? I know people had problems with other devices as well.
Locally, I edited pyHS100 to automatically retry three times whenever there's a failure, and that fixes the homeassistant issue for me. I'd still like to move to python-kasa, though, and am curious if that issue was addressed, or just coincidentally isn't showing up for me with python-kasa.
Adding support for HS300 emeter is more than welcome, so feel free to go and create a PR whenever you want. I'm happy to do code review and help to make it happen. See my previous comment just above yours for some details to keep in mind.
As to your question related to problems caused by number of requests, one of the improvements in python-kasa is its improved query handling. The problem with the current homeassistant integration (and pyhs100) is that each socket is updated separately even when a single request would suffice. On top of that, the emeter information (current, today, monthly) is also queried separately for each socket, which raises the number of requests to the point where the device simply stops answering them.
On integrating python-kasa to homeassistant, I have a custom component ("tplinkng") based on my old pull request (home-assistant/core#30719). There are still some issues to fix before I'll feel comfortable to create a PR (to include it to python-kasa, not to homeassistant repo) for it. As this will work completely independent from the official integration, both official and the custom one can be used in tandem as long as it is necessary to get everything into a publishable state.
Ah! So it's just the number of requests that's doing it. That makes sense that the new code would be better on that, I noticed that it was structured better -- reducing needed communication, not doing net calls on repr(), and some other good stuff.
I did take note of your points above, and they all seem natural and sound, except perhaps "the API should stay as it is." We want to expose it to the downstream user, but that requires a change in some way. I was thinking a method like strip.enable_emeter(enabled=True), or, if it's preferable to only update at a particular rate, perhaps strip.emeter_update_frequency, which could be a settable property that is >=0 | None -- and then document that in SmartStrip and SmartStripPlug docstrings.
Activity
rytilahti commented on May 24, 2020
Hmm, looking at this: https://github.com/python-kasa/python-kasa/blob/master/kasa/tests/fixtures/HS300(US)_1.0_real.json only the whole device information seems to be accessible under the get_realtime. Could you enable debug logging and check how the request & its output looks like for child devices?
tagdara commented on May 26, 2020
I'm using my own system rather than HomeAssistant so my code is going to be more complicated to post, but here's my working function. I interrogate the HS300 strip first and get the plug objects and then get_emeter_realtime() for each of them.
The resulting data is identical to what I would retrieve from pyHS100 (which I'm migrating from for asyncio support):
{ current: 0.017527, voltage: 124.842653, power: 0, total: 11.839 }
The function I use to get the plug data is here. It's a little bit of a data retrofit right now but it works:
async def get_plug(self, plug, parent=None, update=True): try: self.log.debug('.. updating %s' % plug.device_id) if update and "_" not in plug.device_id: await plug.update() if "_" in plug.device_id: short_id=plug.device_id.split("_")[1] else: short_id=plug.device_id plug_data={"hw_info": plug.hw_info, "parent_id": parent, "id": short_id, "alias": plug.alias, "led": plug.led, "is_on": plug.is_on==1, "model":plug.model} if plug.is_on: plug_data["on_since"]=plug.on_since.isoformat() plug_data["energy"]=await plug.get_emeter_realtime() return plug_data except kasa.smartdevice.SmartDeviceException: self.log.error('!! error getting plug (communication error)') except: self.log.error('.. error getting plug', exc_info=True)rytilahti commented on May 27, 2020
Hmm, okay, interesting... I'm wondering if it's possible to define multiple children for those requests to get the information from all child sockets. Otherwise this would bump the number of necessary I/O requests by six (which probably is still workable, compared to previous 18 requests).
tagdara commented on May 27, 2020
It also appears that if you do strip.get_emeter_realtime(), it's only returning data for for the first plug (strip.plugs[0]), not the aggregate for the entire strip. Only walking the individual plugs and requesting the emeter and then adding them up seems to work for that.
ndorf commented on Jun 17, 2020
The "Current State" reported by the kasa command-line utility for my HS300 is definitely for the first plug only, not the aggregate.
rytilahti commented on Jul 2, 2020
Based on my own comment at home-assistant/core#28912 (comment):
there are two potential approaches:
The former makes it clear that there'll be I/O operations needed while the second one hides that as an implementation detail.
The former is "ugly" as we are exposing emeter_realtime etc. properties that are delivering incorrect information, while the latter will cause up to 6 more requests per update cycle. So for the sake of consistency I'd propose the latter. This can be made configurable if 7 requests per update cycle is considered problematic, but I think it's a non-issue.
Opinions?
garyt72 commented on Aug 2, 2020
Any progress on this bug? For what it's worth, I vote for suggestion 2. above.
Also hoping you're able to get #35137 integrated into Home Assistant soon. Just getting the switches to work correctly would be a plus, but it would be really nice to have the power readings for all 6 devices on the HS300 power strip too.
rytilahti commented on Aug 6, 2020
No updates on this one so far, but the second way is likely the way to go. I'm waiting for access to a test device before I'll touch that code path. As that PR is going to be almost like a rewrite, I'd prefer to get it right.
jasper9 commented on Oct 19, 2020
I just noticed this functionality isn't included (child plugs showing power usage). Is there a technical blocker for this not being implemented?
eode commented on Nov 6, 2020
I'd vote on option 2 as well, but include emeters in the strip.update() process, rather than making it a separate call -- or at least provide an option to do so. Then, similarly to usage with other aspects of the strip, the values are cached from the last update().
rytilahti commented on Nov 6, 2020
Yes, the option 2 is the way to go, and all subdevice emeter requests should be done automatically inside the update(). I'd guess that simply adding very short sleeps in-between (or at least limiting the number of concurrent requests) would be enough, when even needed (the pyhs100 implementation does 18 queries, this would only be 7 per update cycle and may work even without any sleeps). So if anyone with access to a HS300 wants to create a PR to implement this, I'm more than happy to help by doing code review & giving feedback/information where I can.
Some things to consider:
Getting access to a test device didn't pan out, nor do I have time to work on this at the moment, so I'm hoping that someone with a device could make this happen :-)
eode commented on Nov 13, 2020
@rytilahti Hey! I'm super-interested in this, as well as getting homeassistant integration working. I have an HS300.
I can't guarantee that I'll get to it, but if I develop this, should I just go ahead and do a PR?
Also, a quick side question -- pyHS100 has some significant issues talking with the HS300 and some other devices -- namely, one in ten queries just fails for reasons unclear. With python-kasa, From a small test locally (while True: await strip.update()) it seems to have been fixed.
Can you confirm? I know people had problems with other devices as well.
Locally, I edited pyHS100 to automatically retry three times whenever there's a failure, and that fixes the homeassistant issue for me. I'd still like to move to python-kasa, though, and am curious if that issue was addressed, or just coincidentally isn't showing up for me with python-kasa.
rytilahti commented on Nov 13, 2020
Hi @eode,
Adding support for HS300 emeter is more than welcome, so feel free to go and create a PR whenever you want. I'm happy to do code review and help to make it happen. See my previous comment just above yours for some details to keep in mind.
As to your question related to problems caused by number of requests, one of the improvements in python-kasa is its improved query handling. The problem with the current homeassistant integration (and pyhs100) is that each socket is updated separately even when a single request would suffice. On top of that, the emeter information (current, today, monthly) is also queried separately for each socket, which raises the number of requests to the point where the device simply stops answering them.
On integrating python-kasa to homeassistant, I have a custom component ("tplinkng") based on my old pull request (home-assistant/core#30719). There are still some issues to fix before I'll feel comfortable to create a PR (to include it to python-kasa, not to homeassistant repo) for it. As this will work completely independent from the official integration, both official and the custom one can be used in tandem as long as it is necessary to get everything into a publishable state.
eode commented on Nov 14, 2020
Ah! So it's just the number of requests that's doing it. That makes sense that the new code would be better on that, I noticed that it was structured better -- reducing needed communication, not doing net calls on repr(), and some other good stuff.
I did take note of your points above, and they all seem natural and sound, except perhaps "the API should stay as it is." We want to expose it to the downstream user, but that requires a change in some way. I was thinking a method like strip.enable_emeter(enabled=True), or, if it's preferable to only update at a particular rate, perhaps strip.emeter_update_frequency, which could be a settable property that is >=0 | None -- and then document that in SmartStrip and SmartStripPlug docstrings.
Re: tplinkng -- thanks for the info.
brendandburns commented on Apr 5, 2021
fwiw, I have a PR here:
#154
which fixes this for me on my HS300.
eode commented on Apr 26, 2021
I got pulled into other projects, thanks @brendandburns .
Release 0.4.0.dev4
Release 0.4.0.dev4
Release 0.4.0.dev4 (#210)
Release 0.4.0.dev4 (python-kasa#210)
Release 0.4.0.dev4 (python-kasa#210)