Skip to content

Conversation

@sbytnar
Copy link
Contributor

@sbytnar sbytnar commented Dec 1, 2023

Hi. I have a KP125M, and these changes make it produce plausible data that matches the Kasa mobile app.
I know these changes may conflict with how Tapo energy monitoring may work, but hopefully this will help other developers see what's needed to get KP125M energy monitoring operational.
Thanks for the latest Tapo protocol integration!

Example output with these changes:

$ kasa --type tapoplug --target $IPADDR --username $USERNAME --password $PASSWORD state
....
	== Current State ==

	<EmeterStatus power=1.35 voltage=None current=None total=0.582>

	== Modules ==
	+ <Module Emeter (emeter) for REDACTED>
$ kasa --type tapoplug --target $IPADDR --username $USERNAME --password $PASSWORD emeter
== Emeter ==
Current: None A
Voltage: None V
Power: 1.35 W
Total consumption: 0.582 kWh
Today: 0.582 kWh
This month: 5.737 kWh

Copy link
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly fine for me, thanks for the PR @sbytnar! Please run pre-commit run -a to fix the linting issues.

Ping @sdb9696, any thoughts on this?

self._device_type = DeviceType.Plug
self.modules: Dict[str, Any] = {}
self.emeter_type = "emeter"
self.modules["emeter"] = Emeter(self, self.emeter_type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to rethink the whole modules system for "tapo" devices separately from the legacy protocol, but I suppose this is fine for now.

@rytilahti rytilahti added the enhancement New feature or request label Dec 1, 2023
@sdb9696
Copy link
Collaborator

sdb9696 commented Dec 1, 2023

Looks good to me. Might be good to have some tests but it's probably better to wait for #557 for that because it was quite a lift to get the test framework working with the tapo protocol.

@codecov
Copy link

codecov bot commented Dec 3, 2023

Codecov Report

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

Comparison is base (a6b7d73) 75.84% compared to head (38ab4d3) 75.75%.

Files Patch % Lines
kasa/tapo/tapoplug.py 42.85% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #560      +/-   ##
==========================================
- Coverage   75.84%   75.75%   -0.09%     
==========================================
  Files          34       34              
  Lines        2865     2875      +10     
  Branches      747      750       +3     
==========================================
+ Hits         2173     2178       +5     
- Misses        635      640       +5     
  Partials       57       57              

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

@sbytnar
Copy link
Contributor Author

sbytnar commented Dec 3, 2023

Regarding test harness data for the KP125M, I tried 'poetry run python -m devtools.dump_devinfo -d 192.168.2.170 --username "$USERNAME" --password "$PASSWORD"' and the first lines around the error are:

...
DEBUG:kasa.aesprotocol:Unable to authenticate with 192.168.2.170, not retrying
FAIL Could not complete send, response was {'error_code': -1501}
...

That seems weird, since the main kasa --type tapoplug commands work.

@sbytnar
Copy link
Contributor Author

sbytnar commented Dec 3, 2023

That seems weird, since the main kasa --type tapoplug commands work.

User error. Attached is the file it produced. The SSID & Nickname are base64 encoded strings that I anonymized as "User Given (SSID|Nickname)".
KP125M.smart_1.0_1.1.3.json

pytest has lots of errors of this form, not just for KP125M.

FAILED kasa/tests/test_emeter.py::test_get_emeter_realtime[/path/to/kasa/tests/fixtures/KP125M.smart_1.0_1.1.3.json0] - KeyError: 'component_nego'

Copy link
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR @sbytnar!

The test harness requires updates to make the tests pass against these new fixtures, that'll be done in #557. Feel free to create a PR to add the fixture file, that'll be merged after the linked PR is done!

@rytilahti rytilahti merged commit bfd1d6a into python-kasa:master Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants