Add support for QUAD-ZIG-SW from smarthjemmet.dk #3400

mortenmoulder · 2024-10-06T19:35:13Z

Proposed change

Add support for smarthjemmet.dk's QUAD-ZIG-SW device.

Additional information

Works with V1 and V2 firmware.

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

…dlers into dev

codecov · 2024-10-06T20:21:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.92%. Comparing base (8b3560e) to head (453ea0f).
Report is 4 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3400      +/-   ##
==========================================
+ Coverage   90.91%   90.92%   +0.01%     
==========================================
  Files         325      326       +1     
  Lines       10566    10584      +18     
==========================================
+ Hits         9606     9624      +18     
  Misses        960      960

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

…dlers into dev

mortenmoulder · 2024-10-06T20:24:25Z

All should be fixed now. Sorry for all the commits.

mortenmoulder · 2024-10-17T22:30:56Z

@TheJulianJES I see that a new release was created yesterday, and mine was not included. Any idea why? The only checks failing are code coverage, which shouldn't prevent it from being merged. Was really hoping to get this quirk implemented in the next release, as I'm guessing I have to wait a few more weeks now.

TheJulianJES · 2024-10-17T23:08:01Z

There's a few things on this PR that I'd likely change. I just haven't found the time to properly review this PR and comment all small change suggestions. I still plan to do that, of course, but there are also other PRs.
Yesterday's release was just to get some bigger ZHA related changes into nightly HA builds. The quirks related changes were mostly ones that didn't need feedback.

Follows the style of other quirks where there's proper quirk defined in a `CustomDevice` class that's inherited from in other quirks that just change the `signature`, but keep the `replacement` and device triggers.

TheJulianJES · 2024-10-18T00:19:46Z

Ok, so I've pushed multiple commits now. The "refactor" commit changes the structure of the (so-called v1) quirk to have a v1 device class with signature, replacement, and device triggers.
Then, a class for firmware version 2 is added that inherits from the first device class. It just changes the signature.
This is similar to the structure we have in other quirks.

The last commit (as of now) "Replace with v2 quirk" completely replaces the CustomDevice v1 quirks with a v2 quirk.
For it, we don't need to have a fixed signature and replacement. .replaces(CR2032PowerConfigurationCluster) will automatically replace all clusters with the same cluster id on endpoint 1. It's used to replace the default PowerConfiguration cluster with the custom CR2032PowerConfigurationCluster.

.replace_cluster_occurrences(CustomMultistateInputCluster) will look at all endpoints and replace all clusters with the same cluster id (of MultistateInput) it can find.

Please test if that v2 quirk works as expected.

With the v1 quirk before, the MultistateInput cluster was always an output/client cluster in the original signature. In the replacement, it was a input/server cluster then. It was also missing on endpoint 1 in the replacement. I don't think that was intentional?

If the replacement CustomMultistateInputCluster needs to be an input/server cluster (so the role swapped compared the original signature), we can do it like this in a v2 quirk (but for all endpoints):

    .removes(MultistateInput.cluster_id, cluster_type=ClusterType.Client, endpoint_id=1)  # remove output cluster
    .adds(CustomMultistateInputCluster, endpoint_id=1)  # default cluster_type is server, so it'll add an input cluster

The test here is no longer really needed for the v2 quirk. There's no real point in testing if all the correct clusters are included. Ideally, we would send messages from the device and see if they are converted to "ZHA events" as expected in a test.

TheJulianJES · 2024-10-18T00:26:18Z

zhaquirks/smarthjemmet/quadzigsw.py

+    async def bind(self):
+        """Prevent bind."""
+        return zigpy.zcl.foundation.Status.SUCCESS
+
+    async def unbind(self):
+        """Prevent unbind."""
+        return zigpy.zcl.foundation.Status.SUCCESS
+
+    async def _configure_reporting(self, *args, **kwargs):
+        """Prevent remote configure reporting."""
+        return (
+            zigpy.zcl.foundation.ConfigureReportingResponse.deserialize(b"\x00")[0],
+        )


Unrelated to converting the whole quirk to a v2 quirk:
Are these necessary? We already set SKIP_CONFIGURATION = True.

mortenmoulder · 2024-10-18T06:16:11Z

@TheJulianJES Hi and thanks for your effort. Unfortunately it's not working.

When I am talking about V1 and V2 in terms of firmware, it's because I have one physical device with two different firmware versions. The signature of the devices are completely different, which is why I need to be able to support both signatures on the same device. V1 of the firmware came out back in 2023, and now I've updated it in late 2024. Does that make sense?

