Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TuyaNoBindPowerConfigurationCluster for Tuya remotes to stop battery drain #2288

Merged
merged 3 commits into from
Mar 24, 2023

Conversation

TheJulianJES
Copy link
Collaborator

@TheJulianJES TheJulianJES commented Mar 21, 2023

Untested at the moment.

This PR prevents the binding of the PowerConfiguration cluster (and setting up attribute reports).
In return, this prevents battery drain on Tuya remotes.

Fixes #2227 (if it works)

Also, why do some Tuya remotes have multiple PowerConfiguration clusters? Should we "remove" all of them except one?

Note: The bind() and _configure_reporting() code is taken out of the LocalDataCluster

@TheJulianJES TheJulianJES added the Tuya Request/PR regarding a Tuya device label Mar 21, 2023
@coveralls
Copy link

coveralls commented Mar 21, 2023

Pull Request Test Coverage Report for Build 4514449044

  • 10 of 10 (100.0%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 83.961%

Totals Coverage Status
Change from base Build 4511912036: 0.01%
Covered Lines: 6915
Relevant Lines: 8236

💛 - Coveralls

@MattWestb
Copy link
Contributor

About unused / not function cluster #973.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (fa60b9d) 83.92% compared to head (dccceb9) 83.93%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #2288   +/-   ##
=======================================
  Coverage   83.92%   83.93%           
=======================================
  Files         260      260           
  Lines        8219     8224    +5     
=======================================
+ Hits         6898     6903    +5     
  Misses       1321     1321           
Impacted Files Coverage Δ
zhaquirks/tuya/__init__.py 75.82% <100.00%> (+0.20%) ⬆️
zhaquirks/tuya/ts0041.py 100.00% <100.00%> (ø)
zhaquirks/tuya/ts0042.py 100.00% <100.00%> (ø)
zhaquirks/tuya/ts0043.py 100.00% <100.00%> (ø)
zhaquirks/tuya/ts0044.py 100.00% <100.00%> (ø)
zhaquirks/tuya/ts0046.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheJulianJES TheJulianJES marked this pull request as ready for review March 23, 2023 06:14
@TheJulianJES
Copy link
Collaborator Author

TheJulianJES commented Mar 23, 2023

Should fix the battery drain issue. Hoping for someone in #2227 to confirm. (Will merge if ready)

Can already confirm that the remote still works as expected.

@nibo2
Copy link

nibo2 commented Mar 23, 2023

I can confirm from logs and tests that __init__.py and ts0044.py both initialize and work fine. Battery percentage and voltage is reported at startup.

I can not yet confirm if battery drain is gone or not. Actually, battery drain seems to have stopped already with SKIP_CONFIGURATION that I have been running for 2 weeks before this latest test. There has been no drop in battery percentage since then, and also no drop with the latest test.

Is there any more tests I/we should do?


async def _configure_reporting(self, *args, **kwargs): # pylint: disable=W0221
"""Prevent remote configure reporting."""
return (foundation.ConfigureReportingResponse.deserialize(b"\x00")[0],)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just guessing, would it be the same that ConfigureReportingResponseRecord(status=Status.SUCCESS).serialize()?`
https://github.com/zigpy/zigpy/blob/1f81b162f84328eb96db18add3104d889fa6c923/zigpy/zcl/foundation.py#L438

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, probably. I think we should also change it in LocalDataCluster then. Didn't want to do that in this PR though.

I think I'd leave it like this and then change both (and some other similar stuff) in a future PR(?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, for sure. 👍🏻 (just doing a quick issues review, not trying to criticize nobodies work)

@TheJulianJES TheJulianJES force-pushed the tjj/tuya_block_power_config_bind branch from dccceb9 to 62d3f28 Compare March 24, 2023 18:53
@TheJulianJES
Copy link
Collaborator Author

Merging now, so we can move forward with #2287.

I tested it on one of my Zemismart remotes and it doesn't break anything and correctly blocks binding of the power configuration cluster (which is likely enough to stop the excessive battery drain).

@TheJulianJES TheJulianJES changed the title Add TuyaNoBindPowerConfigurationCluster for Tuya remotes Add TuyaNoBindPowerConfigurationCluster for Tuya remotes to stop battery drain Mar 24, 2023
@TheJulianJES TheJulianJES merged commit b729f10 into zigpy:dev Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tuya Request/PR regarding a Tuya device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Tuya TS0041 TS0042 TS0043 - no battery
6 participants