Add device support for TS110E Model QS-Zigbee-D02-TRIAC-LN #1422

jonnylangefeld · 2022-03-09T02:35:54Z

This device uses a non-default set level command.

This device looks exactly the same as TS110F but seems to have different zigbee command IDs. This is TS110F on blakadder: https://zigbee.blakadder.com/Lonsonho_QS-Zigbee-D02-TRIAC-LN.html

Tested in local docker environment.
Thanks to @javicalle this was possible.
Fix #1415

@javicalle

This device uses a non-default set level command. Tested in local docker environment. Thanks to @javicalle this was possible. Fix zigpy#1415

javicalle

A little fix for the checks

javicalle · 2022-03-09T08:38:00Z

zhaquirks/tuya/ts110e.py

+                    Scenes.cluster_id,
+                    OnOff.cluster_id,
+                    F000LevelControlCluster,
+                    TuyaZBExternalSwitchTypeCluster.cluster_id,


remove the '.cluster_id' part:

TuyaZBExternalSwitchTypeCluster,

I think first testing if the cluster / attribute is working on the device or not and is its not working then deleting it in the replacement (its always possible adding it later if its needed) but its very likely not working on this device.

The implementation is here:

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

Lines 887 to 902 in 8ae597d

# Tuya Zigbee Cluster 0xE001 Implementation

class ExternalSwitchType(t.enum8):

"""Tuya external switch type enum."""

Toggle = 0x00

State = 0x01

Momentary = 0x02

class TuyaZBExternalSwitchTypeCluster(CustomCluster):

"""Tuya External Switch Type Cluster."""

name = "Tuya External Switch Type Cluster"

cluster_id = TUYA_CLUSTER_E001_ID

ep_attribute = "tuya_external_switch_type"

attributes = {0xD030: ("external_switch_type", ExternalSwitchType)}

I actually noticed that I had to leave in the .cluster_id part, otherwise the quirk wouldn't show up on the device anymore.

javicalle · 2022-03-09T08:39:50Z

zhaquirks/tuya/ts110e.py

+class TuyaLevelPayload(t.Struct):
+    """Tuya Level payload."""
+
+    level: t.uint16_t
+    transtime: t.uint16_t


It should probably move to tuya.__init__.py, but let's keep it here for now.

Or first do the implementation in the quirk and getting it merged and its working OK and then making one more PR for moving it to tuya INIT then tuya INIT is little tricky with many addings and can needing rebasing many times and its PITA (i have one PR waiting for being merged to it).

Yes, that's the idea.
Keep here and merge. When required, move to Tuya INIT. 👍🏻

jonnylangefeld · 2022-06-11T21:16:49Z

zhaquirks/tuya/ts110e.py

+    HALOGEN = 0x02
+
+
+class F000LevelControlCluster(LevelControl):


This LevelControl part is actually making issues. Because it applies to all dimmers in my house now, even the non-Tuya ones. And those obviously don't need this quirk. I think I somehow need to incorporate CustomCluster somewhere. But just adding class F000LevelControlCluster(CustomCluster, LevelControl): doesn't work.
@javicalle do you know how I can change this quirk to only work with the Custom device DimmerSwitchWithNeutral1Gang below?

Not sure what you mean. The F000LevelControlCluster cluster definition is only used in the DimmerSwitchWithNeutral1Gang quirk. An this quirk only applies to _TZ3210_ngqk6jia device.

What is exactly the problem? When they started? Have you updated HA?

If you believe that there's a problem with your quirk, try to remove it an check if the problem persist, but I don't know how that quirk can affect other devices than the _TZ3210_ngqk6jia.

So the zhaquirks.tuya.ts110e.DimmerSwitchWithNeutral1Gang quirk didn't show up on the new dimmer (non tuya) I bought, but the F000LevelControlCluster showed up on the new dimmer I bought. I found that that is because we just used class F000LevelControlCluster(LevelControl):, without implementing a CustomCluster.
I have experimented some more and found that adding CustomCluster as mentioned in my last comment didn't work, because that sent the manufacturer ID to the device and the device didn't recognize that. I found out by continuously comparing the logs between the setting that worked and the one that didn't work.

Long story short, I had to add NoManufacturerCluster to make it work. That is only in the later versions of zha-device-handlers, so I had to update my entire home-assistance instance. I have done that now and updated my quirk to use class F000LevelControlCluster(NoManufacturerCluster, LevelControl):. And that ended up doing what I wanted which is that the F000LevelControlCluster only applies to the tuya dimmer and not to others.

