Set TuyaData.dp_type automatically #2043

Shulyaka · 2022-12-30T16:35:41Z

This is also a part separated from #2017

This PR adds the tuya.TuyaData.__init__() constructor that automatically maps TuyaData.dp_type from zigpy type. That allows to remove the dp_type field from the tuya.mcu.DPToAttributeMapping mapping which may simplify the things.

This is up to the developers to decide whether this is needed or not.

codecov-commenter · 2022-12-30T16:38:26Z

Codecov Report

Base: 83.69% // Head: 83.69% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (1d0387e) compared to base (11d693e).
Patch coverage: 91.66% of modified lines in pull request are covered.

📣 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    #2043   +/-   ##
=======================================
  Coverage   83.69%   83.69%           
=======================================
  Files         257      257           
  Lines        8225     8238   +13     
=======================================
+ Hits         6884     6895   +11     
- Misses       1341     1343    +2

Impacted Files	Coverage Δ
zhaquirks/tuya/ts0601_rcbo.py	`98.59% <ø> (ø)`
zhaquirks/tuya/ts0601_siren.py	`93.96% <ø> (ø)`
zhaquirks/tuya/ts0601_valve.py	`92.30% <ø> (ø)`
zhaquirks/tuya/__init__.py	`75.33% <89.47%> (+0.45%)`	⬆️
zhaquirks/tuya/mcu/__init__.py	`99.50% <100.00%> (-0.02%)`	⬇️
zhaquirks/tuya/ts0601_garage.py	`100.00% <100.00%> (ø)`
zhaquirks/tuya/ts0601_illuminance.py	`100.00% <100.00%> (ø)`
zhaquirks/tuya/ts0601_sensor.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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

coveralls · 2022-12-30T16:43:29Z

Pull Request Test Coverage Report for Build 4091436317

22 of 24 (91.67%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.001%) to 83.697%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
zhaquirks/tuya/init.py	17	19	89.47%

Totals
Change from base Build 4079750249:	0.001%
Covered Lines:	6895
Relevant Lines:	8238

💛 - Coveralls

javicalle · 2022-12-30T19:26:51Z

I like the idea very much, but I'm not sure if will work.
I thought I had done tests to validate the TuyaData.dp_type, but I see that there aren't any, so not sure.

My concern is that the implementation is based on the type of the data:

isinstance(value, whatever)

And the instances of TuyaDatapointData will be created here:

zha-device-handlers/zhaquirks/tuya/mcu/__init__.py

Line 182 in a6556fe

def from_cluster_data(self, data: TuyaClusterData) -> Optional[TuyaCommand]:

Upper in code, the method will be called from here:

zha-device-handlers/zhaquirks/tuya/mcu/__init__.py

Line 230 in a6556fe

def tuya_mcu_command(self, cluster_data: TuyaClusterData):

And data will come in a TuyaClusterData object that is defined as:

zha-device-handlers/zhaquirks/tuya/mcu/__init__.py

Lines 59 to 67 in a6556fe

    
           class TuyaClusterData(t.Struct): 
        
               """Tuya cluster data.""" 
        
               endpoint_id: int 
        
               cluster_name: str 
        
               cluster_attr: str 
        
               attr_value: int  # Maybe also others types? 
        
               expect_reply: bool 
        
               manufacturer: int

Do you see my point? (attr_value: int)

I'm pretty sure that must be a way to avoid the type instantiation as you want but I haven't found it.
In origin, we know the cluster attribute to update and its value, but not sure if a direct mapping Zigbee attribute type <--> MCU DP type will be possible.

Shulyaka · 2022-12-30T22:46:11Z

Fixed annotation for TuyaClusterData.attr_value.

The type is set when we assign the attribute, here is an example for bool:

zha-device-handlers/zhaquirks/tuya/mcu/__init__.py

Line 337 in 507edfc

attr_value=bool(command_id),

Shulyaka · 2022-12-30T23:02:29Z

Hm, that fails for python 3.8 and 3.9, checking.

javicalle · 2022-12-30T23:22:40Z

Ok, I see...
What about this?

zha-device-handlers/zhaquirks/tuya/mcu/__init__.py

Line 109 in a6556fe

