Optimize ZHA ZCL attribute reporting configuration (#55796)

* Refactor ZCL attribute reporting configuration

Configure up to 3 attributes in a single request.

* Use constant for attribute reporting configuration

* Update tests

* Cleanup

* Remove irrelevant for this PR section

homeassistant/components/zha/core/channels/base.py

@@ -8,6 +8,7 @@ import logging
 from typing import Any
 
 import zigpy.exceptions
+from zigpy.zcl.foundation import Status
 
 from homeassistant.const import ATTR_COMMAND
 from homeassistant.core import callback
@@ -23,6 +24,7 @@ from ..const import (
     ATTR_UNIQUE_ID,
     ATTR_VALUE,
     CHANNEL_ZDO,
+    REPORT_CONFIG_ATTR_PER_REQ,
     SIGNAL_ATTR_UPDATED,
     ZHA_CHANNEL_MSG,
     ZHA_CHANNEL_MSG_BIND,
@@ -87,7 +89,7 @@ class ChannelStatus(Enum):
 class ZigbeeChannel(LogMixin):
     """Base channel for a Zigbee cluster."""
 
-    REPORT_CONFIG = ()
+    REPORT_CONFIG: tuple[dict[int | str, tuple[int, int, int | float]]] = ()
     BIND: bool = True
 
     def __init__(
@@ -101,9 +103,8 @@ class ZigbeeChannel(LogMixin):
         self._id = f"{ch_pool.id}:0x{cluster.cluster_id:04x}"
         unique_id = ch_pool.unique_id.replace("-", ":")
         self._unique_id = f"{unique_id}:0x{cluster.cluster_id:04x}"
-        self._report_config = self.REPORT_CONFIG
+        if not hasattr(self, "_value_attribute") and self.REPORT_CONFIG:
-        if not hasattr(self, "_value_attribute") and len(self._report_config) > 0:
+            attr = self.REPORT_CONFIG[0].get("attr")
-            attr = self._report_config[0].get("attr")
             if isinstance(attr, str):
                 self.value_attribute = self.cluster.attridx.get(attr)
             else:
@@ -195,42 +196,42 @@ class ZigbeeChannel(LogMixin):
         if self.cluster.cluster_id >= 0xFC00 and self._ch_pool.manufacturer_code:
             kwargs["manufacturer"] = self._ch_pool.manufacturer_code
 
-        for report in self._report_config:
+        for attr_report in self.REPORT_CONFIG:
-            attr = report["attr"]
+            attr, config = attr_report["attr"], attr_report["config"]
             attr_name = self.cluster.attributes.get(attr, [attr])[0]
-            min_report_int, max_report_int, reportable_change = report["config"]
             event_data[attr_name] = {
-                "min": min_report_int,
+                "min": config[0],
-                "max": max_report_int,
+                "max": config[1],
                 "id": attr,
                 "name": attr_name,
-                "change": reportable_change,
+                "change": config[2],
+                "success": False,
             }
 
+        to_configure = [*self.REPORT_CONFIG]
+        chunk, rest = (
+            to_configure[:REPORT_CONFIG_ATTR_PER_REQ],
+            to_configure[REPORT_CONFIG_ATTR_PER_REQ:],
+        )
+        while chunk:
+            reports = {rec["attr"]: rec["config"] for rec in chunk}
             try:
-                res = await self.cluster.configure_reporting(
+                res = await self.cluster.configure_reporting_multiple(reports, **kwargs)
-                    attr, min_report_int, max_report_int, reportable_change, **kwargs
+                self._configure_reporting_status(reports, res[0])
-                )
+                # if we get a response, then it's a success
-                self.debug(
+                for attr_stat in event_data.values():
-                    "reporting '%s' attr on '%s' cluster: %d/%d/%d: Result: '%s'",
+                    attr_stat["success"] = True
-                    attr_name,
-                    self.cluster.ep_attribute,
-                    min_report_int,
-                    max_report_int,
-                    reportable_change,
-                    res,
-                )
-                event_data[attr_name]["success"] = (
-                    res[0][0].status == 0 or res[0][0].status == 134
-                )
             except (zigpy.exceptions.ZigbeeException, asyncio.TimeoutError) as ex:
                 self.debug(
-                    "failed to set reporting for '%s' attr on '%s' cluster: %s",
+                    "failed to set reporting on '%s' cluster for: %s",
-                    attr_name,
                     self.cluster.ep_attribute,
                     str(ex),
                 )
-                event_data[attr_name]["success"] = False
+                break
+            chunk, rest = (
+                rest[:REPORT_CONFIG_ATTR_PER_REQ],
+                rest[REPORT_CONFIG_ATTR_PER_REQ:],
+            )
 
         async_dispatcher_send(
             self._ch_pool.hass,
@@ -245,6 +246,46 @@ class ZigbeeChannel(LogMixin):
             },
         )
 
+    def _configure_reporting_status(
+        self, attrs: dict[int | str, tuple], res: list | tuple
+    ) -> None:
+        """Parse configure reporting result."""
+        if not isinstance(res, list):
+            # assume default response
+            self.debug(
+                "attr reporting for '%s' on '%s': %s",
+                attrs,
+                self.name,
+                res,
+            )
+            return
+        if res[0].status == Status.SUCCESS and len(res) == 1:
+            self.debug(
+                "Successfully configured reporting for '%s' on '%s' cluster: %s",
+                attrs,
+                self.name,
+                res,
+            )
+            return
+
+        failed = [
+            self.cluster.attributes.get(r.attrid, [r.attrid])[0]
+            for r in res
+            if r.status != Status.SUCCESS
+        ]
+        attrs = {self.cluster.attributes.get(r, [r])[0] for r in attrs}
+        self.debug(
+            "Successfully configured reporting for '%s' on '%s' cluster",
+            attrs - set(failed),
+            self.name,
+        )
+        self.debug(
+            "Failed to configure reporting for '%s' on '%s' cluster: %s",
+            failed,
+            self.name,
+            res,
+        )
+
     async def async_configure(self) -> None:
         """Set cluster binding and attribute reporting."""
         if not self._ch_pool.skip_configuration:
@@ -267,7 +308,7 @@ class ZigbeeChannel(LogMixin):
             return
 
         self.debug("initializing channel: from_cache: %s", from_cache)
-        attributes = [cfg["attr"] for cfg in self._report_config]
+        attributes = [cfg["attr"] for cfg in self.REPORT_CONFIG]
         if attributes:
             await self.get_attributes(attributes, from_cache=from_cache)
 

homeassistant/components/zha/core/channels/hvac.py

@@ -6,7 +6,6 @@ https://home-assistant.io/integrations/zha/
 """
 from __future__ import annotations
 
-import asyncio
 from collections import namedtuple
 from typing import Any
 
@@ -85,6 +84,20 @@ class Pump(ZigbeeChannel):
 class ThermostatChannel(ZigbeeChannel):
     """Thermostat channel."""
 
+    REPORT_CONFIG = (
+        {"attr": "local_temp", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "occupied_cooling_setpoint", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "occupied_heating_setpoint", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "unoccupied_cooling_setpoint", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "unoccupied_heating_setpoint", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "running_mode", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "running_state", "config": REPORT_CONFIG_CLIMATE_DEMAND},
+        {"attr": "system_mode", "config": REPORT_CONFIG_CLIMATE},
+        {"attr": "occupancy", "config": REPORT_CONFIG_CLIMATE_DISCRETE},
+        {"attr": "pi_cooling_demand", "config": REPORT_CONFIG_CLIMATE_DEMAND},
+        {"attr": "pi_heating_demand", "config": REPORT_CONFIG_CLIMATE_DEMAND},
+    )
+
     def __init__(
         self, cluster: zha_typing.ZigpyClusterType, ch_pool: zha_typing.ChannelPoolType
     ) -> None:
@@ -132,19 +145,6 @@ class ThermostatChannel(ZigbeeChannel):
         self._system_mode = None
         self._unoccupied_cooling_setpoint = None
         self._unoccupied_heating_setpoint = None
-        self._report_config = [
-            {"attr": "local_temp", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "occupied_cooling_setpoint", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "occupied_heating_setpoint", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "unoccupied_cooling_setpoint", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "unoccupied_heating_setpoint", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "running_mode", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "running_state", "config": REPORT_CONFIG_CLIMATE_DEMAND},
-            {"attr": "system_mode", "config": REPORT_CONFIG_CLIMATE},
-            {"attr": "occupancy", "config": REPORT_CONFIG_CLIMATE_DISCRETE},
-            {"attr": "pi_cooling_demand", "config": REPORT_CONFIG_CLIMATE_DEMAND},
-            {"attr": "pi_heating_demand", "config": REPORT_CONFIG_CLIMATE_DEMAND},
-        ]
 
     @property
     def abs_max_cool_setpoint_limit(self) -> int:
@@ -285,71 +285,6 @@ class ThermostatChannel(ZigbeeChannel):
 
             chunk, attrs = attrs[:4], attrs[4:]
 
-    async def configure_reporting(self):
-        """Configure attribute reporting for a cluster.
-
-        This also swallows DeliveryError exceptions that are thrown when
-        devices are unreachable.
-        """
-        kwargs = {}
-        if self.cluster.cluster_id >= 0xFC00 and self._ch_pool.manufacturer_code:
-            kwargs["manufacturer"] = self._ch_pool.manufacturer_code
-
-        chunk, rest = self._report_config[:4], self._report_config[4:]
-        while chunk:
-            attrs = {record["attr"]: record["config"] for record in chunk}
-            try:
-                res = await self.cluster.configure_reporting_multiple(attrs, **kwargs)
-                self._configure_reporting_status(attrs, res[0])
-            except (ZigbeeException, asyncio.TimeoutError) as ex:
-                self.debug(
-                    "failed to set reporting on '%s' cluster for: %s",
-                    self.cluster.ep_attribute,
-                    str(ex),
-                )
-                break
-            chunk, rest = rest[:4], rest[4:]
-
-    def _configure_reporting_status(
-        self, attrs: dict[int | str, tuple], res: list | tuple
-    ) -> None:
-        """Parse configure reporting result."""
-        if not isinstance(res, list):
-            # assume default response
-            self.debug(
-                "attr reporting for '%s' on '%s': %s",
-                attrs,
-                self.name,
-                res,
-            )
-            return
-        if res[0].status == Status.SUCCESS and len(res) == 1:
-            self.debug(
-                "Successfully configured reporting for '%s' on '%s' cluster: %s",
-                attrs,
-                self.name,
-                res,
-            )
-            return
-
-        failed = [
-            self.cluster.attributes.get(r.attrid, [r.attrid])[0]
-            for r in res
-            if r.status != Status.SUCCESS
-        ]
-        attrs = {self.cluster.attributes.get(r, [r])[0] for r in attrs}
-        self.debug(
-            "Successfully configured reporting for '%s' on '%s' cluster",
-            attrs - set(failed),
-            self.name,
-        )
-        self.debug(
-            "Failed to configure reporting for '%s' on '%s' cluster: %s",
-            failed,
-            self.name,
-            res,
-        )
-
     @retryable_req(delays=(1, 1, 3))
     async def async_initialize_channel_specific(self, from_cache: bool) -> None:
         """Initialize channel."""

homeassistant/components/zha/core/const.py

@@ -287,6 +287,7 @@ class RadioType(enum.Enum):
         return self._desc
 
 
+REPORT_CONFIG_ATTR_PER_REQ = 3
 REPORT_CONFIG_MAX_INT = 900
 REPORT_CONFIG_MAX_INT_BATTERY_SAVE = 10800
 REPORT_CONFIG_MIN_INT = 30

tests/components/zha/common.py

@@ -1,5 +1,6 @@
 """Common test objects."""
 import asyncio
+import math
 import time
 from unittest.mock import AsyncMock, Mock
 
@@ -99,6 +100,9 @@ def patch_cluster(cluster):
             [zcl_f.ConfigureReportingResponseRecord(zcl_f.Status.SUCCESS, 0x00, 0xAABB)]
         ]
     )
+    cluster.configure_reporting_multiple = AsyncMock(
+        return_value=zcl_f.ConfigureReportingResponse.deserialize(b"\x00")[0]
+    )
     cluster.deserialize = Mock()
     cluster.handle_cluster_request = Mock()
     cluster.read_attributes = AsyncMock(wraps=cluster.read_attributes)
@@ -227,6 +231,7 @@ def reset_clusters(clusters):
     for cluster in clusters:
         cluster.bind.reset_mock()
         cluster.configure_reporting.reset_mock()
+        cluster.configure_reporting_multiple.reset_mock()
         cluster.write_attributes.reset_mock()
 
 
@@ -240,8 +245,21 @@ async def async_test_rejoin(hass, zigpy_device, clusters, report_counts, ep_id=1
     for cluster, reports in zip(clusters, report_counts):
         assert cluster.bind.call_count == 1
         assert cluster.bind.await_count == 1
-        assert cluster.configure_reporting.call_count == reports
+        if reports:
-        assert cluster.configure_reporting.await_count == reports
+            assert cluster.configure_reporting.call_count == 0
+            assert cluster.configure_reporting.await_count == 0
+            assert cluster.configure_reporting_multiple.call_count == math.ceil(
+                reports / zha_const.REPORT_CONFIG_ATTR_PER_REQ
+            )
+            assert cluster.configure_reporting_multiple.await_count == math.ceil(
+                reports / zha_const.REPORT_CONFIG_ATTR_PER_REQ
+            )
+        else:
+            # no reports at all
+            assert cluster.configure_reporting.call_count == reports
+            assert cluster.configure_reporting.await_count == reports
+            assert cluster.configure_reporting_multiple.call_count == reports
+            assert cluster.configure_reporting_multiple.await_count == reports
 
 
 async def async_wait_for_updates(hass):

tests/components/zha/test_channels.py

@@ -1,5 +1,6 @@
 """Test ZHA Core channels."""
 import asyncio
+import math
 from unittest import mock
 from unittest.mock import AsyncMock, patch
 
@@ -123,6 +124,23 @@ async def poll_control_device(zha_device_restored, zigpy_device_mock):
         (0x0020, 1, {}),
         (0x0021, 0, {}),
         (0x0101, 1, {"lock_state"}),
+        (
+            0x0201,
+            1,
+            {
+                "local_temp",
+                "occupied_cooling_setpoint",
+                "occupied_heating_setpoint",
+                "unoccupied_cooling_setpoint",
+                "unoccupied_heating_setpoint",
+                "running_mode",
+                "running_state",
+                "system_mode",
+                "occupancy",
+                "pi_cooling_demand",
+                "pi_heating_demand",
+            },
+        ),
         (0x0202, 1, {"fan_mode"}),
         (0x0300, 1, {"current_x", "current_y", "color_temperature"}),
         (0x0400, 1, {"measured_value"}),
@@ -156,8 +174,14 @@ async def test_in_channel_config(
     await channel.async_configure()
 
     assert cluster.bind.call_count == bind_count
-    assert cluster.configure_reporting.call_count == len(attrs)
+    assert cluster.configure_reporting.call_count == 0
-    reported_attrs = {attr[0][0] for attr in cluster.configure_reporting.call_args_list}
+    assert cluster.configure_reporting_multiple.call_count == math.ceil(len(attrs) / 3)
+    reported_attrs = {
+        a
+        for a in attrs
+        for attr in cluster.configure_reporting_multiple.call_args_list
+        for attrs in attr[0][0]
+    }
     assert set(attrs) == reported_attrs