Support new_klap IoT devices#1580
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1580 +/- ##
==========================================
+ Coverage 93.21% 93.40% +0.18%
==========================================
Files 157 157
Lines 9819 9870 +51
Branches 1005 1013 +8
==========================================
+ Hits 9153 9219 +66
+ Misses 472 462 -10
+ Partials 194 189 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@rytilahti I haven't seen this before: 1 high Annotations Code scanning Use of a broken or weak cryptographic hashing algorithm on sensitive data is used in a hashing algorithm (SHA1) that is insecure. Is this because I modified this file and now it's looking at it? You had the code they flag already in your code, but I'm not sure why it's being flagged as a problem now. |
|
Ok, had my user test and the discovery and CLI tests were successful, but we're getting 400 error responses at first with handshake1 one or two times and then it works. Not sure what that's about yet, will keep poking around. |
|
@rytilahti Ok, here's where it gets weird. I found that it actually isn't a new KLAP, it still uses XOR. The issue isn't the actual transport, it's how it's selecting the transport. I will modify things so that in the device factory, it's still forcing XOR if the device has new_klap. It's a much easier fix and has been working consistently on my end. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements KlapTransportV3 for the new_klap protocol by adding support for a new "new_klap" parameter throughout the discovery and device configuration system. The implementation allows devices that support this new KLAP variant to be properly detected and configured to use XorTransport instead of the standard KlapTransport.
Key changes:
- Added
new_klapparameter to device configuration and discovery classes - Implemented special handling in discovery protocol to finalize devices with new_klap capability
- Updated device factory to route new_klap devices to XorTransport instead of KlapTransport
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| kasa/deviceconfig.py | Added new_klap field to DeviceConnectionParameters with custom serialization |
| kasa/discover.py | Added new_klap support to discovery protocol with special finalization logic |
| kasa/device_factory.py | Added routing logic to use XorTransport for new_klap devices |
| tests/test_discovery.py | Added comprehensive test for new_klap discovery finalization behavior |
| tests/test_deviceconfig.py | Added test configuration and serialization test for new_klap |
| tests/test_device_factory.py | Added test case for new_klap protocol selection |
| devtools/dump_devinfo.py | Updated device info capture to include new_klap parameter |
Comments suppressed due to low confidence (1)
tests/test_cli.py:1
- [nitpick] The test logic has changed from using explicit assert_not_called() and assert_called() to comparing call counts. This approach is less clear about the expected behavior and makes the test harder to understand. Consider using more descriptive assertions or adding comments explaining why call counts are being compared.
import json
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@rytilahti Ok, so you can understand what I did here. I managed to make it where during discovery, it adds a new_klap portion to the management schema if it exists. Then it checks if that exists and if so, it runs the XOR transport on the IOT protocol. Otherwise, it was trying to actually use the KLAP transport which even though it says KLAP it's XOR. This allows the device to initially be discovered, but the problem comes up where the KLAP discovery results don't allow for the segmented device types for IOT. So, I update the device one time, pull the sysinfo, then mutate that back to what a XOR discovery result would be and re-initialize the device from the sysinfo. This seems to be working quite well and reliably for me and the one user who has been testing this with me, @allenrabinovich. Old IOT and new SMART devices are working correctly for me as well. |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
A couple of quick comments, also, please do not forget to change the title of this PR when this is ready for a review :-)
ZeliardM
changed the title
|
@rytilahti Ok, I think I got everything figured out. It is actually KlapTransportV2. I have modified everything to handle this and updated all of the test coverage as well. Everything looks good on my end as well. |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
I cannot test this mysef, but it looks good on surface. I would suggest tidying it up a bit to keep the code more maintainable if and when new authentication methods pop up, but your call!
|
I tested this locally and it's working well so far. Thanks! |
|
Need anymore testing on this? I use this module within MaaSPower https://github.com/gilesknap/maaspower (I'll sync with them once this is merged to a new version to update their project), but I can run the kasa commands on my utility Linux server as a webservice as I use the power commands within Canonical's MaaS (Metal as a Service) solution which manages my k8s master nodes in my home environment. Constantly heartbeats for power states - could be a good test? |
|
I pulled the PR and it's working for me in a test venv: |
|
Any idea when this might make it to production? I had been using the patch in this post after each HomeAssistant update but after the latest 2026.2.3 the patch doesn't work anymore. |
test logI tested the PR and it's working with HS103 firmware 1.1.3. Reminder: after upgrading the plug to firmware 1.1.3, even correct credentials can trigger an authentication error. The fix was to unregister the plug from the app and reinitialize it. |
|
@genzj Thanks for the test and information. There are still some transient issues with Discover and how devices respond that can cause some issues but overall this fix works. I have several other PRs that I'm trying to get all worked on and merged before another release but I'm not sure when that will happen. |
|
any update on the status of this PR? my two HS300s suddenly stopped working with home-assistant today and I think this is why... |
ZeliardM
changed the title
|
Follow-up pass pushed in 71a4147. Covered in this pass:
Local validation:
After the push, GitHub lint and CodeQL were already green; the matrix CI/readthedocs jobs were still running when I checked. |
Implementing KlapTranportV2 to handle IoT devices with newer firmware and a new_klap flag and all associated tests hopefully.
With this PR, I will get the user to pull the specific branch for testing on their end, and I will test on my end to make sure that the old stuff still functions as well.