Fix not claiming cluster handler for some quirks v2 entities

tests/data/devices/bosch-rbsh-mms-zb-eu.json

@@ -836,7 +836,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -882,7 +882,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -928,7 +928,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -974,7 +974,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1022,7 +1022,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1069,7 +1069,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1494,7 +1494,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1586,7 +1586,7 @@
               },
               "id": "1:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:1:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1633,7 +1633,7 @@
               },
               "id": "2:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:2:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -1680,7 +1680,7 @@
               },
               "id": "3:0xfca0",
               "unique_id": "ab:cd:ef:12:75:e4:96:a5:3:0xfca0",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],

tests/data/devices/ikea-of-sweden-vallhorn-wireless-motion-sensor.json

@@ -463,7 +463,7 @@
               },
               "id": "1:0xfc81",
               "unique_id": "8c:6f:b9:ff:fe:26:3b:4a:1:0xfc81",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],
@@ -693,7 +693,7 @@
               },
               "id": "1:0xfc81",
               "unique_id": "8c:6f:b9:ff:fe:26:3b:4a:1:0xfc81",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],

tests/data/devices/third-reality-inc-3rsp02028bz.json

@@ -424,7 +424,7 @@
               },
               "id": "1:0xff03",
               "unique_id": "28:2c:02:bf:ff:eb:4f:2f:1:0xff03",
-              "status": "CREATED",
+              "status": "INITIALIZED",
               "value_attribute": null
             }
           ],

tests/test_button.py

@@ -12,6 +12,7 @@
     PROFILE_ID,
 )
 from zhaquirks.tuya.tuya_valve import ParksideTuyaValveManufCluster
+import zigpy
 from zigpy.exceptions import ZigbeeException
 from zigpy.profiles import zha
 from zigpy.quirks import CustomCluster, CustomDevice, DeviceRegistry
@@ -34,6 +35,7 @@
     update_attribute_cache,
 )
 from zha.application import Platform
+from zha.application.const import ZCL_INIT_ATTRS
 from zha.application.gateway import Gateway
 from zha.application.platforms import EntityCategory, PlatformEntity
 from zha.application.platforms.button import (
@@ -43,6 +45,7 @@
 )
 from zha.application.platforms.button.const import ButtonDeviceClass
 from zha.exceptions import ZHAException
