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.

• 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 :-)

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.

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%.

@@            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.

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?

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

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

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

1. Hardware Version: 2.6
2. 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)

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

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 ==

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

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

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 ;)

I ordered the EP25 and the kp125m

I figure one of them will be useful for testing

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)

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 :-)

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.

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.

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()

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?

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!

• 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?

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 👍

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!

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)

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 👍

@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.

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.

Great, thanks @rytilahti! Onto #557 :)