Fix configuring attribute reporting by TheJulianJES · Pull Request #648 · zigpy/zha

TheJulianJES · 2026-02-07T19:41:52Z

Proposed change

This fixes configuration of attribute reporting after the recent zigpy manufacturer code changes.

configure_reporting_multiple is now called with ZCLAttributeDef and ReportingConfig, instead of the attribute name (str) and a tuple containing min/max/reportable change int values.
_configure_reporting_status was updated to get the attribute name from ZCLAttributeDef.
The cluster_id >= 0xFC00 check for using the manufacturer code is removed, as zigpy now does this better and automatically, grouping respective attributes properly.
The tests were updated to expect the new configure_reporting_multiple calls.
The condition res[0].status == Status.SUCCESS and len(res) == 1 was updated to len(res) == 1 and res[0].status == Status.SUCCESS. This should now properly work with empty lists.
- EDIT: Zigpy should never send us an empty list though. I think it's still fine to not catastrophically break in the case zigpy sends something unexpected though, as that's just for the reporting status. We'll just mark the reports as failed then.
A test was added for configure_reporting_status, which sends reports on attribute reports used by HA frontend.

This change was tested with an IKEA motion sensor, where attribute reporting is now correctly configured for the PowerConfiguration cluster again. This is also represented in the HA frontend.

Additional information

Related PRs:

codecov · 2026-02-07T19:45:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.41%. Comparing base (7754950) to head (182a8bc).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #648      +/-   ##
==========================================
+ Coverage   97.29%   97.41%   +0.12%     
==========================================
  Files          62       62              
  Lines       10712    10719       +7     
==========================================
+ Hits        10422    10442      +20     
+ Misses        290      277      -13

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zha/zigbee/cluster_handlers/__init__.py

TheJulianJES · 2026-02-10T01:38:04Z

We can add a test for configure_reporting_status in this PR or another PR. For example: TheJulianJES:tjj/fix_attr_reporting...TheJulianJES:tjj/fix_attr_reporting_status_tests

TheJulianJES · 2026-02-11T20:01:16Z

@puddly Want me to just add the above test to this PR or make another one? (test for reporting status used by frontend)

puddly · 2026-02-11T20:03:30Z

Let's roll it into this one

TheJulianJES · 2026-02-11T20:11:59Z

Do note the updated tests in this PR still would not really catch an issue where the method signature of zigpy's cluster.configure_reporting_multiple changes. I guess we'd have to properly test the configure_reporting_multiple method in ZHA then and check the final zigpy requests..

I'd keep this PR as is now.

Copilot

Pull request overview

Updates ZHA cluster handler attribute reporting configuration to match recent zigpy manufacturer-code/attribute-definition API changes, and adjusts tests accordingly.

Changes:

Switch configure_reporting_multiple calls to pass {ZCLAttributeDef: ReportingConfig} instead of {str: (min,max,change)}.
Remove manual manufacturer-code selection logic for manufacturer-specific clusters and rely on zigpy’s handling.
Update reporting-status parsing and test expectations, adding coverage for reporting-status emission.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`zha/zigbee/cluster_handlers/__init__.py`	Updates reporting configuration payload format, removes manufacturer kwarg logic, and adjusts reporting-status parsing.
`tests/test_device.py`	Updates join/bind/reporting test assertions to expect `ZCLAttributeDef` + `ReportingConfig` mappings.
`tests/test_cluster_handlers.py`	Updates reporting-related test assertions and adds a parametrized test for reporting-status parsing/emission.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_cluster_handlers.py

zha/zigbee/cluster_handlers/__init__.py

TheJulianJES · 2026-02-11T20:43:38Z

zha/zigbee/cluster_handlers/__init__.py

        """Parse configure reporting result."""
-        if isinstance(res, (Exception, ConfigureReportingResponseRecord)):
+        attr_names = {attr_def.name for attr_def in attrs}
+        if not res or isinstance(res, (Exception, ConfigureReportingResponseRecord)):


We should not be able to hit the isinstance(res, (Exception, ConfigureReportingResponseRecord)) check ever now? Like, returning an exception or just a single ConfigureReportingResponseRecord? Should I just remove this?

If so, should I at least keep the guard against zigpy returning an empty list? Even though that should not happen after zigpy/zigpy#1764 anymore. We'd just mark the attributes as failed to configure if zigpy (or some test mocks elsewhere) somehow return an empty list. Or should we just ignore that and fail however that would fail? (causing SUCCESS)

I've removed it with 182a8bc (this PR). Having zigpy return an exception or a single status report can't happen at all from what I see. It's always a list at least.

