Skip to content

Fix Tuya BlitzWolf motion sensor #3629

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

Merged
merged 6 commits into from
Dec 23, 2024

Conversation

TheJulianJES
Copy link
Collaborator

@TheJulianJES TheJulianJES commented Dec 23, 2024

Proposed change

This reverts the BlitzWolf motion sensors to use the IasZone cluster instead of using the OccupancySensing cluster.
This change was made to keep existing entities working. The missing auto-reset delay was also reintroduced.

cc @prairiesnpr

Additional information

Small follow-up to #3612

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

@TheJulianJES TheJulianJES added Tuya Request/PR regarding a Tuya device bugfix This PR fixes a bug v2 quirk Quirks using v2 API. Might add custom entities that need translation keys in HA. labels Dec 23, 2024
Copy link

codecov bot commented Dec 23, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.84%. Comparing base (1d038ec) to head (120c931).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
zhaquirks/tuya/ts0601_motion.py 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3629      +/-   ##
==========================================
+ Coverage   89.83%   89.84%   +0.01%     
==========================================
  Files         322      322              
  Lines       10350    10371      +21     
==========================================
+ Hits         9298     9318      +20     
- Misses       1052     1053       +1     

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

@TheJulianJES
Copy link
Collaborator Author

TheJulianJES commented Dec 23, 2024

Hmm, looking at the code we had for this before, I'm now also wondering if this sensor correctly sends a "reset" message.
Previously, we just reset it after 15 seconds of no messages.

I think that code was written before we had a proper Tuya datapoint conversion though.

@TheJulianJES
Copy link
Collaborator Author

Ok, got the sensor now and we need a timeout. The current zhaquirks code for MotionWithReset and so on still uses Bus, so that doesn't play well with quirks v2. I'll take another stab or revert that sensor part otherwise.

This code is somewhat copied from `MotionWithReset` and related clusters from the base `zhaquirks` module.
However, that code is outdated and still uses `Bus` somewhat and listens to the IAS zone status change commands. We generally listen to IAS zone status attribute updates now. This is also needed for the Tuya DP conversion.
These aren't needed anymore
@TheJulianJES TheJulianJES changed the title Fix Tuya BlitzWolf motion sensor using wrong cluster Fix Tuya BlitzWolf motion sensor Dec 23, 2024
@TheJulianJES
Copy link
Collaborator Author

Changes work as expected now.
The timeout code is somewhat copied from MotionWithReset and related clusters from the base zhaquirks module.
However, that code is outdated, still uses Bus somewhat, and listens to the IAS zone status change commands.
We generally listen to IAS zone status attribute updates now. This is also needed for the Tuya DP conversion.

@TheJulianJES TheJulianJES merged commit ca569cf into zigpy:dev Dec 23, 2024
9 checks passed
Comment on lines +53 to +54
if self._timer_handle:
self._timer_handle.cancel()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, missed to cover this one line. During a long-ish reset_s time, we should just re-send the motion message once to also cover this and make sure the delay restarts in tests, although the latter is a bit awkward to do, as we currently modify reset_s to 0. We may want to consider using the looptime lib here(?)

@TheJulianJES
Copy link
Collaborator Author

Hmm, reset_s was 15 before the changes in #3612 and is again with this PR, but my sensor seems to need a longer delay.
I'll leave this as-is for now to keep behavior "unchanged", but I wonder why nobody complained before..? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR fixes a bug Tuya Request/PR regarding a Tuya device v2 quirk Quirks using v2 API. Might add custom entities that need translation keys in HA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant