Fix HomeKit Controller overwhelming resource-limited devices by batching characteristic polling (home-assistant#152209)

Author: bdraco

homeassistant/components/homekit_controller/connection.py

@@ -57,7 +57,10 @@
 
 RETRY_INTERVAL = 60  # seconds
 MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE = 3
-
+# HomeKit accessories have varying limits on how many characteristics
+# they can handle per request. Since we don't know each device's specific limit,
+# we batch requests to a conservative size to avoid overwhelming any device.
+MAX_CHARACTERISTICS_PER_REQUEST = 49
 
 BLE_AVAILABILITY_CHECK_INTERVAL = 1800  # seconds
 
@@ -326,16 +329,20 @@ async def async_setup(self) -> None:
         )
         entry.async_on_unload(self._async_cancel_subscription_timer)
 
+        if transport != Transport.BLE:
+            # Although async_populate_accessories_state fetched the accessory database,
+            # the /accessories endpoint may return cached values from the accessory's
+            # perspective. For example, Ecobee thermostats may report stale temperature
+            # values (like 100°C) in their /accessories response after restarting.
+            # We need to explicitly poll characteristics to get fresh sensor readings
+            # before processing the entity map and creating devices.
+            # Use poll_all=True since entities haven't registered their characteristics yet.
+            await self.async_update(poll_all=True)
+
         await self.async_process_entity_map()
 
         if transport != Transport.BLE:
-            # When Home Assistant starts, we restore the accessory map from storage
-            # which contains characteristic values from when HA was last running.
-            # These values are stale and may be incorrect (e.g., Ecobee thermostats
-            # report 100°C when restarting). We need to poll for fresh values before
-            # creating entities. Use poll_all=True since entities haven't registered
-            # their characteristics yet.
-            await self.async_update(poll_all=True)
+            # Start regular polling after entity map is processed
             self._async_start_polling()
 
         # If everything is up to date, we can create the entities