+from zha.zigbee.cluster_handlers.manufacturerspecific import OppleRemoteClusterHandler
 from zha.zigbee.device import Device
 
 ZIGPY_DEVICE = {
@@ -354,3 +357,89 @@ async def test_quirks_write_attr_buttons_uid(zha_gateway: Gateway) -> None:
     assert entity_btn_2.translation_key == "btn_2"
     assert entity_btn_2._unique_id_suffix == "btn_2"
     assert entity_btn_2._attribute_value == 2
+
+
+class OppleCluster(CustomCluster, ManufacturerSpecificCluster):
+    """Aqara manufacturer specific cluster."""
+
+    cluster_id = 0xFCC0
+    ep_attribute = "opple_cluster"
+
+    class ServerCommandDefs(zcl_f.BaseCommandDefs):
+        """Server command definitions."""
+
+        self_test: Final = zcl_f.ZCLCommandDef(
+            id=0x00, schema={"identify_time": t.uint16_t}
+        )
+
+
+async def test_cluster_handler_quirks_unnecessary_claiming(
+    zha_gateway: Gateway,
+) -> None:
+    """Test quirks button doesn't claim cluster handlers unnecessarily."""
+
+    registry = DeviceRegistry()
+    (
+        QuirkBuilder(
+            "Fake_Manufacturer_sensor_2", "Fake_Model_sensor_2", registry=registry
+        )
+        .replaces(OppleCluster)
+        .command_button(
+            FakeManufacturerCluster.ServerCommandDefs.self_test.name,
+            OppleCluster.cluster_id,
+            command_args=(5,),
+            translation_key="self_test",
+            fallback_name="Self test",
+        )
+        .add_to_registry()
+    )
+
+    zigpy_device = create_mock_zigpy_device(
+        zha_gateway,
+        {
+            1: {
+                SIG_EP_INPUT: [
+                    general.Basic.cluster_id,
+                    OppleCluster.cluster_id,
+                ],
+                SIG_EP_OUTPUT: [],
+                SIG_EP_TYPE: zigpy.profiles.zha.DeviceType.OCCUPANCY_SENSOR,
+                SIG_EP_PROFILE: zigpy.profiles.zha.PROFILE_ID,
+            }
+        },
+        manufacturer="Fake_Manufacturer_sensor_2",
+        model="Fake_Model_sensor_2",
+    )
+    zigpy_device = registry.get_device(zigpy_device)
+
+    # Suppress normal endpoint probing, as this will claim the Opple cluster handler
+    # already due to it being in the "CLUSTER_HANDLER_ONLY_CLUSTERS" registry.
+    # We want to test the handler also gets claimed via quirks v2 attributes init.
+    with patch("zha.application.discovery.EndpointProbe.discover_entities"):
+        zha_device = await join_zigpy_device(zha_gateway, zigpy_device)
+    assert isinstance(zha_device.device, CustomDeviceV2)
+
+    # get cluster handler of OppleCluster
+    opple_ch = zha_device.endpoints[1].all_cluster_handlers["1:0xfcc0"]
+    assert isinstance(opple_ch, OppleRemoteClusterHandler)
+
+    # make sure the cluster handler was not claimed,
+    # as no reporting is configured and no attributes are to be read
+    assert opple_ch not in zha_device.endpoints[1].claimed_cluster_handlers.values()
+
+    # check that BIND is left at default of True, though ZHA will ignore it
+    assert opple_ch.BIND is True
+
+    # check ZCL_INIT_ATTRS is empty
+    assert opple_ch.ZCL_INIT_ATTRS == {}
+
+    # check that no ZCL_INIT_ATTRS instance variable was created
+    assert opple_ch.__dict__.get(ZCL_INIT_ATTRS) is None
+    assert opple_ch.ZCL_INIT_ATTRS is OppleRemoteClusterHandler.ZCL_INIT_ATTRS
+
+    # double check we didn't modify the class variable
+    assert OppleRemoteClusterHandler.ZCL_INIT_ATTRS == {}
+
+    # check if REPORT_CONFIG is empty, both instance and class variable
+    assert opple_ch.REPORT_CONFIG == ()
+    assert OppleRemoteClusterHandler.REPORT_CONFIG == ()

tests/test_sensor.py

@@ -1598,7 +1598,7 @@ async def test_state_class(
     assert "Quirks provided an invalid state class: energy" in caplog.text
 
 
-async def test_cluster_handler_quirks_attributes(zha_gateway: Gateway) -> None:
+async def test_cluster_handler_quirks_attribute_reporting(zha_gateway: Gateway) -> None:
     """Test quirks sensor setting up ZCL_INIT_ATTRS and REPORT_CONFIG correctly."""
 
     # Suppress normal endpoint probing, as this will claim the Opple cluster handler
@@ -1615,6 +1615,9 @@ async def test_cluster_handler_quirks_attributes(zha_gateway: Gateway) -> None:
     # make sure the cluster handler was claimed due to reporting config, so ZHA binds it
     assert opple_ch in zha_device.endpoints[1].claimed_cluster_handlers.values()
 
+    # check that BIND is not set to False, as reporting is configured
+    assert opple_ch.BIND is True
+
     # check ZCL_INIT_ATTRS contains sensor attributes that are not in REPORT_CONFIG
     assert opple_ch.ZCL_INIT_ATTRS == {
         "energy": True,
@@ -1645,6 +1648,76 @@ async def test_cluster_handler_quirks_attributes(zha_gateway: Gateway) -> None:
     assert OppleRemoteClusterHandler.REPORT_CONFIG == ()
 
 
+async def test_cluster_handler_quirks_attribute_reading(zha_gateway: Gateway) -> None:
+    """Test quirks sensor setting up ZCL_INIT_ATTRS, claiming cluster handler."""
+
+    registry = DeviceRegistry()
+    (
+        QuirkBuilder(
+            "Fake_Manufacturer_sensor_2", "Fake_Model_sensor_2", registry=registry
+        )
+        .replaces(OppleCluster)
+        .sensor(
+            "last_feeding_size",
+            OppleCluster.cluster_id,
+            translation_key="last_feeding_size",
+            fallback_name="Last feeding size",
+        )
+        .add_to_registry()
+    )
+
+    zigpy_device = create_mock_zigpy_device(
+        zha_gateway,
+        {
+            1: {
+                SIG_EP_INPUT: [
+                    general.Basic.cluster_id,
+                    OppleCluster.cluster_id,
+                ],
+                SIG_EP_OUTPUT: [],
+                SIG_EP_TYPE: zigpy.profiles.zha.DeviceType.OCCUPANCY_SENSOR,
+                SIG_EP_PROFILE: zigpy.profiles.zha.PROFILE_ID,
+            }
+        },
+        manufacturer="Fake_Manufacturer_sensor_2",
+        model="Fake_Model_sensor_2",
+    )
+    zigpy_device = registry.get_device(zigpy_device)
+
+    # Suppress normal endpoint probing, as this will claim the Opple cluster handler
+    # already due to it being in the "CLUSTER_HANDLER_ONLY_CLUSTERS" registry.
+    # We want to test the handler also gets claimed via quirks v2 attributes init.
+    with patch("zha.application.discovery.EndpointProbe.discover_entities"):
+        zha_device = await join_zigpy_device(zha_gateway, zigpy_device)
+    assert isinstance(zha_device.device, CustomDeviceV2)
+
+    # get cluster handler of OppleCluster
+    opple_ch = zha_device.endpoints[1].all_cluster_handlers["1:0xfcc0"]
+    assert isinstance(opple_ch, OppleRemoteClusterHandler)
+
+    # make sure the cluster handler was claimed due to attributes to be initialized
+    # otherwise, ZHA won't configure the cluster handler, so attributes are not read
+    assert opple_ch in zha_device.endpoints[1].claimed_cluster_handlers.values()
+
+    # check that BIND is set to False, as no reporting is configured
+    assert opple_ch.BIND is False
+
+    # check ZCL_INIT_ATTRS contains sensor attributes that are not in REPORT_CONFIG
+    assert opple_ch.ZCL_INIT_ATTRS == {
+        "last_feeding_size": True,
+    }
+    # check that ZCL_INIT_ATTRS is an instance variable and not a class variable now
+    assert opple_ch.ZCL_INIT_ATTRS is opple_ch.__dict__[ZCL_INIT_ATTRS]
+    assert opple_ch.ZCL_INIT_ATTRS is not OppleRemoteClusterHandler.ZCL_INIT_ATTRS
+
+    # double check we didn't modify the class variable
+    assert OppleRemoteClusterHandler.ZCL_INIT_ATTRS == {}
+
+    # check if REPORT_CONFIG is empty, both instance and class variable
+    assert opple_ch.REPORT_CONFIG == ()
+    assert OppleRemoteClusterHandler.REPORT_CONFIG == ()
+
+
 async def test_device_counter_sensors(zha_gateway: Gateway) -> None:
     """Test coordinator counter sensor."""
 

zha/application/discovery.py

@@ -232,6 +232,10 @@ def discover_quirks_v2_entities(self, device: Device) -> Iterator[PlatformEntity
 
             assert cluster_handler
 
+            # flags to determine if we need to claim/bind the cluster handler
+            attribute_initialization_found: bool = False
+            reporting_found: bool = False
+
             for entity_metadata in entity_metadata_list:
                 platform = Platform(entity_metadata.entity_platform.value)
                 metadata_type = type(entity_metadata)
@@ -254,6 +258,7 @@ def discover_quirks_v2_entities(self, device: Device) -> Iterator[PlatformEntity
 
                 # process the entity metadata for ZCL_INIT_ATTRS and REPORT_CONFIG
                 if attr_name := getattr(entity_metadata, "attribute_name", None):
+                    # TODO: ignore "attribute write buttons"? currently, we claim ch
                     # if the entity has a reporting config, add it to the cluster handler
                     if rep_conf := getattr(entity_metadata, "reporting_config", None):
                         # if attr is already in REPORT_CONFIG, remove it first
@@ -268,8 +273,8 @@ def discover_quirks_v2_entities(self, device: Device) -> Iterator[PlatformEntity
                         cluster_handler.REPORT_CONFIG += (
                             AttrReportConfig(attr=attr_name, config=astuple(rep_conf)),
                         )
-                        # claim the cluster handler, so ZHA configures and binds it
-                        endpoint.claim_cluster_handlers([cluster_handler])
+                        # mark cluster handler for claiming and binding later
+                        reporting_found = True
 
                     # not in REPORT_CONFIG, add to ZCL_INIT_ATTRS if it not already in
                     elif attr_name not in cluster_handler.ZCL_INIT_ATTRS:
@@ -283,6 +288,8 @@ def discover_quirks_v2_entities(self, device: Device) -> Iterator[PlatformEntity
                         cluster_handler.ZCL_INIT_ATTRS[attr_name] = (
                             entity_metadata.attribute_initialized_from_cache
                         )
+                        # mark cluster handler for claiming later, but not binding
+                        attribute_initialization_found = True
 
                 yield entity_class(
                     cluster_handlers=[cluster_handler],
@@ -299,6 +306,19 @@ def discover_quirks_v2_entities(self, device: Device) -> Iterator[PlatformEntity
                     [cluster_handler.name],
                 )
 
+            # if the cluster handler is unclaimed, claim it and set BIND accordingly,
+            # so ZHA configures the cluster handler: reporting + reads attributes
+            if (attribute_initialization_found or reporting_found) and (
+                cluster_handler not in endpoint.claimed_cluster_handlers.values()
+            ):
+                endpoint.claim_cluster_handlers([cluster_handler])
+                # BIND is True by default, so only set to False if no reporting found.
+                # We can safely do this, since quirks v2 entities are initialized last,
+                # so if the cluster handler wasn't claimed by EndpointProbe so far,
+                # only v2 entities need it.
+                if not reporting_found:
+                    cluster_handler.BIND = False
+
     @ignore_exceptions_during_iteration
     def discover_coordinator_device_entities(
         self, device: Device