-
Notifications
You must be signed in to change notification settings - Fork 902
Add Third Reality button settings #4352
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
base: dev
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #4352 +/- ##
==========================================
+ Coverage 92.12% 92.25% +0.12%
==========================================
Files 358 365 +7
Lines 11917 11997 +80
==========================================
+ Hits 10979 11068 +89
+ Misses 938 929 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@TheJulianJES codecov fail , please advise on how to fix it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and do I need to remove the previously submitted button.py?
Oh, I think I understood this wrong in some other PR. If you have a v2 quirk for the same device as an old v1 quirk, the old v1 quirk should be removed.
I'll take a look at addressing the comments in this PR now.
zhaquirks/thirdreality/button_v2.py
Outdated
| .number( | ||
| attribute_name=ThirdRealityButtonCluster.AttributeDefs.cancel_bouble_click.name, | ||
| cluster_id=ThirdRealityButtonCluster.cluster_id, | ||
| endpoint_id=1, | ||
| min_value=0, | ||
| max_value=65535, | ||
| step=1, | ||
| translation_key="cancel_bouble_click", | ||
| fallback_name="Cancel double click", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the unit for this? If it's seconds/time, the number device class should ideally also be set to DURATION.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that using .switch is more suitable
zhaquirks/thirdreality/button_v2.py
Outdated
| .number( | ||
| attribute_name=ThirdRealityButtonCluster.AttributeDefs.cancel_bouble_click.name, | ||
| cluster_id=ThirdRealityButtonCluster.cluster_id, | ||
| endpoint_id=1, | ||
| min_value=0, | ||
| max_value=65535, | ||
| step=1, | ||
| translation_key="cancel_bouble_click", | ||
| fallback_name="Cancel double click", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain what this does exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This product has double clicking and single clicking, both of which will send different commands. Some customers do not want double clicking, so we use a private cluster to remove double clicking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but this is a number entity. So, what is the difference between setting it to 0, 10, and 1000 for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, only 0 and 1 have an effect. 0 represents opening and double clicking, and 1 represents shutting down and double clicking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old v1 quirk also used a custom PowerConfiguration cluster to change the battery percentage reported in HA based on the device voltage, instead of the percentage reported by the device. Is this no longer needed?
zha-device-handlers/zhaquirks/thirdreality/button.py
Lines 31 to 35 in 203b56f
| class CustomPowerConfigurationCluster(PowerConfigurationCluster): | |
| """Custom PowerConfigurationCluster.""" | |
| MIN_VOLTS = 2.1 | |
| MAX_VOLTS = 3.0 |
| .device_automation_triggers( | ||
| { | ||
| (DOUBLE_PRESS, DOUBLE_PRESS): {COMMAND: COMMAND_DOUBLE}, | ||
| (SHORT_PRESS, SHORT_PRESS): {COMMAND: COMMAND_SINGLE}, | ||
| (LONG_PRESS, LONG_PRESS): {COMMAND: COMMAND_HOLD}, | ||
| (LONG_RELEASE, LONG_RELEASE): {COMMAND: COMMAND_RELEASE}, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are unchanged, but I don't think (DOUBLE_PRESS, DOUBLE_PRESS) is really nice. I think this is (press_type, button) normally. Device triggers will be replaced at some point anyways.
We'll have to keep this to not break existing automations though.
Yes, it's no longer needed |
Proposed change
This PR has added button quirk v2 and some basic attributes, automation, and private clusters
1.cancel_bouble_click----cancel button double-click function
Additional information
This PR fixes the issue where buttons cannot be automated in zha,and do I need to remove the previously submitted button.py?
Device diagnostics
zha-01JEN6GWZMMHTF70X8QNKV91VP-Third Reality, Inc 3RSB22BZ-0d377385e48c0e61cd04ea86b316bf17.json
Checklist
pre-commitchecks pass / the code has been formatted using Black