For all I care, Quirk V1 or Quirk V2 doesn't matter to me. Quirk V2 looks better with the QuirkBuilder - definitely, but unfortunately it's not working as intended in this case.

Regards to your "It was also missing on endpoint 1 in the replacement. I don't think that was intentional" it actually was. The physical device has 5 buttons, but only 4 of them should work as MultistateInput. The last is used for resetting the device, which is done on a firmware level.

mortenmoulder · 2024-10-18T06:17:53Z

And for good measure, here's the old firmware (firmware V1 from 2023)'s device signature:

{
  "node_descriptor": {
    "logical_type": 2,
    "complex_descriptor_available": 0,
    "user_descriptor_available": 0,
    "reserved": 0,
    "aps_flags": 0,
    "frequency_band": 8,
    "mac_capability_flags": 128,
    "manufacturer_code": 4447,
    "maximum_buffer_size": 80,
    "maximum_incoming_transfer_size": 160,
    "server_mask": 0,
    "maximum_outgoing_transfer_size": 160,
    "descriptor_capability_field": 0
  },
  "endpoints": {
    "1": {
      "profile_id": "0x0104",
      "device_type": "0xfffe",
      "input_clusters": [
        "0x0000",
        "0x0001",
        "0x0007"
      ],
      "output_clusters": [
        "0x0000",
        "0x0001",
        "0x0012"
      ]
    },
    "2": {
      "profile_id": "0x0104",
      "device_type": "0xfffe",
      "input_clusters": [
        "0x0007"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "3": {
      "profile_id": "0x0104",
      "device_type": "0xfffe",
      "input_clusters": [
        "0x0007"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "4": {
      "profile_id": "0x0104",
      "device_type": "0xfffe",
      "input_clusters": [
        "0x0007"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "5": {
      "profile_id": "0x0104",
      "device_type": "0xfffe",
      "input_clusters": [
        "0x0007"
      ],
      "output_clusters": [
        "0x0012"
      ]
    }
  },
  "manufacturer": "smarthjemmet.dk",
  "model": "QUAD-ZIG-SW",
  "class": "zigpy.device.Device"
}

and here is the new firmware V2 (from late 2024) device signature:

{
  "node_descriptor": {
    "logical_type": 2,
    "complex_descriptor_available": 0,
    "user_descriptor_available": 0,
    "reserved": 0,
    "aps_flags": 0,
    "frequency_band": 8,
    "mac_capability_flags": 128,
    "manufacturer_code": 4447,
    "maximum_buffer_size": 80,
    "maximum_incoming_transfer_size": 160,
    "server_mask": 0,
    "maximum_outgoing_transfer_size": 160,
    "descriptor_capability_field": 0
  },
  "endpoints": {
    "1": {
      "profile_id": "0x0104",
      "device_type": "0x0006",
      "input_clusters": [
        "0x0000",
        "0x0001"
      ],
      "output_clusters": [
        "0x0000"
      ]
    },
    "2": {
      "profile_id": "0x0104",
      "device_type": "0x0006",
      "input_clusters": [
        "0x0007",
        "0x0012"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "3": {
      "profile_id": "0x0104",
      "device_type": "0x0006",
      "input_clusters": [
        "0x0007",
        "0x0012"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "4": {
      "profile_id": "0x0104",
      "device_type": "0x0006",
      "input_clusters": [
        "0x0007",
        "0x0012"
      ],
      "output_clusters": [
        "0x0012"
      ]
    },
    "5": {
      "profile_id": "0x0104",
      "device_type": "0x0006",
      "input_clusters": [
        "0x0007",
        "0x0012"
      ],
      "output_clusters": [
        "0x0012"
      ]
    }
  },
  "manufacturer": "smarthjemmet.dk",
  "model": "QUAD-ZIG-SW",
  "class": "QUAD-ZIG-SW.QUAD_ZIG_SW"
}

mortenmoulder · 2024-10-18T20:43:48Z

@TheJulianJES Help is not really wanted. What I originally had worked perfectly. Only thing not passing is the code coverage. Is that a requirement to get it merged?

TheJulianJES · 2024-10-18T20:45:45Z

"Help wanted" just means this needs to be looked again. A v2 quirk should work.
And yes, the code should be covered fully. That's not really a big issue though.

mortenmoulder · 2024-10-18T20:50:53Z

Okay I see. Is it for you to look at "at some point" as a reminder? Or someone else? My question really is, do I need to take action?

TheJulianJES · 2024-10-19T00:31:30Z

I'll take another look again at this soon. In the mean time, you can try to see if you can make the v2 quirk work if you want.
E.g. by copying/pasting the lines at the bottom from this comment into the quirk (replacing the replace_cluster_occurrences).
(Adding those two lines for endoints (1?) 2, 3, 4, 5.)

Regards to your "It was also missing on endpoint 1 in the replacement. I don't think that was intentional" it actually was. The physical device has 5 buttons, but only 4 of them should work as MultistateInput.

range(1, 6) gives us device triggers for five buttons though. That was also in the original code.
Isn't that one batch of device triggers too many then?
It's also a bit weird, because the original device signature has the MultistateInputCluster on all 5 endpoints (as an output cluster). Could be firmware thing of course, still weird though..

Unfortunately it's not working.

On which HA version did you test the v2 quirk? Did you test it by replacing the whole quirks library or just inserting the one quirks file as a "custom quirk" (using the ZHA config option) (recommended for testing this).
Also, did the quirk apply at least? E.g., in device settings in HA or in the device signature, was there a CustomDeviceV2 quirk used?

mortenmoulder · 2024-10-19T20:20:17Z

@TheJulianJES I am doing the custom quirk with the ZHA config option. It was matched to the signature of one of the devices I tested (the old V1 firmware), but no events were shown when I pressed the buttons.

Also, I appreciate the help. Let me know if there is anything I can do.

TheJulianJES · 2024-10-21T20:05:49Z

zhaquirks/smarthjemmet/quadzigsw.py

+                event_args = {VALUE: value}
+                self.listener_event(ZHA_SEND_EVENT, action, event_args)
+
+            super()._update_attribute(0, action)


If the changes work, please also try and see what these additional changes to this line do:

replacing 0 with MultistateInput.AttributeDefs.present_value.id in this line (sensor entity in HA?)

completely removing this line

I believe this was copied from other quirks where it doesn't actually work, as there is no attribute with id=0 on this cluster.

OptionHenrik · 2024-10-24T20:43:05Z

@OptionHenrik Yes, the quirk was working perfectly before @TheJulianJES's changes. Feel free to go look at the commit just before his changes, and grab the file from there to give it a go yourself.

Works like a charm … great work so far even though it’s only a custom quirk. I hope you guys get it integrated into the standard ZHA soon

TheJulianJES · 2024-10-24T20:45:35Z

Please also test the v2 quirk now. It was updated a few days ago.

radhus · 2024-10-27T20:29:47Z

@mortenmoulder can confirm your quirk works with just a model name change for MULTI-ZIG-SW as well! Great job.

I'll try to find time to test out @TheJulianJES changes later.

bangert · 2024-12-09T18:18:29Z

Any news here on the current version in this PR?

OptionHenrik · 2024-12-09T19:32:55Z

Any news here on the current version in this PR?

See my previous comment. Works like a charm with ZHA if you install it as a quirk.

TheJulianJES · 2024-12-09T19:45:33Z

It would also be ideal to know if the updated v2 quirk version works correctly now. Then, we'd be able to finally merge this (after I add some tests).

bangert · 2024-12-09T22:06:01Z

@mortenmoulder would this quirk also work for MULTI-ZIG-SW?

bangert · 2024-12-13T00:09:25Z

trying to get the latest version of this quirk to work with MULTI-ZIG-SW it doesnt work, and i get the following in the HA logs:

2024-12-13 01:04:27.370 ERROR (MainThread) [homeassistant.components.logbook.processor] Error with zha describe event for zha_event
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/logbook/processor.py", line 259, in _humanify
    data = describe_event(event_cache_get(row))
  File "/usr/src/homeassistant/homeassistant/components/zha/logbook.py", line 73, in async_describe_zha_event
    event_type = event_type.replace("_", " ").title()
                 ^^^^^^^^^^^^^^^^^^
AttributeError: 'bool' object has no attribute 'replace'

this is on HA 2024.12.2

bangert · 2025-01-27T21:51:05Z

okay - some more feedback. the current quirk (latest version on this PR) does not actually for me on the sister device MULTI-ZIG-SW, but it is recognised if i add two lines. The first is line 28

MODEL_ALT_ID = "MULTI-ZIG-SW"

and its usage on line 58/59

.applies_to(MANUFACTURER, MODEL_ALT_ID)

meaning it will show up as "MULTI-ZIG-SW by smarthjemmet.dk" and show Quirk: zigpy.quirks.v2.CustomDeviceV2 in zigbee info.

Nothing happens when pushing any of the buttons though.

bangert · 2025-02-01T05:29:46Z

with this patch the quadzigsw.py quirk works for me on a MULTI_ZIG_SW:

--- quadzigsw-orig.py   2025-02-01 06:26:31.211611131 +0100
+++ quadzigsw.py        2025-02-01 06:27:26.903118134 +0100
@@ -25,6 +25,7 @@
 
 MANUFACTURER = "smarthjemmet.dk"
 MODEL_ID = "QUAD-ZIG-SW"
+MODEL_ALT_ID = "MULTI-ZIG-SW"
 
 ACTION_TYPE = {
     0: COMMAND_RELEASE,
@@ -43,20 +44,19 @@
 
 
 class CustomMultistateInputCluster(CustomCluster, MultistateInput):
-    """Multistate input cluster."""
+    """CustomMultistate input cluster."""
 
     def _update_attribute(self, attrid, value):
         super()._update_attribute(attrid, value)
         if attrid == MultistateInput.AttributeDefs.present_value.id:
-            if action := ACTION_TYPE.get(value) is not None:
+            if (action := ACTION_TYPE.get(value)) is not None:
                 event_args = {VALUE: value}
                 self.listener_event(ZHA_SEND_EVENT, action, event_args)
 
-            super()._update_attribute(0, action)
-
 
 (
     QuirkBuilder(MANUFACTURER, MODEL_ID)
+    .applies_to(MANUFACTURER, MODEL_ALT_ID)
     .skip_configuration()
     .replaces(CR2032PowerConfigurationCluster)
     .removes(MultistateInput.cluster_id, cluster_type=ClusterType.Client, endpoint_id=2)

TheJulianJES · 2025-02-01T05:40:08Z

@bangert Thanks! The issue with the walrus operator was an oversight on my part. I'll merge these changes and add tests for this device soon, then we can finally merge this quirk.

bangert · 2025-02-05T16:30:53Z

@TheJulianJES i have re-tested the latest version on a MULTI-ZIG-SW v1 firmware: it still works.

@mortenmoulder @OptionHenrik could you test with QUAD-ZIG-SW and/or v2 firmware?

TheJulianJES

Thanks!

mortenmoulder and others added 7 commits October 6, 2024 21:33

Add support for QUAD-ZIG-SW from smarthjemmet.dk

b05cfbe

Apply pre-commit auto fixes

312e1d6

Fix docstrings

9bd1af7

Merge branch 'dev' of https://github.com/mortenmoulder/zha-device-han…

d4eb649

…dlers into dev

Apply pre-commit auto fixes

04baa50

Update file name and add tests

745b05d

Apply pre-commit auto fixes

1170d36

mortenmoulder added 2 commits October 6, 2024 22:22

Fix docstring

df836e2

Merge branch 'dev' of https://github.com/mortenmoulder/zha-device-han…

367ea10

…dlers into dev

TheJulianJES added the new quirk Adds support for a new device label Oct 10, 2024

TheJulianJES added 5 commits October 18, 2024 01:29

Remove empty __init__

65d32f6

Remove unused _current_state

e4fd04f

Use walrus operator for action

c303d4e

Refactor quirk CustomDevice classes

8619f67

Follows the style of other quirks where there's proper quirk defined in a `CustomDevice` class that's inherited from in other quirks that just change the `signature`, but keep the `replacement` and device triggers.

Replace with v2 quirk

d14e974

Remove test

a246d9c

The test here is no longer really needed for the v2 quirk. There's no real point in testing if all the correct clusters are included. Ideally, we would send messages from the device and see if they are converted to "ZHA events" as expected in a test.

TheJulianJES reviewed Oct 18, 2024

View reviewed changes

TheJulianJES added the help wanted Extra attention is needed label Oct 18, 2024

TheJulianJES reviewed Oct 21, 2024

View reviewed changes

TheJulianJES added waiting for reply Waiting for a reply (e.g. to test a custom quirk) and removed help wanted Extra attention is needed labels Oct 26, 2024

TheJulianJES mentioned this pull request Oct 26, 2024

[Device Support Request] smarthjemmet.dk QUAD-ZIG-SW #2846

Closed

TheJulianJES linked an issue Oct 26, 2024 that may be closed by this pull request

[Device Support Request] smarthjemmet.dk QUAD-ZIG-SW #2846

Closed

TheJulianJES added the needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). label Jan 27, 2025

TheJulianJES self-assigned this Feb 1, 2025

TheJulianJES added the needs review This PR should be reviewed soon, as it generally looks good. label Feb 1, 2025

TheJulianJES added 5 commits February 1, 2025 06:42

Merge branch 'dev' into mortenmoulder-dev

2667f9e

Fix walrus (thanks @bangert)

c4d03d0

Remove updating attribute 0 on press

00eb2bf

Also apply to MULTI-ZIG-SW model

1443e59

Add tests

b12b59c

TheJulianJES removed the waiting for reply Waiting for a reply (e.g. to test a custom quirk) label Feb 1, 2025

Combine if statements

453ea0f

TheJulianJES added the ready PR should be ready to merge label Feb 5, 2025

TheJulianJES approved these changes Feb 6, 2025

View reviewed changes

TheJulianJES merged commit 78687d2 into zigpy:dev Feb 6, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for QUAD-ZIG-SW from smarthjemmet.dk #3400

Add support for QUAD-ZIG-SW from smarthjemmet.dk #3400

mortenmoulder commented Oct 6, 2024 •

edited

Loading

codecov bot commented Oct 6, 2024 •

edited

Loading

mortenmoulder commented Oct 6, 2024

mortenmoulder commented Oct 17, 2024

TheJulianJES commented Oct 17, 2024

TheJulianJES commented Oct 18, 2024 •

edited

Loading

TheJulianJES Oct 18, 2024

mortenmoulder commented Oct 18, 2024

mortenmoulder commented Oct 18, 2024

mortenmoulder commented Oct 18, 2024 •

edited by TheJulianJES

Loading

TheJulianJES commented Oct 18, 2024 •

edited

Loading

mortenmoulder commented Oct 18, 2024

TheJulianJES commented Oct 19, 2024

mortenmoulder commented Oct 19, 2024 •

edited

Loading

TheJulianJES Oct 21, 2024

OptionHenrik commented Oct 24, 2024

TheJulianJES commented Oct 24, 2024 •

edited

Loading

radhus commented Oct 27, 2024

bangert commented Dec 9, 2024

OptionHenrik commented Dec 9, 2024

TheJulianJES commented Dec 9, 2024

bangert commented Dec 9, 2024

bangert commented Dec 13, 2024

bangert commented Jan 27, 2025 •

edited

Loading

bangert commented Feb 1, 2025

TheJulianJES commented Feb 1, 2025

bangert commented Feb 5, 2025

TheJulianJES left a comment

Add support for QUAD-ZIG-SW from smarthjemmet.dk #3400

Add support for QUAD-ZIG-SW from smarthjemmet.dk #3400

Conversation

mortenmoulder commented Oct 6, 2024 • edited Loading

Proposed change

Additional information

Checklist

codecov bot commented Oct 6, 2024 • edited Loading

Codecov Report

mortenmoulder commented Oct 6, 2024

mortenmoulder commented Oct 17, 2024

TheJulianJES commented Oct 17, 2024

TheJulianJES commented Oct 18, 2024 • edited Loading

TheJulianJES Oct 18, 2024

Choose a reason for hiding this comment

mortenmoulder commented Oct 18, 2024

mortenmoulder commented Oct 18, 2024

mortenmoulder commented Oct 18, 2024 • edited by TheJulianJES Loading

TheJulianJES commented Oct 18, 2024 • edited Loading

mortenmoulder commented Oct 18, 2024

TheJulianJES commented Oct 19, 2024

mortenmoulder commented Oct 19, 2024 • edited Loading

TheJulianJES Oct 21, 2024

Choose a reason for hiding this comment

OptionHenrik commented Oct 24, 2024

TheJulianJES commented Oct 24, 2024 • edited Loading

radhus commented Oct 27, 2024

bangert commented Dec 9, 2024

OptionHenrik commented Dec 9, 2024

TheJulianJES commented Dec 9, 2024

bangert commented Dec 9, 2024

bangert commented Dec 13, 2024

bangert commented Jan 27, 2025 • edited Loading

bangert commented Feb 1, 2025

TheJulianJES commented Feb 1, 2025

bangert commented Feb 5, 2025

TheJulianJES left a comment

Choose a reason for hiding this comment

mortenmoulder commented Oct 6, 2024 •

edited

Loading

codecov bot commented Oct 6, 2024 •

edited

Loading

TheJulianJES commented Oct 18, 2024 •

edited

Loading

mortenmoulder commented Oct 18, 2024 •

edited by TheJulianJES

Loading

TheJulianJES commented Oct 18, 2024 •

edited

Loading

mortenmoulder commented Oct 19, 2024 •

edited

Loading

TheJulianJES commented Oct 24, 2024 •

edited

Loading

bangert commented Jan 27, 2025 •

edited

Loading