-
Notifications
You must be signed in to change notification settings - Fork 762
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 support for Third Reality plug private cluster ID #3078
Conversation
Change-Id: I71ec608ad7ffec94719672fdf9016bf4e905b0ff
Change-Id: I1f256ba39952135fb5fdc47af8a05e282af01aa1
Change-Id: Ia82568ce4e96e049dfc3e0e4ca4c77ae3e700c2e
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3078 +/- ##
==========================================
+ Coverage 87.90% 88.20% +0.29%
==========================================
Files 300 301 +1
Lines 9222 9412 +190
==========================================
+ Hits 8107 8302 +195
+ Misses 1115 1110 -5 ☔ View full report in Codecov by Sentry. |
Change-Id: Ie0c59078abd59d0337f85d80f580aab22b1fbc05
Change-Id: I80e87387f56ccbb1fb3be7257fdb99dc80dec958
Change-Id: I20507348a47fb94a2d28995eae0f374ca4573536
This code adds the private cluster ID to the Third Reality plug. Please help with it, thank you. |
@dmulcahey @TheJulianJES Could you help to review the merge require, thanks. |
zhaquirks/thirdreality/plug.py
Outdated
PROFILE_ID: zha.PROFILE_ID, | ||
DEVICE_TYPE: zha.DeviceType.SMART_PLUG, | ||
INPUT_CLUSTERS: [ | ||
Basic.cluster_id, # 0x0000 |
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.
Please remove all cluster id comments like # 0x0000
.
It's inconsistent with most of our codebase.
zhaquirks/thirdreality/plug.py
Outdated
PROFILE_ID: 0xA1E0, | ||
DEVICE_TYPE: 0x0061, |
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.
There's a zgp profile for this.
PROFILE_ID: zgp.PROFILE_ID,
DEVICE_TYPE: zgp.DeviceType.PROXY_BASIC,
zhaquirks/thirdreality/plug.py
Outdated
PROFILE_ID: 0xA1E0, | ||
DEVICE_TYPE: 0x0061, |
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.
Also use the zgp profile/constants here.
zhaquirks/thirdreality/plug.py
Outdated
reset_summation_delivered: Final = ZCLAttributeDef( | ||
id=RESET_SUMMATION_DELIVERED_ATTR_ID, | ||
type=ControlMode, | ||
is_manufacturer_specific=True, | ||
) |
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.
Using a write_attr_button
from quirks v2, a button would be added to HA that would set the attribute to CLEAR
.
I'm assuming the plug automatically resets it back to NOT
afterwards?
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.
We only need to clear the summation delivered of PLUG to zero when the button is pressed.
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.
Hello, is there any case of related projects that you mentioned in this functional code?
zhaquirks/thirdreality/plug.py
Outdated
RESET_SUMMATION_DELIVERED_ATTR_ID = 0x0000 | ||
|
||
|
||
class ControlMode(t.uint8_t): |
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 NOT
and CLEAR
parts don't really have an effect, as this is not an enum.
There's a longer discussion here: #2723 (comment)
I'd just remove this section for now and set type=t.uint8_t
for the attribute.
(When the button is later exposed to HA, it'll be clear that 1
means reset.)
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
Hello, please check it out again, thanks. |
@TheJulianJES |
|
There's an example in the quirks v2 docs PR and/or similar in the open Bosch PR. |
@TheJulianJES @dmulcahey I put the code below. Could you please help me see if the code is written wrong? Thank you!
|
Replacing """Third Reality Plug devices."""
from typing import Final
import zigpy.types as t
from zigpy.zcl.foundation import BaseAttributeDefs, ZCLAttributeDef
from zigpy.quirks.v2 import add_to_registry_v2
from zhaquirks import CustomCluster
from zigpy.quirks.v2 import ClusterType
from zhaquirks.thirdreality import THIRD_REALITY
class ThirdRealityPlugCluster(CustomCluster):
cluster_id = 0xFF03
class AttributeDefs(BaseAttributeDefs):
reset_summation_delivered: Final = ZCLAttributeDef(
id=0x0000,
type=t.uint8_t,
is_manufacturer_specific=True,
)
(
add_to_registry_v2(THIRD_REALITY, "3RSP02028BZ")
.adds(ThirdRealityPlugCluster)
.write_attr_button(
attribute_name=ThirdRealityPlugCluster.AttributeDefs.reset_summation_delivered.name,
attribute_value=0x01,
cluster_id=ThirdRealityPlugCluster.cluster_id,
cluster_type=ClusterType.Server,
endpoint_id=1,
translation_key="reset_summation_delivered"
)
) |
@puddly Thank you for your patience, now the code works. But I still have a question, that is, how to set the default name of the button control that appears on the page? As you can see, the current name is "Third Reality, Inc 3RSP0202...". |
@hwzolin A translation key (default: attribute name) for Home Assistant/ZHA itself needs to be added. |
@TheJulianJES Do you mean like this: https://github.com/hwzolin/core/blob/dev/homeassistant/components/zha/strings.json? |
@TheJulianJES Hello, I have updated the code about zha quirk v2, please check it out. If there is no problem, please merge it. |
|
||
|
||
( | ||
add_to_registry_v2(THIRD_REALITY, "3RSP02028BZ") |
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.
You can use .also_applies_to(THIRD_REALITY, "3RSPE01044BZ")
to make this quirk apply for both plugs.
The second quirk below can be removed then.
cluster_id=ThirdRealityPlugCluster.cluster_id, | ||
cluster_type=ClusterType.Server, | ||
endpoint_id=1, | ||
translation_key="reset_summation_delivered" |
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 think ZHA should already use the attribute name as the translation key by default. Since it's the same, this isn't needed IIRC.
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.
@TheJulianJES
I'm sorry, I've been so busy lately, so I didn't pay attention to your reply in time.
I've made some changes to the code as you asked, and the modified code looks like this:
"""Third Reality Plug devices."""
from typing import Final
from zigpy.quirks import DeviceRegistry
from zigpy.quirks.v2 import ClusterType, add_to_registry_v2
import zigpy.types as t
from zigpy.zcl.foundation import BaseAttributeDefs, ZCLAttributeDef
from zhaquirks import CustomCluster
from zhaquirks.thirdreality import THIRD_REALITY
class ThirdRealityPlugCluster(CustomCluster):
"""The private cluster id of Third Reality's PLUG."""
cluster_id = 0xFF03
class AttributeDefs(BaseAttributeDefs):
"""Attribute of private cluster id."""
reset_summation_delivered: Final = ZCLAttributeDef(
id=0x0000,
name="reset_summation_delivered",
type=t.uint8_t,
is_manufacturer_specific=True
)
(
add_to_registry_v2(THIRD_REALITY, "3RSP02028BZ")
.also_applies_to(THIRD_REALITY, "3RSPE01044BZ")
.adds(ThirdRealityPlugCluster.cluster_id)
.replaces(ThirdRealityPlugCluster)
.write_attr_button(
attribute_name=ThirdRealityPlugCluster.AttributeDefs.reset_summation_delivered.name,
attribute_value=0x01,
cluster_id=ThirdRealityPlugCluster.cluster_id
)
)
I'm not sure if the code changes are the same as you were asking for at the time, but the results don't work as expected. The name of this button in HA is still "Third Realtiy, Inc" + the model id of the device.
As shown in the following GIF, the button click function has been realized. So I don't know what is not enough to improve? Where do I need to modify? Can you please take some time out of your busy schedule to help me fix the imperfections.
There will be other devices that need to be supported in the future. I think if one device can realize the function, then there will be something to consider later.
attribute_name=ThirdRealityPlugCluster.AttributeDefs.reset_summation_delivered.name, | ||
attribute_value=0x01, | ||
cluster_id=ThirdRealityPlugCluster.cluster_id, | ||
cluster_type=ClusterType.Server, |
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 cluster_type
is already Server
by default, so this can be removed.
attribute_value=0x01, | ||
cluster_id=ThirdRealityPlugCluster.cluster_id, | ||
cluster_type=ClusterType.Server, | ||
endpoint_id=1, |
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.
endpoint_id=1
is default, so this can be removed.
@TheJulianJES Hello, due to the busy work of the previous project, this project has been put on hold for the time being. |
|
Proposed change
Add support for the "reset summation delivered" attribute for Third Reality plugs.
Additional information
Checklist
pre-commit
checks pass / the code has been formatted using Black