Add Tuya cover `_TZE200_eevqq1uv` #3114

matt-sullivan · 2024-04-17T07:01:47Z

Similar to other Tuya window cover devices, but reports multiple tuya data points in set_data_response, so this implementation is based on the newer TuyaMCUCluster rather than TuyaManufCluster.

Sold as Zemismart ZM25R3 roller blind motor.

Proposed change

Additional information

Fixes #3068

Once merged it might make sense to change other existing tuya covers to use the new cluster implementations added in this PR. I think they could be replaced/consolidated with very little change, but I'm not sure how to how to test that sort of refactoring.

Checklist

The changes are tested and work correctly
pre-commit checks pass / the code has been formatted using Black
Tests have been added to verify that the new code works

codecov · 2024-04-17T07:03:17Z

Codecov Report

Attention: Patch coverage is 96.06299% with 5 lines in your changes missing coverage. Please review.

Project coverage is 89.58%. Comparing base (263e68c) to head (ad529f3).
Report is 1 commits behind head on dev.

Files with missing lines	Patch %	Lines
zhaquirks/tuya/mcu/__init__.py	96.77%	3 Missing ⚠️
zhaquirks/tuya/__init__.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3114      +/-   ##
==========================================
+ Coverage   89.44%   89.58%   +0.13%     
==========================================
  Files         311      311              
  Lines       10031    10136     +105     
==========================================
+ Hits         8972     9080     +108     
+ Misses       1059     1056       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/test_tuya_cover.py

matt-sullivan · 2024-04-17T07:17:38Z

zhaquirks/tuya/mcu/__init__.py

+    attributes = WindowCovering.attributes.copy()
+    # cover direction and lift percent control attributes are not very useful to HA, but needed to
+    # allow TuyaMCUCluster to map to the data point.
+    attributes.update({ATTR_COVER_DIRECTION: (ATTR_COVER_DIRECTION_NAME, t.enum8)})


I'm surprised I need a new attribute to track motor status (i.e. opening, closing, stopped) but I couldn't find anything in the standard zigbee WindowCovering.

TheJulianJES

Nice job, especially on the test coverage!
I'll have a closer look at this soon.

TheJulianJES · 2024-04-23T22:55:25Z

zhaquirks/tuya/mcu/__init__.py

+    def __init__(self, *args, **kwargs):
+        """Init."""
+        super().__init__(*args, **kwargs)


This can be removed.

Done. Although it's a bit awkward if the DPToAttributeMapping converter needs to refer to self. I worked around it by instead using data_point_handlers, which finds a function by name and will call an instance method.

matt-sullivan · 2024-04-23T23:48:18Z

Nice job, especially on the test coverage! I'll have a closer look at this soon.

Thanks @TheJulianJES. Since raising the PR I've figured out some more tuya data points, and I've started changing it to use the quirks v2 registration. If you'd prefer to wait a few days it might be easier to review it all at once?

TheJulianJES · 2024-04-23T23:51:49Z

Yep, feel free to change the PR.

Btw., if you're exposing custom entities to ZHA via quirks v2, the translation keys are added with the "quirks bump PR" in HA. Just let me know what translation keys I should add to the file when bumping quirks then. (only applicable if you're using custom entities)

stast1 · 2024-04-24T07:22:43Z

I tested with _TZE200_68nvbio9 and it works great! I propose to add _TZE200_68nvbio9 here

matt-sullivan · 2024-05-22T12:05:09Z

@TheJulianJES , this PR is now ready for review again.

It turned out to be much bigger than I expected, including some refactoring and a new base class. I'm happy for feedback to adjust the design, but thought it best to publish a proof of concept to show you.

The new base class (TuyaCommandCluster) is used to provide generic processing of zigbee commands into tuya set_data commands, without needing to define cluster attributes for tuya data points that don't have sensible values to show. It does this similar to how TuyaMCUCluster maps cluster attributes to tuya set_data commands with dp_to_attribute.

matt-sullivan · 2024-05-22T13:59:37Z

zhaquirks/tuya/ts0601_cover.py

