Add support for _TZE200_yi4jtqq1 illuminance sensor. #1795

gmsoft-tuxicoman · 2022-09-29T07:02:29Z

This is a simple illuminance sensort with a maximum of 1000 lumen. So it's only really suited for inside, not outside.

The quirk provide both the raw lumen value as well as the low/medium/high brightness level.

coveralls · 2022-09-29T07:04:02Z

Pull Request Test Coverage Report for Build 3291717546

25 of 25 (100.0%) changed or added relevant lines in 1 file are covered.
27 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.5%) to 80.509%

Files with Coverage Reduction	New Missed Lines	%
zhaquirks/xiaomi/init.py	27	83.79%

Totals
Change from base Build 3212112661:	0.5%
Covered Lines:	6043
Relevant Lines:	7506

💛 - Coveralls

codecov-commenter · 2022-09-29T07:05:54Z

Codecov Report

Base: 80.00% // Head: 80.50% // Increases project coverage by +0.50% 🎉

Coverage data is based on head (69aec27) compared to base (2292c4e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1795      +/-   ##
==========================================
+ Coverage   80.00%   80.50%   +0.50%     
==========================================
  Files         239      241       +2     
  Lines        7430     7506      +76     
==========================================
+ Hits         5944     6043      +99     
+ Misses       1486     1463      -23

Impacted Files	Coverage Δ
zhaquirks/tuya/ts0601_illuminance.py	`100.00% <100.00%> (ø)`
zhaquirks/tuya/ts001x.py	`100.00% <0.00%> (ø)`
zhaquirks/lidl/TS0501A.py	`100.00% <0.00%> (ø)`
zhaquirks/lixee/zlinky.py	`100.00% <0.00%> (ø)`
zhaquirks/lixee/__init__.py	`100.00% <0.00%> (ø)`
zhaquirks/inovelli/VZM31SN.py	`100.00% <0.00%> (ø)`
zhaquirks/tuya/air/ts0601_air_quality.py	`100.00% <0.00%> (ø)`
zhaquirks/xiaomi/aqara/plug_mmeu01.py
zhaquirks/gledopto/glc009.py	`100.00% <0.00%> (ø)`
zhaquirks/xiaomi/aqara/plug_eu.py	`100.00% <0.00%> (ø)`
... and 1 more

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.

dmulcahey · 2022-10-01T17:05:18Z

Please rebase this and install / run the precommit checks

dmulcahey · 2022-10-10T15:26:43Z

@javicalle lmk what you think when you have a moment please. Thanks!

javicalle

These would be the minimum requirements to review.

javicalle · 2022-10-12T09:37:05Z

zhaquirks/tuya/ts0601_illuminance.py

+TUYA_BRIGHTNESS_LEVEL_ATTR = 0x01  # 0-2 "Low, Medium, High"
+TUYA_ILLUMINANCE_ATTR = 0x02  # [0, 0, 3, 232] illuminance


No problem with setting as constants, but I would prefer not to use the _ATTR sufix because it can be confusing (they are not clusters attributes).
Replace all around with a DataPoint nomenclature:

TUYA_BRIGHTNESS_LEVEL_DP = 0x01 # 0-2 "Low, Medium, High" TUYA_ILLUMINANCE_DP = 0x02 # [0, 0, 3, 232] illuminance

javicalle · 2022-10-12T09:43:06Z

zhaquirks/tuya/ts0601_illuminance.py

+    attributes = IlluminanceMeasurement.attributes.copy()
+    attributes.update(
+        {
+            0xFF00: ("brightness_level", t.CharacterString, True),


The illuminanceMeasurement cluster don't have any reference to brightness_level so not sure if that could be the correct attribute name 🤔
We can keep the reference for now but I would replace for manufacturer_brightness_level or manufacturer_measured_value.

javicalle · 2022-10-12T09:49:02Z

zhaquirks/tuya/ts0601_illuminance.py

+TUYA_BRIGHTNESS_LEVEL_ATTR = 0x01  # 0-2 "Low, Medium, High"
+TUYA_ILLUMINANCE_ATTR = 0x02  # [0, 0, 3, 232] illuminance
+
+TUYA_BRIGHTNESS_LEVEL_STR = ["Low", "Medium", "High"]


What about if we use:

class BrightnessLevel(t.enum8): """Brightness level enum.""" LOW = 0x00 MEDIUM = 0x01 HIGH = 0x02

That way you can define the attribute as:

0xFF00: ("brightness_level", BrightnessLevel, True),

And convert as:

dp_to_attribute: Dict[int, DPToAttributeMapping] = { TUYA_BRIGHTNESS_LEVEL_ATTR: DPToAttributeMapping( TuyaIlluminanceMeasurement.ep_attribute, "brightness_level", lambda x: BrightnessLevel(x), ),

javicalle

I intend to migrate the class hierarchy for these types of devices, so I am going to ask you to make the following changes.
These changes are not a prerequisite, but they will help us with future maintenance.

If you're going to tackle the changes, I'd suggest making the other changes first, then attacking these changes.

javicalle · 2022-10-12T10:11:30Z

zhaquirks/tuya/ts0601_illuminance.py

+class TuyaIlluminanceCluster(TuyaNewManufCluster):
+    """Tuya Illuminance cluster."""
+
+    dp_to_attribute: Dict[int, DPToAttributeMapping] = {
+        TUYA_BRIGHTNESS_LEVEL_ATTR: DPToAttributeMapping(
+            TuyaIlluminanceMeasurement.ep_attribute,
+            "brightness_level",
+            lambda x: TUYA_BRIGHTNESS_LEVEL_STR[x],
+        ),
+        TUYA_ILLUMINANCE_ATTR: DPToAttributeMapping(
+            TuyaIlluminanceMeasurement.ep_attribute,
+            "measured_value",
+            lambda x: (10000.0 * math.log10(x) + 1.0 if x != 0 else 0),
+        ),
+    }
+
+    data_point_handlers = {
+        TUYA_BRIGHTNESS_LEVEL_ATTR: "_dp_2_attr_update",
+        TUYA_ILLUMINANCE_ATTR: "_dp_2_attr_update",
+    }


You will need to change the imports statements to the new ones:

from zhaquirks.tuya import TuyaLocalCluster from zhaquirks.tuya.mcu import ( DPToAttributeMapping, TuyaDPType, TuyaMCUCluster, )

Change your TuyaIlluminanceCluster class declaration to something like:

class TuyaIlluminanceCluster(TuyaMCUCluster): """Tuya Illuminance cluster.""" dp_to_attribute: Dict[int, DPToAttributeMapping] = { TUYA_BRIGHTNESS_LEVEL_ATTR: DPToAttributeMapping( TuyaIlluminanceMeasurement.ep_attribute, "brightness_level", # or the new name for attribute dp_type=TuyaDPType.ENUM, converter=lambda x: BrightnessLevel(x), ), TUYA_ILLUMINANCE_ATTR: DPToAttributeMapping( TuyaIlluminanceMeasurement.ep_attribute, "measured_value", dp_type=TuyaDPType.VALUE, converter=lambda x: (10000.0 * math.log10(x) + 1.0 if x != 0 else 0), ), } data_point_handlers = { TUYA_BRIGHTNESS_LEVEL_ATTR: "_dp_2_attr_update", TUYA_ILLUMINANCE_ATTR: "_dp_2_attr_update", }

Thats all.

javicalle · 2022-10-12T10:33:49Z

Not happy with another ts0601_whatever.py file, but not sure if there is a better fit. Keep as is for now.

The TuyaIlluminanceMeasurement must be moved to a generic class.
I like the security code check, but not sure if it is correct (I believe that would be something like if x > 1 else 0 or maybe a parenthesis is needed here).
Keep as is and I will move in another PR.

The device doesn't have a battery report?
The new implementation would log any not implemented report (warning level).

gmsoft-tuxicoman · 2022-10-20T13:24:56Z

I've made all the changes.

This device doesn't have a battery sensor as it's USB powered.

javicalle

Just one left.

javicalle · 2022-10-20T14:39:02Z

zhaquirks/tuya/ts0601_illuminance.py

+    attributes = IlluminanceMeasurement.attributes.copy()
+    attributes.update(
+        {
+            0xFF00: ("brightness_level", BrightnessLevel, True),


Maybe manufacturer_brightness_level here?

Must be the same value as in dp_to_attribute mapping.

javicalle

LGTM

Thanks for your work and patience.

Add support for _TZE200_yi4jtqq1 illuminance sensor.

299c9d8

gmsoft-tuxicoman added 3 commits October 10, 2022 08:36

Merge branch 'zigpy:dev' into _TZE200_yi4jtqq1

1aae28e

Fix linting.

a5ef626

Some more fixes.

4dbb5f5

javicalle suggested changes Oct 12, 2022

View reviewed changes

gmsoft-tuxicoman added 3 commits October 20, 2022 15:05

Update code according to PR comments part 1.

e768d8f

Convert to TuyaMCUCluster.

c148fcf

Linting fixes.

2d6a958

javicalle suggested changes Oct 20, 2022

View reviewed changes

Make attribute names match.

69aec27

javicalle approved these changes Oct 20, 2022

View reviewed changes

javicalle added the needs final review PR is ready for a final review from a maintainer label Oct 20, 2022

dmulcahey approved these changes Oct 22, 2022

View reviewed changes

dmulcahey merged commit cb10ea0 into zigpy:dev Oct 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for _TZE200_yi4jtqq1 illuminance sensor. #1795

Add support for _TZE200_yi4jtqq1 illuminance sensor. #1795

gmsoft-tuxicoman commented Sep 29, 2022

coveralls commented Sep 29, 2022 •

edited

Loading

codecov-commenter commented Sep 29, 2022 •

edited

Loading

dmulcahey commented Oct 1, 2022

dmulcahey commented Oct 10, 2022

javicalle left a comment

javicalle Oct 12, 2022

javicalle Oct 12, 2022

javicalle Oct 12, 2022

javicalle left a comment

javicalle Oct 12, 2022

javicalle commented Oct 12, 2022

gmsoft-tuxicoman commented Oct 20, 2022

javicalle left a comment

javicalle Oct 20, 2022

javicalle left a comment

		TUYA_BRIGHTNESS_LEVEL_ATTR = 0x01 # 0-2 "Low, Medium, High"
		TUYA_ILLUMINANCE_ATTR = 0x02 # [0, 0, 3, 232] illuminance

Add support for _TZE200_yi4jtqq1 illuminance sensor. #1795

Add support for _TZE200_yi4jtqq1 illuminance sensor. #1795

Conversation

gmsoft-tuxicoman commented Sep 29, 2022

coveralls commented Sep 29, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3291717546

💛 - Coveralls

codecov-commenter commented Sep 29, 2022 • edited Loading

Codecov Report

dmulcahey commented Oct 1, 2022

dmulcahey commented Oct 10, 2022

javicalle left a comment

Choose a reason for hiding this comment

javicalle Oct 12, 2022

Choose a reason for hiding this comment

javicalle Oct 12, 2022

Choose a reason for hiding this comment

javicalle Oct 12, 2022

Choose a reason for hiding this comment

javicalle left a comment

Choose a reason for hiding this comment

javicalle Oct 12, 2022

Choose a reason for hiding this comment

javicalle commented Oct 12, 2022

gmsoft-tuxicoman commented Oct 20, 2022

javicalle left a comment

Choose a reason for hiding this comment

javicalle Oct 20, 2022

Choose a reason for hiding this comment

javicalle left a comment

Choose a reason for hiding this comment

coveralls commented Sep 29, 2022 •

edited

Loading

codecov-commenter commented Sep 29, 2022 •

edited

Loading