I will update this PR accordingly as there are some other changes with the latest updates on the dev branch.

codecov-commenter · 2022-06-13T01:54:06Z

Codecov Report

Merging #1422 (25ab689) into dev (e3fa034) will decrease coverage by 0.23%.
The diff coverage is 72.50%.

@@            Coverage Diff             @@
##              dev    #1422      +/-   ##
==========================================
- Coverage   80.52%   80.29%   -0.24%     
==========================================
  Files         222      233      +11     
  Lines        6414     7018     +604     
==========================================
+ Hits         5165     5635     +470     
- Misses       1249     1383     +134

Impacted Files	Coverage Δ
zhaquirks/tuya/ts110e.py	`72.50% <72.50%> (ø)`
zhaquirks/tuya/ts004f.py	`80.64% <0.00%> (-19.36%)`	⬇️
zhaquirks/tuya/ts0601_siren.py	`93.96% <0.00%> (-6.04%)`	⬇️
zhaquirks/xiaomi/aqara/tvoc.py	`73.80% <0.00%> (-2.67%)`	⬇️
zhaquirks/xiaomi/aqara/roller_curtain_e1.py	`59.70% <0.00%> (-2.57%)`	⬇️
zhaquirks/__init__.py	`77.89% <0.00%> (-1.48%)`	⬇️
zhaquirks/tuya/mcu/__init__.py	`98.77% <0.00%> (-1.23%)`	⬇️
zhaquirks/xiaomi/aqara/plug_mmeu01.py	`78.12% <0.00%> (-0.83%)`	⬇️
zhaquirks/xiaomi/__init__.py	`82.37% <0.00%> (-0.81%)`	⬇️
zhaquirks/xiaomi/mija/smoke.py	`89.47% <0.00%> (-0.53%)`	⬇️
... and 62 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3fa034...25ab689. Read the comment docs.

coveralls · 2022-06-13T01:54:22Z

Pull Request Test Coverage Report for Build 2489301802

29 of 40 (72.5%) changed or added relevant lines in 1 file are covered.
604 unchanged lines in 29 files lost coverage.
Overall coverage decreased (-0.2%) to 80.294%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
zhaquirks/tuya/ts110e.py	29	40	72.5%

Files with Coverage Reduction	New Missed Lines	%
zhaquirks/tuya/mcu/init.py	2	98.17%
zhaquirks/xiaomi/aqara/ctrl_ln.py	2	88.89%
zhaquirks/xiaomi/mija/smoke.py	2	89.47%
zhaquirks/aurora/aurora_dimmer.py	3	82.61%
zhaquirks/tuya/ts0601_electric_heating.py	3	57.58%
zhaquirks/danfoss/thermostat.py	4	82.76%
zhaquirks/tuya/ts130f.py	4	83.02%
zhaquirks/xiaomi/aqara/opple_remote.py	5	88.89%
zhaquirks/konke/init.py	6	75.68%
zhaquirks/tuya/ts004f.py	6	80.65%

Totals
Change from base Build 1947732782:	-0.2%
Covered Lines:	5635
Relevant Lines:	7018

💛 - Coveralls

jonnylangefeld · 2022-06-13T15:18:52Z

After lots of testing I'm pretty confident this is working code now. All the checks are succeeding too. I've also updated it to work with the new attributes and server_commands (vs the old manufacturer_server_attributes and manufacturer_attributes). @javicalle should we merge it?

javicalle · 2022-06-13T18:38:36Z

zhaquirks/tuya/ts110e.py

+        }
+    )
+
+    # 0xF000 reported values are 10-1000, convert to 0-254


The conversion is 1-255 (which I think is the range that HA handles).

And the values 10 & 1000 I think correspond to the manufacturer_min_level and manufacturer_max_level values.
If someone modifies the values that conversion can do strange things.
The truth is that I don't know what could be the best way to handle this situation, but it is probably something quite common in dimmers that would be addressed in a generic way.

I've experimented with this quite a while and this is what worked best for me. Would you agree to merging this as is (because it seems to work with the default manufacturer_min_level and manufacturer_max_level) and if someone in the future comes in and finds a better approach to calculate the values we just update the formula? At least this PR supports the device in the first place 😊

I agree. We can go ahead with the version as is.

javicalle · 2022-06-13T18:42:39Z

zhaquirks/tuya/ts110e.py

+            return await super().command(
+                TUYA_CUSTOM_LEVEL_COMMAND,
+                TuyaLevelPayload(level=brightness, transtime=0),
+                manufacturer=manufacturer,
+                expect_reply=expect_reply,
+                tsn=tsn,
+            )