+        entity_type=EntityType.STANDARD,
+        translation_key="small_step_open",
+    ).command_button(
+        WINDOW_COVER_COMMAND_SMALL_STEP_NAME,


I think these command definitions are correct, but they currently cause an error 'Platform zha does not generate unique IDs'

I think that's because the zha component uses the command id to generate an unique id, but does not expect multiple instances of the same command or take the command parameters into account. I've created a proof of concept change to zha matt-sullivan/home-assistant-core@e7bdec2 that works by adding a simple index to each unique id.

I'm not sure if that's a good way to do it. Maybe the quirk should specify an id, or zha should include the translation_key in the id?

Uh, good that you noticed this!
There may be some bigger ZHA refactor changes coming soon. Hoping I don't forget about this, so this can be addressed soon-ish.

matt-sullivan · 2024-05-22T14:06:37Z

zhaquirks/tuya/ts0601_cover.py

+        entity_type=EntityType.STANDARD,
+        translation_key="small_step_open",
+    ).command_button(
+        WINDOW_COVER_COMMAND_SMALL_STEP_NAME,


These command definitions create HA entities with ids like button.[device name]none[int]. I.e. there doesn't seem to be any sensible naming applied to the entity. is that expected behaviour, or something wrong with these definitions?

It's not really a problem, but feels like it could be better. Could the zha component use the translation key, command name, or another parameter to set the entity id?

matt-sullivan · 2024-05-22T14:21:51Z

@TheJulianJES, I created a zha commit with the translation keys this needs: matt-sullivan/home-assistant-core@1dff881

matt-sullivan · 2024-06-27T13:27:54Z

Hi @TheJulianJES, I'm sure you're busy, but could you please review and approve or provide feedback on this PR soon?

TheJulianJES · 2024-10-15T23:16:03Z

I know it's been a long time. I've still not forgotten about this time, sorry that I couldn't get to it yet.
There are some smaller quirks v2 changes (just including fallback_name and using QuirkBuilder(manf, model) and calling .add_to_registry() at the end).
I'll come back to this once some other Tuya related changes are merged (and I find some time). I may be able to rebase it then.
After that, we can hopefully merge it and hope that existing Tuya covers don't break lol.

matt-sullivan · 2024-10-16T13:29:43Z

Thanks @TheJulianJES. If you plan to look at it soon I can update it for those quirks v2 changes you mentioned.

Please let me know if there's anything else I can do to make it easier to review. At one stage I had implemented mostly in one new class/quirk, but I gradually understood how to re-use and share more of the existing Tuya code. If there's anything about the design you're not happy with I'm happy to change it.

I think it's low risk to existing Tuya covers - most of the changes are either new code or simple renaming of existing variables and methods. At least one person tested an early version of this, but I'm not sure if/how we can test other covers before it's merged.

Similar to other Tuya window cover devices, but reports multiple tuya data points in set_data_response, so this implementation is based on the newer TuyaMCUCluster rather than TuyaManufCluster. Sold as Zemismart ZM25R3 roller blind motor.

…erage

TuyaManufCluster has a TUYA_MCU_COMMAND listener that is passed a command to send. Many existing calls to TUYA_MCU_COMMAND were calling the TuyaMCUCluster variant which takes a TuyaClusterData structure. I've renamed those calls to TUYA_MCU_SET_CLUSTER_DATA so sit alongside TUYA_MCU_SET_DATAPOINTS and distinguish the data type they accept.

…points

…itialization/state

Update tests to use new zigpy request kwarg format

matt-sullivan commented Apr 17, 2024

View reviewed changes

tests/test_tuya_cover.py Outdated Show resolved Hide resolved

matt-sullivan commented Apr 17, 2024

View reviewed changes

matt-sullivan marked this pull request as ready for review April 17, 2024 22:23

TheJulianJES added Tuya Request/PR regarding a Tuya device has tests PR has tests labels Apr 21, 2024

TheJulianJES reviewed Apr 23, 2024

View reviewed changes

matt-sullivan force-pushed the dev branch from 63128c5 to 44c5d9b Compare May 22, 2024 11:26

matt-sullivan commented May 22, 2024

View reviewed changes

TheJulianJES added the needs review This PR should be reviewed soon label Jul 1, 2024

matt-sullivan added 11 commits October 30, 2024 00:18

Add Tuya cover _TZE200_eevqq1uv

d489d4f

Similar to other Tuya window cover devices, but reports multiple tuya data points in set_data_response, so this implementation is based on the newer TuyaMCUCluster rather than TuyaManufCluster. Sold as Zemismart ZM25R3 roller blind motor.

Add unit test for tuya cover unsupported command to increase test cov…

d545939

…erage

Change to quirk v2, add support for blind direction and limit settings

cdae82c

Rename TUYA_MCU_SET_DATA to TUYA_MCU_SET_DATAPOINTS

45144e3

Add TuyaCommandCluster to process commands based on map to tuya data …

34a508d

…points

Reorder tuya window cluster classes

785356c

Refactor TuyaNewManufClusterForWindowCover to avoid needing object in…

699dd05

…itialization/state

Update command_to_dp map to handle WINDOW_COVER_COMMAND_LIFTPERCENT

9732e7a

Use command_to_dp to implement main window cover move commands

8641f46

Fix async read_attributes override

e25cb6a

matt-sullivan added 2 commits October 30, 2024 00:18

Removed unused import

f446e66

Update tuya cover to latest quirks v2

51a30f0

Update tests to use new zigpy request kwarg format

matt-sullivan force-pushed the dev branch from 44c5d9b to 51a30f0 Compare October 29, 2024 14:31

Apply pre-commit auto fixes

ad529f3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Tuya cover `_TZE200_eevqq1uv` #3114

Add Tuya cover `_TZE200_eevqq1uv` #3114

matt-sullivan commented Apr 17, 2024 •

edited

Loading

codecov bot commented Apr 17, 2024 •

edited

Loading

matt-sullivan Apr 17, 2024

TheJulianJES left a comment

TheJulianJES Apr 23, 2024

matt-sullivan May 22, 2024

matt-sullivan commented Apr 23, 2024

TheJulianJES commented Apr 23, 2024

stast1 commented Apr 24, 2024

matt-sullivan commented May 22, 2024

matt-sullivan May 22, 2024

TheJulianJES May 28, 2024

matt-sullivan May 22, 2024

matt-sullivan commented May 22, 2024

matt-sullivan commented Jun 27, 2024

TheJulianJES commented Oct 15, 2024

matt-sullivan commented Oct 16, 2024

Add Tuya cover _TZE200_eevqq1uv #3114

Are you sure you want to change the base?

Add Tuya cover _TZE200_eevqq1uv #3114

Conversation

matt-sullivan commented Apr 17, 2024 • edited Loading

Proposed change

Additional information

Checklist

codecov bot commented Apr 17, 2024 • edited Loading

Codecov Report

matt-sullivan Apr 17, 2024

Choose a reason for hiding this comment

TheJulianJES left a comment

Choose a reason for hiding this comment

TheJulianJES Apr 23, 2024

Choose a reason for hiding this comment

matt-sullivan May 22, 2024

Choose a reason for hiding this comment

matt-sullivan commented Apr 23, 2024

TheJulianJES commented Apr 23, 2024

stast1 commented Apr 24, 2024

matt-sullivan commented May 22, 2024

matt-sullivan May 22, 2024

Choose a reason for hiding this comment

TheJulianJES May 28, 2024

Choose a reason for hiding this comment

matt-sullivan May 22, 2024

Choose a reason for hiding this comment

matt-sullivan commented May 22, 2024

matt-sullivan commented Jun 27, 2024

TheJulianJES commented Oct 15, 2024

matt-sullivan commented Oct 16, 2024

Add Tuya cover `_TZE200_eevqq1uv` #3114

Add Tuya cover `_TZE200_eevqq1uv` #3114

matt-sullivan commented Apr 17, 2024 •

edited

Loading

codecov bot commented Apr 17, 2024 •

edited

Loading