Yeah, returning exception objects (instead of raising them) is a legacy artifact and something that we've slowly been trying to get rid of. There are more instances within the ZHA library that still account for this possibility. I believe we do still have a few asyncio.gather(..., return_exceptions=True) invocations that were the cause of this pattern.

…eReportingResponseRecord`

TheJulianJES · 2026-02-11T20:52:32Z

zha/zigbee/cluster_handlers/__init__.py

+        if not res:
            self.debug(
                "attr reporting for '%s' on '%s': %s",
-                attrs,
+                attr_names,
                self.name,
                res,
            )
-            for attr in attrs:
-                event_data[attr]["status"] = Status.FAILURE.name
+            for attr_name in attr_names:
+                event_data[attr_name]["status"] = Status.FAILURE.name
            return


Technically, zigpy should also never return an empty list. Do we want to keep this failure case in case zigpy breaks again, so we report all attributes as failed to configure then?

If we remove this, they'd report as being successfully configured otherwise, which also isn't great. But this should never happen (again), soo..

I think we can keep it in. In the near future I'd like to persist the attribute configuration state in the database so this code likely will be reworked anyways.

puddly · 2026-02-13T00:24:36Z

I've tested this PR as well (along with all of the other zigpy + ZHA changes) and it works as expected.

jclsn · 2026-02-14T12:31:34Z

The fixes haven't landed in 26.2.2, have they?

puddly · 2026-02-14T13:25:02Z

They have.

jclsn · 2026-02-14T13:36:21Z

Ah found it home-assistant/core#162894

jclsn · 2026-02-14T16:05:08Z

Yeah works again. Firmware versions are still unknown, but I guess this is related to something else?

TheJulianJES added 3 commits February 7, 2026 20:31

Remove handling for manufacturer_code, now done by zigpy

b954341

Pass ZCLAttributeDef and ReportingConfig

3da1161

Update tests to expect new configure_reporting_multiple calls

1e96ec6

Swap conditions to be useful

b2c551e

TheJulianJES commented Feb 7, 2026

View reviewed changes

zha/zigbee/cluster_handlers/__init__.py Show resolved Hide resolved

TheJulianJES mentioned this pull request Feb 7, 2026

Fix configure_reporting_multiple not returning responses zigpy/zigpy#1764

Merged

TheJulianJES added 5 commits February 10, 2026 02:36

Test _configure_reporting_status

def5a7a

Import Color cluster

6d3269c

Swap to custom TestClusterHandler for Color cluster

c697339

Add ids with nicer names

295872b

Assert entire calls

ac89d9b

TheJulianJES marked this pull request as ready for review February 11, 2026 20:08

TheJulianJES requested a review from Copilot February 11, 2026 20:10

Copilot started reviewing on behalf of TheJulianJES February 11, 2026 20:11 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

tests/test_cluster_handlers.py Outdated Show resolved Hide resolved

zha/zigbee/cluster_handlers/__init__.py Show resolved Hide resolved

zha/zigbee/cluster_handlers/__init__.py Show resolved Hide resolved

TheJulianJES added 2 commits February 11, 2026 21:34

Guard against zigpy (or mocks) returning an empty list as response

5e04a5b

Fix res typing

fa797c3

TheJulianJES commented Feb 11, 2026

View reviewed changes

Remove check for zigpy returning an Exception or a single `Configur…

182a8bc

…eReportingResponseRecord`

TheJulianJES commented Feb 11, 2026

View reviewed changes

This was referenced Feb 11, 2026

IKEA Inspelning : Metering Not polling/Slow polling home-assistant/core#162642

Closed

Philips Hue Motion Sensor (SML001) stopped working with ZHA home-assistant/core#162863

Closed

puddly merged commit b5e3a92 into zigpy:dev Feb 13, 2026
9 checks passed

Conversation

TheJulianJES commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Additional information

Uh oh!

codecov bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

TheJulianJES commented Feb 10, 2026

Uh oh!

TheJulianJES commented Feb 11, 2026

Uh oh!

puddly commented Feb 11, 2026

Uh oh!

TheJulianJES commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheJulianJES Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

TheJulianJES Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

puddly Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

TheJulianJES Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

puddly Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

puddly commented Feb 13, 2026

Uh oh!

Uh oh!

jclsn commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

puddly commented Feb 14, 2026

Uh oh!

jclsn commented Feb 14, 2026

Uh oh!

jclsn commented Feb 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TheJulianJES commented Feb 7, 2026 •

edited

Loading

codecov bot commented Feb 7, 2026 •

edited

Loading

jclsn commented Feb 14, 2026 •

edited

Loading