-
Notifications
You must be signed in to change notification settings - Fork 737
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 Tuya DPs mapped to multiple cluster attributes #3643
base: dev
Are you sure you want to change the base?
Changes from all commits
f22ee00
0cf12e5
d36c1c2
98ff595
32c3fbb
47bd335
6142170
965a3ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,7 +196,7 @@ def __init__( | |
) -> None: | ||
"""Init the TuyaQuirkBuilder.""" | ||
self.tuya_data_point_handlers: dict[int, str] = {} | ||
self.tuya_dp_to_attribute: dict[int, DPToAttributeMapping] = {} | ||
self.tuya_dp_to_attribute: dict[int, list[DPToAttributeMapping]] = {} | ||
self.new_attributes: set[foundation.ZCLAttributeDef] = set() | ||
super().__init__(manufacturer, model, registry) | ||
# quirk_file will point to the init call above if called from this QuirkBuilder, | ||
|
@@ -504,18 +504,31 @@ def tuya_dp( | |
"""Add Tuya DP Converter.""" | ||
self.tuya_dp_to_attribute.update( | ||
{ | ||
dp_id: DPToAttributeMapping( | ||
ep_attribute, | ||
attribute_name, | ||
converter=converter, | ||
dp_converter=dp_converter, | ||
endpoint_id=endpoint_id, | ||
) | ||
dp_id: [ | ||
DPToAttributeMapping( | ||
ep_attribute, | ||
attribute_name, | ||
converter=converter, | ||
dp_converter=dp_converter, | ||
endpoint_id=endpoint_id, | ||
) | ||
] | ||
} | ||
) | ||
self.tuya_data_point_handlers.update({dp_id: dp_handler}) | ||
return self | ||
|
||
def tuya_dp_multi( | ||
self, | ||
dp_id: int, | ||
attribute_mapping: list[DPToAttributeMapping], | ||
dp_handler: str = "_dp_2_attr_update", | ||
) -> QuirkBuilder: # fmt: skip | ||
"""Add Tuya DP Converter that maps to multiple attributes.""" | ||
self.tuya_dp_to_attribute.update({dp_id: attribute_mapping}) | ||
self.tuya_data_point_handlers.update({dp_id: dp_handler}) | ||
return self | ||
Comment on lines
+521
to
+530
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, is there any benefit to also having this method instead of just calling If so, we may want to add a test for this, though I'd need to check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of the reason was that calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I just noticed that I missed the tests when opening this PR from the original one: https://github.com/zigpy/zha-device-handlers/pull/3644/files#diff-d01eb6338c5bfa4635fb433e6a8fc5b9cf3de58e784218f8930c5ccaedd927f5 Copied them to this PR now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, one more thing regarding this. Can we either block subsequent calls to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added that in a new PR #3827 since it required changing 2 quirks that were invalid. |
||
|
||
def tuya_dp_attribute( | ||
self, | ||
dp_id: int, | ||
|
@@ -810,7 +823,7 @@ class TuyaReplacementCluster(replacement_cluster): # type: ignore[valid-type] | |
"""Replacement Tuya Cluster.""" | ||
|
||
data_point_handlers: dict[int, str] | ||
dp_to_attribute: dict[int, DPToAttributeMapping] | ||
dp_to_attribute: dict[int, list[DPToAttributeMapping]] | ||
|
||
class AttributeDefs(NewAttributeDefs): | ||
"""Attribute Definitions.""" | ||
|
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.
Well, this code also never expected a tuple, so the "old multi DP code".
Needs updating in a future PR if we keep that code around.
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.
Personally I would prefer to drop the tuple from
DPToAttributeMapping
. Even if a few custom quirks break, I think it is worth it to have the code a bit clean. Except if there are a lot of custom quirks using 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.
Ok, we probably should remove it soon then.
I guess we could also convert the old tuple-style to the new
_dp_to_attributes
format with the list in__init__
if we really wanted to. Then, we wouldn't need to care about handling it again in_dp_2_attr_update
or for the "mark attributes as valid". I think that could be nice. Otherwise, we can just remove it soon.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 am fine with both removing or converting it to
_dp_to_attributes
. Personally, I prefer to remove it just to keep the code simpler and easier to maintain.But let me know which approach you prefer and I'll do that. Is it fine if we leave that to a follow-up PR? This one is not breaking it anyway.
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.
yeah, that's fine.