cluster_data = TuyaClusterData(

The method is invoked when you update one cluster attribute from the 'manage cluster' view (and maybe other places). 🤔

Shulyaka · 2022-12-30T23:51:32Z

What about this?

In this case we are using the type of the cluster attribute.
See https://github.com/zigpy/zigpy/blob/master/zigpy/zcl/__init__.py#L547
I believe they should also match the tuya type in most cases.

Shulyaka · 2022-12-30T23:59:05Z

Hm, that fails for python 3.8 and 3.9, checking.

Ok, I removed the annotation fix because I couldn't easily make it work for python below 3.10. zigpy.types.Struct doesn't want to accept typing.Union.

Regardless, it looks like TuyaClusterData.attr_value can have types other than int, I don't know if it is a zigpy bug or not.

dmulcahey · 2022-12-31T12:21:26Z

Are we good with this?

dmulcahey · 2023-01-02T19:25:12Z

@Shulyaka @javicalle please let me know when this is mergable

Shulyaka · 2023-01-10T10:10:29Z

From my perspective, it is ready. @javicalle just needs to decide if the idea worth it.

dmulcahey · 2023-01-10T15:46:30Z

@javicalle thoughts?

javicalle · 2023-01-12T22:29:26Z

I meant to do some testing and validate this change but all my installation is broken and I'm having trouble spending time to fix it.
Please let me check it out over the weekend.

javicalle · 2023-01-16T09:06:26Z

First of all, I apologize to @Shulyaka for not having been able to review the topic before 🙇🏻‍♂️

Apart from a minor comment, let's go for it!
@dmulcahey would it be possible to not include the change until the 01/25 beta release?
@Shulyaka anything I can help you with, just tell me.

javicalle · 2023-01-16T09:07:21Z

zhaquirks/tuya/__init__.py

+        if value is None:
+            return
+        elif isinstance(value, (t.bitmap8, t.bitmap16, t.bitmap32)):
+            self.dp_type = TuyaDPType.BITMAP
+        elif isinstance(value, (bool, t.Bool)):
+            self.dp_type = TuyaDPType.BOOL
+        elif isinstance(value, enum.Enum):
+            self.dp_type = TuyaDPType.ENUM
+        elif isinstance(value, int):
+            self.dp_type = TuyaDPType.VALUE
+        elif isinstance(value, str):
+            self.dp_type = TuyaDPType.STRING
+        elif isinstance(value, t.Struct):
+            self.dp_type = TuyaDPType.RAW
+


Maybe a safe check here to be sure to assign the dp_type?

I have a different idea, to set it default to RAW

dmulcahey · 2023-01-23T12:21:15Z

@Shulyaka can you rebase this and see if any of the recently merged tuya quirks need changes? Once that’s done I’ll merge this and get a release produced for the beta.

Shulyaka · 2023-02-04T11:35:09Z

Done.
Sorry for the delay, I had a flu

Shulyaka force-pushed the refactoring_2 branch from 06ed754 to 507edfc Compare December 30, 2022 23:54

javicalle reviewed Jan 16, 2023

View reviewed changes

TheJulianJES added the Tuya Request/PR regarding a Tuya device label Jan 16, 2023

TheJulianJES added the waiting for changes Waiting for changes to the PR label Jan 31, 2023

Shulyaka force-pushed the refactoring_2 branch from 507edfc to 03d7de6 Compare February 4, 2023 11:32

Set TuyaData.dp_type automatically

3a50615

Shulyaka force-pushed the refactoring_2 branch from 03d7de6 to 151e331 Compare February 4, 2023 11:33

set function even when value is None

943d8c7

Shulyaka force-pushed the refactoring_2 branch from 151e331 to 943d8c7 Compare February 4, 2023 11:36

default TuyaData.dp_type to RAW

1d0387e

dmulcahey merged commit b8368c5 into zigpy:dev Feb 4, 2023

buzzeddesign mentioned this pull request Mar 1, 2023

Add support for ZY-M100 wall mount #1928

Merged

javicalle mentioned this pull request Mar 3, 2023

[BUG] Custom quirks not working in HA 2023.03 #2250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set TuyaData.dp_type automatically #2043

Set TuyaData.dp_type automatically #2043

Shulyaka commented Dec 30, 2022

codecov-commenter commented Dec 30, 2022 •

edited

Loading

coveralls commented Dec 30, 2022 •

edited

Loading

javicalle commented Dec 30, 2022 •

edited

Loading

Shulyaka commented Dec 30, 2022

Shulyaka commented Dec 30, 2022

javicalle commented Dec 30, 2022

Shulyaka commented Dec 30, 2022 •

edited

Loading

Shulyaka commented Dec 30, 2022

dmulcahey commented Dec 31, 2022

dmulcahey commented Jan 2, 2023

Shulyaka commented Jan 10, 2023

dmulcahey commented Jan 10, 2023

javicalle commented Jan 12, 2023

javicalle commented Jan 16, 2023

javicalle Jan 16, 2023

Shulyaka Feb 4, 2023

Shulyaka Feb 4, 2023

dmulcahey commented Jan 23, 2023

Shulyaka commented Feb 4, 2023

Set TuyaData.dp_type automatically #2043

Set TuyaData.dp_type automatically #2043

Conversation

Shulyaka commented Dec 30, 2022

codecov-commenter commented Dec 30, 2022 • edited Loading

Codecov Report

coveralls commented Dec 30, 2022 • edited Loading

Pull Request Test Coverage Report for Build 4091436317

💛 - Coveralls

javicalle commented Dec 30, 2022 • edited Loading

Shulyaka commented Dec 30, 2022

Shulyaka commented Dec 30, 2022

javicalle commented Dec 30, 2022

Shulyaka commented Dec 30, 2022 • edited Loading

Shulyaka commented Dec 30, 2022

dmulcahey commented Dec 31, 2022

dmulcahey commented Jan 2, 2023

Shulyaka commented Jan 10, 2023

dmulcahey commented Jan 10, 2023

javicalle commented Jan 12, 2023

javicalle commented Jan 16, 2023

javicalle Jan 16, 2023

Choose a reason for hiding this comment

Shulyaka Feb 4, 2023

Choose a reason for hiding this comment

Shulyaka Feb 4, 2023

Choose a reason for hiding this comment

dmulcahey commented Jan 23, 2023

Shulyaka commented Feb 4, 2023

codecov-commenter commented Dec 30, 2022 •

edited

Loading

coveralls commented Dec 30, 2022 •

edited

Loading

javicalle commented Dec 30, 2022 •

edited

Loading

Shulyaka commented Dec 30, 2022 •

edited

Loading