Repository: zigpy/zha-device-handlers
Pull request #3438 (2024-10-17 04:03:18)

Send all button press events for Sinope SW2500ZB/DM2500ZB
## Proposed change
<!--
  Explain your proposed change below.
-->
Added support for both press and release button events in order to properly support short and long press events.
Also added the pressed button and text description in the event arguments to simplify writing automations processing it.

ZHA now send both when we start pressing the Switch's up or down button, and when we release. Before, we were only sending events when a button was released after being pressed for a long time, or if double pressed, limiting possibilities. Now all events are sent: pressed, short released, long released and double pressed.

Note that I also added two extra arguments the the send `zha_event` because it was a bit hard to extract which button was pressed (up or down). It's now a new attribute `{button: turn_on}` or `{button: turn_off}` and a `description` that contains the `ActionButton` name (Pressed_on, Released_off, Double_on, etc.)

## Additional information
<!--
  Please include any additional information that is important to this PR.
  For example, if this PR is a potentially breaking change, mention that here.
  If this PR requires other PRs to be merged in HA Core or other projects, mention that.
  Lastly, if this PR fixes a specific issue, please include "Fixes #xxxx".
-->
I have been using this as a custom quirks for a while. I have two dimmer switches DM2500ZB dimmer switches and I'm emulating a 3-way dimmers with the downstairs dimmer using an HA automation to control upstairs dimmer that has the stairs lights connect to it. It works fine.

I also have two SW2500ZB, the events are also sent as expected.


## Checklist
<!--
  Put an 'x' in all boxes that apply.
  Note: You do not need to tick all boxes before creating a PR.
-->

- [x] The changes are tested and work correctly
- [x] `pre-commit` checks pass / the code has been formatted using Black
- [x] Tests have been added to verify that the new code works


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zigpy/zha-device-handlers/pull/3438?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) Report
Attention: Patch coverage is `92.30769%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 89.22%. Comparing base [(`fa734ad`)](https://app.codecov.io/gh/zigpy/zha-device-handlers/commit/fa734ad56de6e502d3483a25773e02ba6f45a571?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) to head [(`f9df3d8`)](https://app.codecov.io/gh/zigpy/zha-device-handlers/commit/f9df3d85250231b4d83eed3d32dcb4818e3050b5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy).
> Report is 9 commits behind head on dev.

| [Files with missing lines](https://app.codecov.io/gh/zigpy/zha-device-handlers/pull/3438?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) | Patch % | Lines |
|---|---|---|
| [zhaquirks/sinope/light.py](https://app.codecov.io/gh/zigpy/zha-device-handlers/pull/3438?src=pr&el=tree&filepath=zhaquirks%2Fsinope%2Flight.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-emhhcXVpcmtzL3Npbm9wZS9saWdodC5weQ==) | 91.30% | [2 Missing :warning: ](https://app.codecov.io/gh/zigpy/zha-device-handlers/pull/3438?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##              dev    #3438      +/-   ##
==========================================
+ Coverage   88.71%   89.22%   +0.51%     
==========================================
  Files         306      307       +1     
  Lines        9820     9897      +77     
==========================================
+ Hits         8712     8831     +119     
+ Misses       1108     1066      -42     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/zigpy/zha-device-handlers/pull/3438?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy).

bassdr: @claudegel FYI.
bassdr: I've tested my latest changes in HA and it works fine.

Patch:
diff --git a/zhaquirks/sinope/__init__.py b/zhaquirks/sinope/__init__.py
index 1a2bffb..ff59f31 100644
--- a/zhaquirks/sinope/__init__.py
+++ b/zhaquirks/sinope/__init__.py
@@ -1,21 +1,25 @@
 """Module for Sinope quirks implementations."""
 
 from zigpy.quirks import CustomCluster
+import zigpy.types as t
 from zigpy.zcl.clusters.general import DeviceTemperature
 
 from zhaquirks.const import (
     ARGS,
     ATTRIBUTE_ID,
     ATTRIBUTE_NAME,
+    BUTTON,
     CLUSTER_ID,
     COMMAND,
-    COMMAND_BUTTON_DOUBLE,
-    COMMAND_BUTTON_HOLD,
-    COMMAND_BUTTON_SINGLE,
+    COMMAND_M_INITIAL_PRESS,
+    COMMAND_M_LONG_RELEASE,
+    COMMAND_M_MULTI_PRESS_COMPLETE,
+    COMMAND_M_SHORT_RELEASE,
     DOUBLE_PRESS,
     ENDPOINT_ID,
     LONG_PRESS,
     SHORT_PRESS,
+    SHORT_RELEASE,
     TURN_OFF,
     TURN_ON,
     VALUE,
@@ -25,42 +29,108 @@ SINOPE = "Sinope Technologies"
 SINOPE_MANUFACTURER_CLUSTER_ID = 0xFF01
 ATTRIBUTE_ACTION = "action_report"
 
+
+class ButtonAction(t.enum8):
+    """Action_report values."""
+
+    Pressed_on = 0x01
+    Released_on = 0x02
+    Long_on = 0x03
+    Double_on = 0x04
+    Pressed_off = 0x11
+    Released_off = 0x12
+    Long_off = 0x13
+    Double_off = 0x14
+
+
 LIGHT_DEVICE_TRIGGERS = {
     (SHORT_PRESS, TURN_ON): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_SINGLE,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 2},
+        COMMAND: COMMAND_M_INITIAL_PRESS,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_ON,
+            VALUE: ButtonAction.Pressed_on,
+        },
     },
     (SHORT_PRESS, TURN_OFF): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_SINGLE,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 18},
+        COMMAND: COMMAND_M_INITIAL_PRESS,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_OFF,
+            VALUE: ButtonAction.Pressed_off,
+        },
+    },
+    (SHORT_RELEASE, TURN_ON): {
+        ENDPOINT_ID: 1,
+        CLUSTER_ID: 65281,
+        COMMAND: COMMAND_M_SHORT_RELEASE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_ON,
+            VALUE: ButtonAction.Released_on,
+        },
+    },
+    (SHORT_RELEASE, TURN_OFF): {
+        ENDPOINT_ID: 1,
+        CLUSTER_ID: 65281,
+        COMMAND: COMMAND_M_SHORT_RELEASE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_OFF,
+            VALUE: ButtonAction.Released_off,
+        },
     },
     (DOUBLE_PRESS, TURN_ON): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_DOUBLE,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 4},
+        COMMAND: COMMAND_M_MULTI_PRESS_COMPLETE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_ON,
+            VALUE: ButtonAction.Double_on,
+        },
     },
     (DOUBLE_PRESS, TURN_OFF): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_DOUBLE,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 20},
+        COMMAND: COMMAND_M_MULTI_PRESS_COMPLETE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_OFF,
+            VALUE: ButtonAction.Double_off,
+        },
     },
     (LONG_PRESS, TURN_ON): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_HOLD,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 3},
+        COMMAND: COMMAND_M_LONG_RELEASE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_ON,
+            VALUE: ButtonAction.Long_on,
+        },
     },
     (LONG_PRESS, TURN_OFF): {
         ENDPOINT_ID: 1,
         CLUSTER_ID: 65281,
-        COMMAND: COMMAND_BUTTON_HOLD,
-        ARGS: {ATTRIBUTE_ID: 84, ATTRIBUTE_NAME: ATTRIBUTE_ACTION, VALUE: 19},
+        COMMAND: COMMAND_M_LONG_RELEASE,
+        ARGS: {
+            ATTRIBUTE_ID: 84,
+            ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
+            BUTTON: TURN_OFF,
+            VALUE: ButtonAction.Long_off,
+        },
     },
 }
 
diff --git a/zhaquirks/sinope/light.py b/zhaquirks/sinope/light.py
index a95d4b3..328315f 100644
--- a/zhaquirks/sinope/light.py
+++ b/zhaquirks/sinope/light.py
@@ -29,14 +29,20 @@ from zhaquirks import EventableCluster
 from zhaquirks.const import (
     ATTRIBUTE_ID,
     ATTRIBUTE_NAME,
-    COMMAND_BUTTON_DOUBLE,
-    COMMAND_BUTTON_HOLD,
+    BUTTON,
+    COMMAND_M_INITIAL_PRESS,
+    COMMAND_M_LONG_RELEASE,
+    COMMAND_M_MULTI_PRESS_COMPLETE,
+    COMMAND_M_SHORT_RELEASE,
+    DESCRIPTION,
     DEVICE_TYPE,
     ENDPOINTS,
     INPUT_CLUSTERS,
     MODELS_INFO,
     OUTPUT_CLUSTERS,
     PROFILE_ID,
+    TURN_OFF,
+    TURN_ON,
     VALUE,
     ZHA_SEND_EVENT,
 )
@@ -45,6 +51,7 @@ from zhaquirks.sinope import (
     LIGHT_DEVICE_TRIGGERS,
     SINOPE,
     SINOPE_MANUFACTURER_CLUSTER_ID,
+    ButtonAction,
     CustomDeviceTemperatureCluster,
 )
 
@@ -73,19 +80,6 @@ class DoubleFull(t.enum8):
     On = 0x01
 
 
-class ButtonAction(t.enum8):
-    """Action_report values."""
-
-    Single_on = 0x01
-    Single_release_on = 0x02
-    Long_on = 0x03
-    Double_on = 0x04
-    Single_off = 0x11
-    Single_release_off = 0x12
-    Long_off = 0x13
-    Double_off = 0x14
-
-
 class SinopeTechnologiesManufacturerCluster(CustomCluster):
     """SinopeTechnologiesManufacturerCluster manufacturer cluster."""
 
@@ -191,29 +185,53 @@ class SinopeTechnologiesManufacturerCluster(CustomCluster):
                 hdr, args, dst_addressing=dst_addressing
             )
 
-        value = attr.value.value
+        action = self.Action(attr.value.value)
+
+        command, button = self._get_command_from_action(action)
+        if not command or not button:
+            return
+
         event_args = {
             ATTRIBUTE_ID: 84,
             ATTRIBUTE_NAME: ATTRIBUTE_ACTION,
-            VALUE: value.value,
+            BUTTON: button,
+            DESCRIPTION: action.name,
+            VALUE: action.value,
         }
-        action = self._get_command_from_action(self.Action(value))
-        if not action:
-            return
-        self.listener_event(ZHA_SEND_EVENT, action, event_args)
 
-    def _get_command_from_action(self, action: ButtonAction) -> str | None:
-        # const lookup = {2: 'up_single', 3: 'up_hold', 4: 'up_double',
-        #             18: 'down_single', 19: 'down_hold', 20: 'down_double'};
+        self.debug(
+            "SINOPE ZHA_SEND_EVENT command: '%s' event_args: %s",
+            command,
+            event_args,
+        )
+
+        self.listener_event(ZHA_SEND_EVENT, command, event_args)
+
+    def _get_command_from_action(
+        self, action: ButtonAction
+    ) -> tuple[str | None, str | None]:
+        # const lookup = {1: 'up_single', 2: 'up_single_released', 3: 'up_hold', 4: 'up_double',
+        #             17: 'down_single, 18: 'down_single_released', 19: 'down_hold', 20: 'down_double'};
         match action:
-            case self.Action.Single_off | self.Action.Single_on:
-                return None
-            case self.Action.Double_off | self.Action.Double_on:
-                return COMMAND_BUTTON_DOUBLE
-            case self.Action.Long_off | self.Action.Long_on:
-                return COMMAND_BUTTON_HOLD
+            case self.Action.Pressed_off:
+                return COMMAND_M_INITIAL_PRESS, TURN_OFF
+            case self.Action.Pressed_on:
+                return COMMAND_M_INITIAL_PRESS, TURN_ON
+            case self.Action.Released_off:
+                return COMMAND_M_SHORT_RELEASE, TURN_OFF
+            case self.Action.Released_on:
+                return COMMAND_M_SHORT_RELEASE, TURN_ON
+            case self.Action.Double_off:
+                return COMMAND_M_MULTI_PRESS_COMPLETE, TURN_OFF
+            case self.Action.Double_on:
+                return COMMAND_M_MULTI_PRESS_COMPLETE, TURN_ON
+            case self.Action.Long_off:
+                return COMMAND_M_LONG_RELEASE, TURN_OFF
+            case self.Action.Long_on:
+                return COMMAND_M_LONG_RELEASE, TURN_ON
             case _:
-                return None
+                self.debug("SINOPE unhandled action: %s", action)
+                return None, None
 
 
 class LightManufacturerCluster(EventableCluster, SinopeTechnologiesManufacturerCluster):


Test patch:
diff --git a/tests/test_sinope.py b/tests/test_sinope.py
index 0680d45..3f377a8 100644
--- a/tests/test_sinope.py
+++ b/tests/test_sinope.py
@@ -11,7 +11,14 @@ from zigpy.zcl.clusters.measurement import FlowMeasurement
 
 from tests.common import ClusterListener
 import zhaquirks
-from zhaquirks.const import COMMAND_BUTTON_DOUBLE, COMMAND_BUTTON_HOLD
+from zhaquirks.const import (
+    COMMAND_M_INITIAL_PRESS,
+    COMMAND_M_LONG_RELEASE,
+    COMMAND_M_MULTI_PRESS_COMPLETE,
+    COMMAND_M_SHORT_RELEASE,
+    TURN_OFF,
+    TURN_ON,
+)
 from zhaquirks.sinope import SINOPE_MANUFACTURER_CLUSTER_ID
 from zhaquirks.sinope.light import (
     SinopeTechnologieslight,
@@ -92,20 +99,22 @@ def _get_packet_data(
 
 @pytest.mark.parametrize("quirk", (SinopeTechnologieslight,))
 @pytest.mark.parametrize(
-    "press_type,exp_event",
+    "press_type,button,exp_event",
     (
-        (ButtonAction.Single_off, None),
-        (ButtonAction.Single_on, None),
-        (ButtonAction.Double_on, COMMAND_BUTTON_DOUBLE),
-        (ButtonAction.Double_off, COMMAND_BUTTON_DOUBLE),
-        (ButtonAction.Long_on, COMMAND_BUTTON_HOLD),
-        (ButtonAction.Long_off, COMMAND_BUTTON_HOLD),
+        (ButtonAction.Pressed_off, TURN_OFF, COMMAND_M_INITIAL_PRESS),
+        (ButtonAction.Pressed_on, TURN_ON, COMMAND_M_INITIAL_PRESS),
+        (ButtonAction.Released_off, TURN_OFF, COMMAND_M_SHORT_RELEASE),
+        (ButtonAction.Released_on, TURN_ON, COMMAND_M_SHORT_RELEASE),
+        (ButtonAction.Double_on, TURN_ON, COMMAND_M_MULTI_PRESS_COMPLETE),
+        (ButtonAction.Double_off, TURN_OFF, COMMAND_M_MULTI_PRESS_COMPLETE),
+        (ButtonAction.Long_on, TURN_ON, COMMAND_M_LONG_RELEASE),
+        (ButtonAction.Long_off, TURN_OFF, COMMAND_M_LONG_RELEASE),
         # Should gracefully handle broken actions.
-        (t.uint8_t(0x00), None),
+        (t.uint8_t(0x00), None, None),
     ),
 )
 async def test_sinope_light_switch(
-    zigpy_device_from_quirk, quirk, press_type, exp_event
+    zigpy_device_from_quirk, quirk, press_type, button, exp_event
 ):
     """Test that button presses are sent as events."""
     device: Device = zigpy_device_from_quirk(quirk)
@@ -126,7 +135,16 @@ async def test_sinope_light_switch(
         ),
     )
     data = _get_packet_data(foundation.GeneralCommand.Report_Attributes, attr)
-    device.handle_message(260, cluster_id, endpoint_id, endpoint_id, data)
+
+    device.packet_received(
+        t.ZigbeePacket(
+            profile_id=260,
+            cluster_id=cluster_id,
+            src_ep=endpoint_id,
+            dst_ep=endpoint_id,
+            data=t.SerializableBytes(data),
+        )
+    )
 
     if exp_event is None:
         assert cluster_listener.zha_send_event.call_count == 0
@@ -137,6 +155,8 @@ async def test_sinope_light_switch(
             {
                 "attribute_id": 84,
                 "attribute_name": "action_report",
+                "button": button,
+                "description": press_type.name,
                 "value": press_type.value,
             },
         )
@@ -162,7 +182,15 @@ async def test_sinope_light_switch_non_action_report(zigpy_device_from_quirk, qu
 
     # read attributes general command
     data = _get_packet_data(foundation.GeneralCommand.Read_Attributes)
-    device.handle_message(260, cluster_id, endpoint_id, endpoint_id, data)
+    device.packet_received(
+        t.ZigbeePacket(
+            profile_id=260,
+            cluster_id=cluster_id,
+            src_ep=endpoint_id,
+            dst_ep=endpoint_id,
+            data=t.SerializableBytes(data),
+        )
+    )
     # no ZHA events emitted because we only handle Report_Attributes
     assert cluster_listener.zha_send_event.call_count == 0
 
@@ -174,7 +202,15 @@ async def test_sinope_light_switch_non_action_report(zigpy_device_from_quirk, qu
         ),  # 0x29 = t.int16s
     )
     data = _get_packet_data(foundation.GeneralCommand.Report_Attributes, attr)
-    device.handle_message(260, cluster_id, endpoint_id, endpoint_id, data)
+    device.packet_received(
+        t.ZigbeePacket(
+            profile_id=260,
+            cluster_id=cluster_id,
+            src_ep=endpoint_id,
+            dst_ep=endpoint_id,
+            data=t.SerializableBytes(data),
+        )
+    )
     # ZHA event emitted because we pass non "action_report"
     # reports to the base class handler.
     assert cluster_listener.zha_send_event.call_count == 1