Activity

1. rytilahti commented on Oct 14, 2024

   

   rytilahti

   on Oct 14, 2024

   Member

   More actions

   Alas the error code is very generic, so figuring out why it isn't working is a bit tricky. I'm wondering if this could be related to #1145. Are you using version 0.7.5? If so, could you try if downgrading to 0.7.4 fixes the issue?

   Alternatively, if you know how to do a packet capture between the device and your phone, you could use a script in the devtools directory to decrypt the communications to find out what requests the app is sending to the device.
2. Fulch36 commented on Oct 22, 2024

   

   Fulch36

   on Oct 22, 2024

   Contributor

   More actions

   I've also purchased one of these units and have managed to get it working but it does require some code changes. There seems to be two separate but related issues.

   ---

   Firstly, current_consumption isn't read from the correct place for this device.
   

   python-kasa/kasa/smart/modules/energy.py

   Lines 27 to 31 in 53fafc3

   	def current_consumption(self) -> float | None:
   	"""Current power in watts."""
   	if (power := self.energy.get("current_power")) is not None:
   	return power / 1_000
   	return None

   My interpretation of the above is that the value for current_consumption would normally be returned as part of the response for a get_energy_usage request - is that correct? That strikes me as odd given the below appears to imply that it's necessary, in newer versions of the protocol, to send a get_current_power request to get that information.

   python-kasa/kasa/smart/modules/energy.py

   Lines 18 to 23 in 53fafc3

   	req = {
   	"get_energy_usage": None,
   	}
   	if self.supported_version > 1:
   	req["get_current_power"] = None
   	return req

   Should there not also be an extra if clause in the getter, to correspond to the difference in required requests, as determined by the supported protocol?

   Indeed, for the P304M in particular, the current_consumption must be read from the response to the get_current_power request, e.g.

   self.data.get("get_current_power").get("current_power")

   It also doesn't require the division by 1000, the value returned by this device is in W, as determined by comparing the JSON response to the value shown in the app.

   ---

   The second issue seems a bit trickier to me but that may just be down to my lack of familiarity with the codebase. This device seems to be the first example of a device using the Smart protocol, in which the parent doesn't support energy monitoring but the child devices, do support it.

   I can see that there is an existing practice of implementing a _check_supported method to filter properties in these instances. Some modules, such as autooff, seem to handle this very neatly:
   

   python-kasa/kasa/smart/modules/autooff.py

   Lines 99 to 105 in 53fafc3

   	async def _check_supported(self):
   	"""Additional check to see if the module is supported by the device.
   	Parent devices that report components of children such as P300 will not have
   	the auto_off_status is sysinfo.
   	"""
   	return "auto_off_status" in self._device.sys_info

   However, I cannot see any differences in the device_info between the parent and child devices, that are specific to energy monitoring. To get this to work at my end, I've just copied the _check_supported method from autooff, into the energy module but I know that isn't the "right" way to solve it.

   @rytilahti Please advise on how you'd normally proceed at this point. If it'd be useful, I'm happy to share the debug info from the kasa CLI? I don't think I understand enough of the context here to raise a useful PR at this stage.
3. Fulch36 commented on Oct 22, 2024

   

   Fulch36

   on Oct 22, 2024

   Contributor

   More actions

   Okay, I've now discovered the test fixtures in the repo which is helping to clear up some of the aspects of the code I found confusing. In particular the P110(UK) does indeed seem to report current_power in two different places: the response for get_energy_usage and the response for get_current_power. The P304M only reports it as part of get_current_power.

   Can I assume a PR with the output from the dump_devinfo script would be a useful starting point?
4. rytilahti commented on Oct 22, 2024

   

   rytilahti

   on Oct 22, 2024

   Member

   More actions

   Hi @Fulch36 and thanks for your insights! Just to reply you briefly right away, creating a fixture file would indeed be a good first step to understand the device and the necessary changes. You can create one by following the instructions at https://python-kasa.readthedocs.io/en/stable/contribute.html#contributing-fixture-files .

   I think all all existing devices support also both of those commands, but the get_energy_usage was chosen as it reports much more information than the get_current_power. Here's how a P115 responds to those calls:

   $ kasa --host 192.168.xx.xx command get_current_power
   {"get_current_power": {"current_power": 4}}
   $ kasa --host 192.168.xx.xx command get_energy_usage
   {"get_energy_usage": {"today_runtime": 901, "month_runtime": 31140, "today_energy": 68, "month_energy": 2797, "local_time": "2024-10-22 15:01:11", "electricity_charge": [0, 0, 0], "current_power": 3900}}
   

   Now, you mentioned that the information is per-socket, so this device may need a different handling for this. At the moment, we are not performing separate update queries for child devices as part of the update cycle. This is mostly due to two reasons: 1) there has been no need for that so far and 2) child devs are commonly separate devices, e.g., battery-powered sensors for hubs.

   Btw, you can try to execute the get_energy_usage on individual sockets using kasa command --child <device id> get_energy_usage or kasa command --name 'socket alias' get_energy_usage commands. Feel free to experiment and report back! The simplest way is to define KASA_USERNAME and KASA_PASSWORD environment variables, but you can also use kasa --username <email> --password <pass> command.
5. Fulch36 commented on Oct 22, 2024

   

   Fulch36

   on Oct 22, 2024

   Contributor

   More actions

   Thanks for the response @rytilahti!

   I've created a branch on my fork with the fixture for the P304M here. It's worth noting that I got quite a lot of errors from the dump_devinfo script when I ran it against the device, let me know if sharing that output would be useful.

   I've also pushed the current set of changes to get the energy module working with the P304M to a WIP branch, you can find that here.

   With those changes included, here is the response I get from the commands you've provided:

   $ kasa --host 192.168.XX.XX command get_current_power --child "Smart Plug 1"
   {"get_current_power": {"current_power": 1}}
   $ kasa --host 192.168.XX.XX command get_energy_usage --child "Smart Plug 1"
   {"get_energy_usage": {"today_runtime": 0, "month_runtime": 220, "today_energy": 0, "month_energy": 14, "local_time": "2024-10-22 18:21:50", "electricity_charge": [0, 0, 0]}}
   

   As you can see, the value for current_power is only returned in the get_current_power response on this device.
6. rytilahti commented on Oct 22, 2024

   

   rytilahti

   on Oct 22, 2024

   Member

   More actions

   If you don't mind, please create pull requests. That'll make it easier to see the changes :-)
7. Fulch36 commented on Oct 22, 2024

   

   Fulch36

   on Oct 22, 2024

   Contributor

   More actions

   As requested, have created two draft PRs to allow for changes to be reviewed.

   Reacted by Teemu R.
8. rytilahti commented on Oct 22, 2024

   

   rytilahti

   on Oct 22, 2024

   Member

   More actions

   It's worth noting that I got quite a lot of errors from the dump_devinfo script when I ran it against the device, let me know if sharing that output would be useful.

   Thanks for the PRs! The dump_devinfo script aims to collect as much information as possible to help future developments so errors are expected, so no need for logs if nothing seems out of place.
9. rytilahti commented on Oct 28, 2024

   

   rytilahti

   on Oct 28, 2024

   Member

   More actions

   Fixed with #1186 thanks to @Fulch36!
10. rytilahti

    closed this as completedon Oct 28, 2024