Add support for the protocol used by TAPO devices and some newer KASA devices. #552

sdb9696 · 2023-11-23T17:19:33Z

Related to #499 and #548

kasa/tapoprotocol.py

rytilahti

This is really cool, great work @sdb9696! Just wanted to give some quick feedback.

We should find a nice level of abstraction for tapo devices in general, I ordered a test bulb to help with testing and dev but I'm not going to have much time to tinker until holidays.
I am wondering how much of this is similar to the klap protocol, i.e., should it make sense to merge them or at least find a way to share some code?
I presume some of the code is sourced from other projects, are there license incompatibilities?

kasa/tapoplug.py

kasa/tapoprotocol.py

sdb9696 · 2023-11-27T17:01:43Z

This is really cool, great work @sdb9696! Just wanted to give some quick feedback.

We should find a nice level of abstraction for tapo devices in general, I ordered a test bulb to help with testing and dev but I'm not going to have much time to tinker until holidays.

Yes I was thinking the same. However I wanted to keep this PR as close to your initial #499 as possible so we didn't start off with a giant PR.

I am wondering how much of this is similar to the klap protocol, i.e., should it make sense to merge them or at least find a way to share some code?

There is some overlap, particularly around the retry logic which could be any httpx connection. Again as above I wanted to keep this PR as simple as possible. My suggestion would be to get this working for the Kasa EP25 and a TAPO plug, and then do some refactoring and code re-use in a separate PR. I'm happy to do it the other way round if you want but I thought it would be easier for you to keep things ticking along if you have less to review each time.

I presume some of the code is sourced from other projects, are there license incompatibilities?

Some of the code, i.e. the Cipher, KeyPair and the SnowflakeId came from https://github.com/petretiandrea/plugp100 which seems to have the same GNU GPL 3 license as this library so I don't think there are any incompatibilities. Let me know if you agree.

Anyway, let me know how you want to proceed, i.e. with incremental PRs or incremental commits to this PR.

rytilahti · 2023-11-27T22:14:10Z

We should find a nice level of abstraction for tapo devices in general, I ordered a test bulb to help with testing and dev but I'm not going to have much time to tinker until holidays.

Yes I was thinking the same. However I wanted to keep this PR as close to your initial #499 as possible so we didn't start off with a giant PR.

So I don't have a clear opinion on this. What I was thinking was that it would be great to have a base class straight from the beginning, as that will likely happen in the near future anyway and we know that there exists common functionalities among the devices.

Another thing that caught my mind was that maybe it's also a good idea to contain the tapo functionality inside its own package (e.g., inside python-kasa/kasa/tapo) from the get-go to avoid moving them around soon-ish. There will likely be much more files coming in when other device categories get added, so having them separated from kasa-specific code makes sense. The base class does not need to have much, just that it exists there as a place to easily start adding common functionalities when we get that far.

There is some overlap, particularly around the retry logic which could be any httpx connection. Again as above I wanted to keep this PR as simple as possible. My suggestion would be to get this working for the Kasa EP25 and a TAPO plug, and then do some refactoring and code re-use in a separate PR. I'm happy to do it the other way round if you want but I thought it would be easier for you to keep things ticking along if you have less to review each time.

Sounds reasonable to me, maybe some parts of the common functionalities can be refactored to the protocol base class to avoid inconsistencies among protocol handling, but that can wait for now.

Some of the code, i.e. the Cipher, KeyPair and the SnowflakeId came from petretiandrea/plugp100 which seems to have the same GNU GPL 3 license as this library so I don't think there are any incompatibilities. Let me know if you agree.

Yupp, that's fine. Please add a docstring note with a link to the plugp100 repository and mention the compatible, I think that should be good enough :-)

rytilahti · 2023-11-27T22:49:08Z

On another note, maybe the protocol itself should just be renamed and remain in the base package? Given it's not just used for tapo devices, but as an transport & encryption layer for the newer kasa-branded, too.

kasa/tapoplug.py

kasa/aesprotocol.py

+
+def _md5(payload: bytes) -> bytes:
+    digest = hashes.Hash(hashes.MD5())  # noqa: S303
+    digest.update(payload)


kasa/aesprotocol.py

+
+def _sha1(payload: bytes) -> str:
+    sha1_algo = hashlib.sha1()  # noqa: S324
+    sha1_algo.update(payload)


kasa/aesprotocol.py

+    @staticmethod
+    def create_key_pair(key_size: int = 1024):
+        """Create a key pair."""
+        private_key = rsa.generate_private_key(public_exponent=65537, key_size=key_size)


codecov · 2023-11-28T10:25:46Z

Codecov Report

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