@@ -938,20 +945,26 @@ async def async_update(
         async with self._polling_lock:
             _LOGGER.debug("Starting HomeKit device update: %s", self.unique_id)
 
-            try:
-                new_values_dict = await self.get_characteristics(to_poll)
-            except AccessoryNotFoundError:
-                # Not only did the connection fail, but also the accessory is not
-                # visible on the network.
-                self.async_set_available_state(False)
-                return
-            except (AccessoryDisconnectedError, EncryptionError):
-                # Temporary connection failure. Device may still available but our
-                # connection was dropped or we are reconnecting
-                self._poll_failures += 1
-                if self._poll_failures >= MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE:
+            new_values_dict: dict[tuple[int, int], dict[str, Any]] = {}
+            to_poll_list = list(to_poll)
+
+            for i in range(0, len(to_poll_list), MAX_CHARACTERISTICS_PER_REQUEST):
+                batch = to_poll_list[i : i + MAX_CHARACTERISTICS_PER_REQUEST]
+                try:
+                    batch_values = await self.get_characteristics(batch)
+                    new_values_dict.update(batch_values)
+                except AccessoryNotFoundError:
+                    # Not only did the connection fail, but also the accessory is not
+                    # visible on the network.
                     self.async_set_available_state(False)
-                return
+                    return
+                except (AccessoryDisconnectedError, EncryptionError):
+                    # Temporary connection failure. Device may still available but our
+                    # connection was dropped or we are reconnecting
+                    self._poll_failures += 1
+                    if self._poll_failures >= MAX_POLL_FAILURES_TO_DECLARE_UNAVAILABLE:
+                        self.async_set_available_state(False)
+                    return
 
             self._poll_failures = 0
             self.process_new_events(new_values_dict)

tests/components/homekit_controller/test_connection.py

@@ -13,6 +13,9 @@
 import pytest
 
 from homeassistant.components.climate import ATTR_CURRENT_TEMPERATURE
+from homeassistant.components.homekit_controller.connection import (
+    MAX_CHARACTERISTICS_PER_REQUEST,
+)
 from homeassistant.components.homekit_controller.const import (
     DEBOUNCE_COOLDOWN,
     DOMAIN,
@@ -377,9 +380,15 @@ def _create_accessory(accessory: Accessory) -> Service:
         state = await helper.poll_and_get_state()
         assert state.state == STATE_OFF
         assert mock_get_characteristics.call_count == 2
-        # Verify everything is polled
-        assert mock_get_characteristics.call_args_list[0][0][0] == {(1, 10), (1, 11)}
-        assert mock_get_characteristics.call_args_list[1][0][0] == {(1, 10), (1, 11)}
+        # Verify everything is polled (convert to set for comparison since batching changes the type)
+        assert set(mock_get_characteristics.call_args_list[0][0][0]) == {
+            (1, 10),
+            (1, 11),
+        }
+        assert set(mock_get_characteristics.call_args_list[1][0][0]) == {
+            (1, 10),
+            (1, 11),
+        }
 
         # Test device goes offline
         helper.pairing.available = False
@@ -526,3 +535,84 @@ async def mock_get_characteristics(
     state = hass.states.get("climate.homew")
     assert state is not None
     assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 22.5
+
+
+async def test_characteristic_polling_batching(
+    hass: HomeAssistant, get_next_aid: Callable[[], int]
+) -> None:
+    """Test that characteristic polling is batched to MAX_CHARACTERISTICS_PER_REQUEST."""
+
+    # Create a large accessory with many characteristics (more than 49)
+    def create_large_accessory_with_many_chars(accessory: Accessory) -> None:
+        """Create an accessory with many characteristics to test batching."""
+        # Add multiple services with many characteristics each
+        for service_num in range(10):  # 10 services
+            service = accessory.add_service(
+                ServicesTypes.LIGHTBULB, name=f"Light {service_num}"
+            )
+            # Each lightbulb service gets several characteristics
+            service.add_char(CharacteristicsTypes.ON)
+            service.add_char(CharacteristicsTypes.BRIGHTNESS)
+            service.add_char(CharacteristicsTypes.HUE)
+            service.add_char(CharacteristicsTypes.SATURATION)
+            service.add_char(CharacteristicsTypes.COLOR_TEMPERATURE)
+            # Set initial values
+            for char in service.characteristics:
+                if char.type != CharacteristicsTypes.IDENTIFY:
+                    char.value = 0
+
+    helper = await setup_test_component(
+        hass, get_next_aid(), create_large_accessory_with_many_chars
+    )
+
+    # Track the get_characteristics calls
+    get_chars_calls = []
+    original_get_chars = helper.pairing.get_characteristics
+
+    async def mock_get_characteristics(chars):
+        """Mock get_characteristics to track batch sizes."""
+        get_chars_calls.append(list(chars))
+        return await original_get_chars(chars)
+
+    # Clear any calls from setup
+    get_chars_calls.clear()
+
+    # Patch get_characteristics to track calls
+    with mock.patch.object(
+        helper.pairing, "get_characteristics", side_effect=mock_get_characteristics
+    ):
+        # Trigger an update through time_changed which simulates regular polling
+        # time_changed expects seconds, not a datetime
+        await time_changed(hass, 300)  # 5 minutes in seconds
+        await hass.async_block_till_done()
+
+    # We created 10 lightbulb services with 5 characteristics each = 50 total
+    # Plus any base accessory characteristics that are pollable
+    # This should result in exactly 2 batches
+    assert len(get_chars_calls) == 2, (
+        f"Should have made exactly 2 batched calls, got {len(get_chars_calls)}"
+    )
+
+    # Check that no batch exceeded MAX_CHARACTERISTICS_PER_REQUEST
+    for i, batch in enumerate(get_chars_calls):
+        assert len(batch) <= MAX_CHARACTERISTICS_PER_REQUEST, (
+            f"Batch {i} size {len(batch)} exceeded maximum {MAX_CHARACTERISTICS_PER_REQUEST}"
+        )
+
+    # Verify the total number of characteristics polled
+    total_chars = sum(len(batch) for batch in get_chars_calls)
+    # Each lightbulb has: ON, BRIGHTNESS, HUE, SATURATION, COLOR_TEMPERATURE = 5
+    # 10 lightbulbs = 50 characteristics
+    assert total_chars == 50, (
+        f"Should have polled exactly 50 characteristics, got {total_chars}"
+    )
+
+    # The first batch should be full (49 characteristics)
+    assert len(get_chars_calls[0]) == 49, (
+        f"First batch should have exactly 49 characteristics, got {len(get_chars_calls[0])}"
+    )
+
+    # The second batch should have exactly 1 characteristic
+    assert len(get_chars_calls[1]) == 1, (
+        f"Second batch should have exactly 1 characteristic, got {len(get_chars_calls[1])}"
+    )