This is fine to me, but probably you can get the same result without extending the NoManufacturerCluster and putting here:

manufacturer=foundation.ZCLHeader.NO_MANUFACTURER_ID,

Hmm that doesn't work because then this Class would also show up as cluster in the dimmers of other devices. F000LevelControlCluster has to inherit in some way from CustomCluster and NoManufacturerCluster does that for us (including setting manufacturer=foundation.ZCLHeader.NO_MANUFACTURER_ID,).

this Class would also show up as cluster in the dimmers of other devices

That sentence not make sense to me. A quirk shouldn't modify other devices that don't use the quirk.
Trying to figure out how can be done I believe that some code like this could make the described behaviour:

# attributes = LevelControl.attributes.copy() attributes.update( { # 0xF000 TUYA_LEVEL_ATTRIBUTE: ("manufacturer_current_level", t.uint16_t), } )

If not making a copy of parent class attributes, maybe the code is modifying the parent attributes and changes for any device that uses the parent class 🤔

In any case, your proposed code is fine to me.

Hmm yeah I don’t know, but it did show up as level control cluster on this new dimmer I bought…

javicalle

LGTM

jonnylangefeld added 3 commits March 8, 2022 18:33

Add device support for TS110E

cff1380

This device uses a non-default set level command. Tested in local docker environment. Thanks to @javicalle this was possible. Fix zigpy#1415

Lint

a3a37bf

Lint

afd2b8a

javicalle suggested changes Mar 9, 2022

View reviewed changes

jonnylangefeld added 2 commits March 12, 2022 15:34

Update ts110e.py

92a2f41

Update ts110e.py

cdd1184

jonnylangefeld commented Jun 11, 2022

View reviewed changes

Update ts110e.py

25ab689

Update ts110e.py

b4c49fd

javicalle reviewed Jun 13, 2022

View reviewed changes

javicalle approved these changes Jun 15, 2022

View reviewed changes

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

dmulcahey merged commit 5bbe4e0 into zigpy:dev Jul 8, 2022

AlpineWhite mentioned this pull request Jan 26, 2023

[Device Support Request] QS-Zigbee-D02-TRIAC-2C-LN - TS110E _TZ3210_pagajpog #1777

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add device support for TS110E Model QS-Zigbee-D02-TRIAC-LN #1422

Add device support for TS110E Model QS-Zigbee-D02-TRIAC-LN #1422

jonnylangefeld commented Mar 9, 2022

javicalle left a comment

javicalle Mar 9, 2022

MattWestb Mar 9, 2022

jonnylangefeld Jun 11, 2022

javicalle Mar 9, 2022

MattWestb Mar 9, 2022

javicalle Mar 9, 2022

jonnylangefeld Jun 11, 2022

javicalle Jun 12, 2022 •

edited

Loading

jonnylangefeld Jun 13, 2022

codecov-commenter commented Jun 13, 2022

coveralls commented Jun 13, 2022 •

edited

Loading

jonnylangefeld commented Jun 13, 2022

javicalle Jun 13, 2022

jonnylangefeld Jun 15, 2022

javicalle Jun 15, 2022

javicalle Jun 13, 2022

jonnylangefeld Jun 15, 2022

javicalle Jun 15, 2022

jonnylangefeld Jun 16, 2022

javicalle left a comment

	# Tuya Zigbee Cluster 0xE001 Implementation
	class ExternalSwitchType(t.enum8):
	"""Tuya external switch type enum."""

	Toggle = 0x00
	State = 0x01
	Momentary = 0x02


	class TuyaZBExternalSwitchTypeCluster(CustomCluster):
	"""Tuya External Switch Type Cluster."""

	name = "Tuya External Switch Type Cluster"
	cluster_id = TUYA_CLUSTER_E001_ID
	ep_attribute = "tuya_external_switch_type"
	attributes = {0xD030: ("external_switch_type", ExternalSwitchType)}

Add device support for TS110E Model QS-Zigbee-D02-TRIAC-LN #1422

Add device support for TS110E Model QS-Zigbee-D02-TRIAC-LN #1422

Conversation

jonnylangefeld commented Mar 9, 2022

javicalle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javicalle Jun 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 13, 2022

Codecov Report

coveralls commented Jun 13, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2489301802

💛 - Coveralls

jonnylangefeld commented Jun 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javicalle left a comment

Choose a reason for hiding this comment

javicalle Jun 12, 2022 •

edited

Loading

coveralls commented Jun 13, 2022 •

edited

Loading