Comparison is base (9de3f69) 82.29% compared to head (94981d1) 75.84%.

Files	Patch %	Lines
kasa/aesprotocol.py	25.00%	201 Missing ⚠️
kasa/tapo/tapodevice.py	54.32%	37 Missing ⚠️
kasa/tapo/tapoplug.py	56.75%	16 Missing ⚠️
kasa/discover.py	85.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #552      +/-   ##
==========================================
- Coverage   82.29%   75.84%   -6.45%     
==========================================
  Files          30       33       +3     
  Lines        2468     2861     +393     
  Branches      694      747      +53     
==========================================
+ Hits         2031     2170     +139     
- Misses        379      634     +255     
+ Partials       58       57       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sdb9696 · 2023-11-28T10:30:26Z

So I don't have a clear opinion on this. What I was thinking was that it would be great to have a base class straight from the beginning, as that will likely happen in the near future anyway and we know that there exists common functionalities among the devices.

Ok that is done. Base class of TapoDevice(SmartDevice) and TapoPlug derives from TapoDevice (not SmartPlug)

Another thing that caught my mind was that maybe it's also a good idea to contain the tapo functionality inside its own package (e.g., inside python-kasa/kasa/tapo) from the get-go to avoid moving them around soon-ish. There will likely be much more files coming in when other device categories get added, so having them separated from kasa-specific code makes sense.

That's done

Yupp, that's fine. Please add a docstring note with a link to the plugp100 repository and mention the compatible, I think that should be good enough :-)

That's done

@ezekieldas could you please check this still works with your EP25?

ezekieldas · 2023-11-28T14:31:42Z

This looks fantastic: https://pastebin.com/xRAvpT9u

Perhaps non-issue but there is that datetime.timedelta ValueError at the end.

korishev · 2023-11-28T15:28:59Z

I have several of these new EP25 plugs and can verify that this at least appears to be working:

Hardware Version: 2.6
Firmware Version: 1.0.1 Build 230614 - Rel. 150219

I am getting a similar ValueError to @ezekieldas at the end of output.

Happy to help test, debug, etc. (and send one or two devices to devs for testing. If they don't work with Home Assistant, they are useless to me)

sdb9696 · 2023-11-28T15:35:10Z

The ValueError should be fixed if you take the latest version of this PR updated just now

ezekieldas · 2023-11-28T16:13:29Z

Yes, so I did another checkout of pull552 and this did correct the ValueError:

        [ ... ]
	== Device specific information ==
	is_hw_v2: False
	overheated: False
	signal_level: 3
	auto_off_status: off
	auto_off_remain_time: 0
	SSID: foobarXYZ
	On since: 2023-11-18 05:04:27

	== Modules ==

bdraco · 2023-11-28T19:26:13Z

I'm going to order a device to have something to test with. Any idea if the P125M will work even though it supports matter? I can't see to find any of them that don't support matter for sale and will ship to Hawaii

ezekieldas · 2023-11-28T19:40:58Z

Any idea if the P125M will work even though it supports matter?

I haven't used or researched these, nor am I familiar with matter. However kp125m is listed on the integrations page. See the comment: supported via Matter, but without energy monitoring features

sdb9696 · 2023-11-28T19:41:36Z

I’m not sure I’m afraid. From looking at decompiled Kasa & TAPO .apk I’d say they support both but it’s difficult to be sure. I guess the worst case is we end up with a Matter protocol ;)

bdraco · 2023-11-28T19:46:45Z

I ordered the EP25 and the kp125m

I figure one of them will be useful for testing

rytilahti · 2023-11-28T21:12:52Z

To my understanding, based on some issue comments), the matter-based devices are using the "tapo" protocol (unsure if it's just the transport or also for the command set) as a secondary communications protocol. The emeter features do not work over the matter at the moment, as I think only the recently announced upcoming 1.2 (or 1.3?) has support for such.

For EP25, it seems that the new hw version (v4?) is the one that has moved to use the AES encryption, and I'm quite confident others will follow as it just makes sense :-)

edit: KP125M seems to use KLAP & tapo commands as per #450 (comment)

rytilahti

So I received my test bulb today and my pending deadline was postponed a bit, so I had some time to play around with it :-)

Here are some comments, I'll create a draft PR based on this PR that adds support for tapo bulbs. Some notes based on my experiments:

Adding support for the bulbs felt really easy, I think the effort put into building a good foundation and abstractions were useful! :-)
get_<x>_info and set_<x>_info come in pairs, so when get_device_info report on brightness, it can be set using set_device_info much like device_on.
hue, saturation and brightness are stored in separate fields, so I suppose multi assignments are possible. I haven't yet tested this out, but will do so prior creating the PR.
Some communications feel rather sluggish, due to multiple queries (device state, then the emeter & the usage). Maybe these could be condensed into a single request? Just a thought..

kasa/tapo/tapodevice.py

rytilahti · 2023-11-28T22:22:33Z

sdb9696#14 adds support for bulbs. Maybe it would make more sense to base it on top of python-kasa master instead of the branch for the PR at hand to keep the merges small, but github ui's diff view makes it easier to see how few changes were required for basic functionalities :-)

sdb9696 · 2023-11-29T12:12:46Z

Unbelievably, overnight my Tapo P110 updated it's firmware to the KLAP encryption. It's now running sw_version 1.3.0 Build 230905 Rel.152200.

{'device_id': '**', 'owner': '**', 'device_type': 'SMART.TAPOPLUG', 'device_model': 'P110(UK)', 'ip': '**', 'mac': '**', 'is_support_iot_cloud': True, 'obd_src': 'tplink', 'factory_default': False, 'mgt_encrypt_schm': {'is_support_https': False, 'encrypt_type': 'KLAP', 'http_port': 80, 'lv': 2}}}

As a result I set to looking at what is needed to support this configuration. I've created a draft PR to support TAPO/KLAP, but I don't know if I've broken the AES transport in the process. I no longer have a device with this configuration :(.

@ezekieldas would you mind testing your EP25 with this new PR to see if it still works with your device.

@rytilahti there are more comments in the other PR regarding the approach. I've also addressed all your previous review comments in this PR.

ezekieldas · 2023-11-29T14:02:56Z

Here are the results from #557: https://pastebin.com/XPpDRS3e It ends with an AuthenticationException.

Just to be certain, I compared same args/creds used in this test with the previous test on #552, and that remains successful.

rytilahti

Would you mind adding some tests to make it easier to review the code?
Also, should this get merged before it makes sense to review #557?

kasa/tapo/tapodevice.py

rytilahti · 2023-11-29T15:05:29Z

kasa/tapo/tapoplug.py

+        self._energy = await self.protocol.query("get_energy_usage")
+        self._emeter = await self.protocol.query("get_current_power")


The bulb seems to support these commands, too, but I'm not sure if we can detect the availability using component_nego or some other way.

Here's the response for the from the bulb (kasa -d --type tapobulb --username $KASA_TAPO_EMAIL --password $KASA_TAPO_PASSWORD --host 192.168.xx.xx raw-command "" component_nego):

<< {'component_list': [{'id': 'device', 'ver_code': 2}, {'id': 'firmware', 'ver_code': 2}, {'id': 'quick_setup', 'ver_code': 3}, {'id': 'inherit', 'ver_code': 1}, {'id': 'time', 'ver_code': 1}, {'id': 'wireless', 'ver_code': 1}, {'id': 'schedule', 'ver_code': 2}, {'id': 'countdown', 'ver_code': 2}, {'id': 'antitheft', 'ver_code': 1}, {'id': 'account', 'ver_code': 1}, {'id': 'synchronize', 'ver_code': 1}, {'id': 'sunrise_sunset', 'ver_code': 1}, {'id': 'cloud_connect', 'ver_code': 1}, {'id': 'default_states', 'ver_code': 1}, {'id': 'preset', 'ver_code': 1}, {'id': 'brightness', 'ver_code': 1}, {'id': 'color', 'ver_code': 1}, {'id': 'color_temperature', 'ver_code': 1}, {'id': 'auto_light', 'ver_code': 1}, {'id': 'on_off_gradually', 'ver_code': 1}, {'id': 'device_local_time', 'ver_code': 1}, {'id': 'light_effect', 'ver_code': 1}, {'id': 'iot_cloud', 'ver_code': 1}, {'id': 'bulb_quick_control', 'ver_code': 1}]} {"component_list": }

I updated the list to the draft bulb PR, so it's easily available.

kasa/aesprotocol.py

rytilahti · 2023-11-29T15:30:30Z

Please add a debug output for the query, that'll make it easier to debug issues when a query is failing :-)

diff --git a/kasa/aesprotocol.py b/kasa/aesprotocol.py
index f74ae54..f357de4 100644
--- a/kasa/aesprotocol.py
+++ b/kasa/aesprotocol.py
@@ -304,6 +304,12 @@ class TPLinkAes(TPLinkProtocol):
         raise SmartDeviceException("Query reached somehow to unreachable")
 
     async def _execute_query(self, request: Union[str, Dict], retry_count: int) -> Dict:
+        _LOGGER.debug(
+            "%s >> %s",
+            self.host,
+            _LOGGER.isEnabledFor(logging.DEBUG) and pf(request),
+        )
+
         if not self.http_client:
             self.http_client = httpx.AsyncClient()

sdb9696 · 2023-11-29T15:33:06Z

Here are the results from #557: https://pastebin.com/XPpDRS3e It ends with an AuthenticationException.

Just to be certain, I compared same args/creds used in this test with the previous test on #552, and that remains successful.

Hi @ezekieldas. I think I've fixed it, could you try again?

ezekieldas · 2023-11-29T15:50:07Z

It definitely looks fixed! https://pastebin.com/66Z1XGR5

I do a quick scan (human, me) of the output each time, and here I see the val for timezone looking a bit funky (see line 156):
Time: 2023-11-29 07:36:36-08:00 (tz: {'timezone': datetime.timezone(datetime.timedelta(days=-1, seconds=57600))}

...but other than that, it looks good!

sdb9696 · 2023-11-29T18:07:22Z

Some communications feel rather sluggish, due to multiple queries (device state, then the emeter & the usage). Maybe these could be condensed into a single request? Just a thought..

I've just figured out how to do multiple requests in one go. I'll add support for this in a later PR if that's ok?

rytilahti · 2023-11-29T18:12:29Z

I've just figured out how to do multiple requests in one go. I'll add support for this in a later PR if that's ok?

That's fine, let's get preliminary support in and we can then iterate and optimize 👍

sdb9696 · 2023-11-29T18:44:18Z

It definitely looks fixed! https://pastebin.com/66Z1XGR5

I do a quick scan (human, me) of the output each time, and here I see the val for timezone looking a bit funky (see line 156): Time: 2023-11-29 07:36:36-08:00 (tz: {'timezone': datetime.timezone(datetime.timedelta(days=-1, seconds=57600))}

...but other than that, it looks good!

Thank you very much!

sdb9696 · 2023-11-29T18:57:13Z

Would you mind adding some tests to make it easier to review the code? Also, should this get merged before it makes sense to review #557?

The trouble is I can no longer test this PR with a device myself because my P110 upgraded to KLAP overnight. Also the changes in #557 are going to make it much easier to write the FakeTransport for testing the TAPO protocol and test using the "new" fixtures with TAPO responses. I don't suppose we could hold off on the tests until #557? It will save me a lot of time and tests is my number 1 priority at the moment (as you may have guessed from #556)

rytilahti · 2023-11-29T20:06:35Z

Sounds reasonable! I'll go through this one later tonight and see if there is something that should be handled before merging this. But I think in the end we will just merge this as is and iterate on later PRs 👍

rytilahti · 2023-11-29T21:30:44Z

@sdb9696 could you update the title and the description to be more descriptive? Doesn't need to be anything fancy, but given the size and the importance of this change, it would be great to have descriptive ones for anyone reading the changelogs/commit logs. I will now check if there's something that should be changed prior the merge.

kasa/discover.py

kasa/tapo/tapodevice.py

kasa/tapo/tapoplug.py

kasa/tests/test_discovery.py

sdb9696 · 2023-11-30T11:06:10Z

Now updated with changes or replies. N.B. I have squashed all the commits except the latest to make rebasing #557 on top of this PR easier.

rytilahti

This looks good to go, thanks @sdb9696! 💯

sdb9696 · 2023-11-30T12:21:30Z

Great, thanks @rytilahti! Onto #557 :)

github-advanced-security bot found potential problems Nov 23, 2023

View reviewed changes

kasa/tapoprotocol.py Fixed Show resolved Hide resolved

kasa/tapoprotocol.py Fixed Show fixed Hide fixed

kasa/tapoprotocol.py Fixed Show fixed Hide fixed

rytilahti reviewed Nov 27, 2023

View reviewed changes

kasa/tapoplug.py Outdated Show resolved Hide resolved

kasa/tapoplug.py Outdated Show resolved Hide resolved

kasa/tapoprotocol.py Outdated Show resolved Hide resolved

kasa/tapoprotocol.py Outdated Show resolved Hide resolved

rytilahti added the enhancement New feature or request label Nov 27, 2023

This was referenced Nov 27, 2023

WIP: Add preliminary support for tapo plugs #499

Closed

Unable to find tplink kasa devices on network home-assistant/core#93879

Closed

rytilahti reviewed Nov 27, 2023

View reviewed changes

kasa/tapoplug.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Nov 28, 2023

View reviewed changes

sdb9696 force-pushed the add_tapo_support branch 2 times, most recently from f5e320d to d5200a7 Compare November 28, 2023 13:47

sdb9696 force-pushed the add_tapo_support branch from d5200a7 to 9f5580b Compare November 28, 2023 15:33

sdb9696 marked this pull request as ready for review November 28, 2023 15:55

rytilahti reviewed Nov 28, 2023

View reviewed changes

kasa/tapo/tapodevice.py Outdated Show resolved Hide resolved

kasa/tapo/tapodevice.py Outdated Show resolved Hide resolved

kasa/tapo/tapodevice.py Outdated Show resolved Hide resolved

kasa/tapo/tapodevice.py Outdated Show resolved Hide resolved

rytilahti mentioned this pull request Nov 28, 2023

KP 125M / support for matter devices #450

Closed

sdb9696 force-pushed the add_tapo_support branch from dbe58bb to 1008bf9 Compare November 29, 2023 09:56

sdb9696 mentioned this pull request Nov 29, 2023

Add klap support for TAPO protocol by splitting out Transports and Protocols #557

Merged

rytilahti reviewed Nov 29, 2023

View reviewed changes

rytilahti mentioned this pull request Nov 29, 2023

Add support for tapo bulbs #558

Merged

rytilahti requested changes Nov 29, 2023

View reviewed changes

sdb9696 force-pushed the add_tapo_support branch from 43f5b8e to 3964415 Compare November 30, 2023 08:12

sdb9696 changed the title ~~Add Tapo protocol~~ Add support for the protocol used by TAPO devices and some newer KASA devices. Nov 30, 2023

sdb9696 force-pushed the add_tapo_support branch from 3964415 to 24586de Compare November 30, 2023 10:47

sdb9696 added 2 commits November 30, 2023 11:01

Add Tapo protocol support

ef4f631

Update get_device_instance and test_unsupported following review

94981d1

sdb9696 force-pushed the add_tapo_support branch from 24586de to 94981d1 Compare November 30, 2023 11:03

sdb9696 requested a review from rytilahti November 30, 2023 11:06

rytilahti approved these changes Nov 30, 2023

View reviewed changes

rytilahti merged commit 63d64ad into python-kasa:master Nov 30, 2023

sdb9696 mentioned this pull request Nov 30, 2023

Add klap lv2 #550

Closed

sdb9696 deleted the add_tapo_support branch December 6, 2023 19:29

		self._energy = await self.protocol.query("get_energy_usage")
		self._emeter = await self.protocol.query("get_current_power")

Add support for the protocol used by TAPO devices and some newer KASA devices. #552

Add support for the protocol used by TAPO devices and some newer KASA devices. #552

Conversation

sdb9696 commented Nov 23, 2023

rytilahti left a comment

Choose a reason for hiding this comment

sdb9696 commented Nov 27, 2023

rytilahti commented Nov 27, 2023 • edited Loading

rytilahti commented Nov 27, 2023

Check failure

Check failure

Check failure

codecov bot commented Nov 28, 2023 • edited Loading

Codecov Report

sdb9696 commented Nov 28, 2023

ezekieldas commented Nov 28, 2023

korishev commented Nov 28, 2023

sdb9696 commented Nov 28, 2023

ezekieldas commented Nov 28, 2023

bdraco commented Nov 28, 2023

ezekieldas commented Nov 28, 2023 • edited Loading

sdb9696 commented Nov 28, 2023

bdraco commented Nov 28, 2023

rytilahti commented Nov 28, 2023 • edited Loading

rytilahti left a comment

Choose a reason for hiding this comment

rytilahti commented Nov 28, 2023

sdb9696 commented Nov 29, 2023

ezekieldas commented Nov 29, 2023

rytilahti left a comment

Choose a reason for hiding this comment

rytilahti Nov 29, 2023

Choose a reason for hiding this comment

rytilahti commented Nov 29, 2023

sdb9696 commented Nov 29, 2023

ezekieldas commented Nov 29, 2023

sdb9696 commented Nov 29, 2023

rytilahti commented Nov 29, 2023

sdb9696 commented Nov 29, 2023

sdb9696 commented Nov 29, 2023 • edited Loading

rytilahti commented Nov 29, 2023

rytilahti commented Nov 29, 2023 • edited Loading

sdb9696 commented Nov 30, 2023

rytilahti left a comment

Choose a reason for hiding this comment

sdb9696 commented Nov 30, 2023

rytilahti commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 28, 2023 •

edited

Loading

ezekieldas commented Nov 28, 2023 •

edited

Loading

rytilahti commented Nov 28, 2023 •

edited

Loading

sdb9696 commented Nov 29, 2023 •

edited

Loading

rytilahti commented Nov 29, 2023 •

edited

Loading