From b3bd34a024e91fed133d6249852e5506c6643ca2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 22 Oct 2023 02:08:28 -1000 Subject: [PATCH] Avoid dispatching same state to passive bluetooth entities (#102430) --- .../bluetooth/passive_update_processor.py | 45 +++- .../test_passive_update_processor.py | 193 +++++++++++++++++- 2 files changed, 224 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/bluetooth/passive_update_processor.py b/homeassistant/components/bluetooth/passive_update_processor.py index b4ac15fc28c..8138587b9b5 100644 --- a/homeassistant/components/bluetooth/passive_update_processor.py +++ b/homeassistant/components/bluetooth/passive_update_processor.py @@ -21,6 +21,7 @@ from homeassistant.helpers.entity import Entity, EntityDescription from homeassistant.helpers.entity_platform import async_get_current_platform from homeassistant.helpers.event import async_track_time_interval from homeassistant.helpers.storage import Store +from homeassistant.helpers.typing import UNDEFINED from homeassistant.util.enum import try_parse_enum from .const import DOMAIN @@ -134,12 +135,33 @@ class PassiveBluetoothDataUpdate(Generic[_T]): default_factory=dict ) - def update(self, new_data: PassiveBluetoothDataUpdate[_T]) -> None: - """Update the data.""" - self.devices.update(new_data.devices) - self.entity_descriptions.update(new_data.entity_descriptions) - self.entity_data.update(new_data.entity_data) - self.entity_names.update(new_data.entity_names) + def update( + self, new_data: PassiveBluetoothDataUpdate[_T] + ) -> set[PassiveBluetoothEntityKey] | None: + """Update the data and returned changed PassiveBluetoothEntityKey or None on device change. + + The changed PassiveBluetoothEntityKey can be used to filter + which listeners are called. + """ + device_change = False + changed_entity_keys: set[PassiveBluetoothEntityKey] = set() + for key, device_info in new_data.devices.items(): + if device_change or self.devices.get(key, UNDEFINED) != device_info: + device_change = True + self.devices[key] = device_info + for incoming, current in ( + (new_data.entity_descriptions, self.entity_descriptions), + (new_data.entity_names, self.entity_names), + (new_data.entity_data, self.entity_data), + ): + # mypy can't seem to work this out + for key, data in incoming.items(): # type: ignore[attr-defined] + if current.get(key, UNDEFINED) != data: # type: ignore[attr-defined] + changed_entity_keys.add(key) # type: ignore[arg-type] + current[key] = data # type: ignore[index] + # If the device changed we don't need to return the changed + # entity keys as all entities will be updated + return None if device_change else changed_entity_keys def async_get_restore_data(self) -> RestoredPassiveBluetoothDataUpdate: """Serialize restore data to storage.""" @@ -520,6 +542,7 @@ class PassiveBluetoothDataProcessor(Generic[_T]): self, data: PassiveBluetoothDataUpdate[_T] | None, was_available: bool | None = None, + changed_entity_keys: set[PassiveBluetoothEntityKey] | None = None, ) -> None: """Update all registered listeners.""" if was_available is None: @@ -542,6 +565,12 @@ class PassiveBluetoothDataProcessor(Generic[_T]): # if the key is in the data entity_key_listeners = self._entity_key_listeners for entity_key in data.entity_data: + if ( + was_available + and changed_entity_keys is not None + and entity_key not in changed_entity_keys + ): + continue if maybe_listener := entity_key_listeners.get(entity_key): for update_callback in maybe_listener: update_callback(data) @@ -573,8 +602,8 @@ class PassiveBluetoothDataProcessor(Generic[_T]): "Processing %s data recovered", self.coordinator.name ) - self.data.update(new_data) - self.async_update_listeners(new_data, was_available) + changed_entity_keys = self.data.update(new_data) + self.async_update_listeners(new_data, was_available, changed_entity_keys) class PassiveBluetoothProcessorEntity(Entity, Generic[_PassiveBluetoothDataProcessorT]): diff --git a/tests/components/bluetooth/test_passive_update_processor.py b/tests/components/bluetooth/test_passive_update_processor.py index 5baff65f29a..9e3f954a0c5 100644 --- a/tests/components/bluetooth/test_passive_update_processor.py +++ b/tests/components/bluetooth/test_passive_update_processor.py @@ -112,6 +112,65 @@ GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE = PassiveBluetoothDataUpdate( }, ) +GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE_WITH_TEMP_CHANGE = PassiveBluetoothDataUpdate( + devices={ + None: DeviceInfo( + name="Test Device", model="Test Model", manufacturer="Test Manufacturer" + ), + }, + entity_data={ + PassiveBluetoothEntityKey("temperature", None): 15.5, + PassiveBluetoothEntityKey("pressure", None): 1234, + }, + entity_names={ + PassiveBluetoothEntityKey("temperature", None): "Temperature", + PassiveBluetoothEntityKey("pressure", None): "Pressure", + }, + entity_descriptions={ + PassiveBluetoothEntityKey("temperature", None): SensorEntityDescription( + key="temperature", + native_unit_of_measurement=UnitOfTemperature.CELSIUS, + device_class=SensorDeviceClass.TEMPERATURE, + ), + PassiveBluetoothEntityKey("pressure", None): SensorEntityDescription( + key="pressure", + native_unit_of_measurement="hPa", + device_class=SensorDeviceClass.PRESSURE, + ), + }, +) + + +GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE_WITH_DEVICE_NAME_AND_TEMP_CHANGE = ( + PassiveBluetoothDataUpdate( + devices={ + None: DeviceInfo( + name="Changed", model="Test Model", manufacturer="Test Manufacturer" + ), + }, + entity_data={ + PassiveBluetoothEntityKey("temperature", None): 15.5, + PassiveBluetoothEntityKey("pressure", None): 1234, + }, + entity_names={ + PassiveBluetoothEntityKey("temperature", None): "Temperature", + PassiveBluetoothEntityKey("pressure", None): "Pressure", + }, + entity_descriptions={ + PassiveBluetoothEntityKey("temperature", None): SensorEntityDescription( + key="temperature", + native_unit_of_measurement=UnitOfTemperature.CELSIUS, + device_class=SensorDeviceClass.TEMPERATURE, + ), + PassiveBluetoothEntityKey("pressure", None): SensorEntityDescription( + key="pressure", + native_unit_of_measurement="hPa", + device_class=SensorDeviceClass.PRESSURE, + ), + }, + ) +) + async def test_basic_usage( hass: HomeAssistant, @@ -189,9 +248,9 @@ async def test_basic_usage( inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) - # Each listener should receive the same data - # since both match - assert len(entity_key_events) == 2 + # Only the all listener should receive the new data + # since temperature is not in the new data + assert len(entity_key_events) == 1 assert len(all_events) == 2 # On the second, the entities should already be created @@ -206,8 +265,130 @@ async def test_basic_usage( # Each listener should not trigger any more now # that they were cancelled + assert len(entity_key_events) == 1 + assert len(all_events) == 2 + assert len(mock_entity.mock_calls) == 2 + assert coordinator.available is True + + unregister_processor() + cancel_coordinator() + + +async def test_entity_key_is_dispatched_on_entity_key_change( + hass: HomeAssistant, + mock_bleak_scanner_start: MagicMock, + mock_bluetooth_adapters: None, +) -> None: + """Test entity key listeners are only dispatched on change.""" + await async_setup_component(hass, DOMAIN, {DOMAIN: {}}) + update_count = 0 + + @callback + def _mock_update_method( + service_info: BluetoothServiceInfo, + ) -> dict[str, str]: + return {"test": "data"} + + @callback + def _async_generate_mock_data( + data: dict[str, str], + ) -> PassiveBluetoothDataUpdate: + """Generate mock data.""" + assert data == {"test": "data"} + nonlocal update_count + update_count += 1 + if update_count > 2: + return ( + GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE_WITH_DEVICE_NAME_AND_TEMP_CHANGE + ) + if update_count > 1: + return GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE_WITH_TEMP_CHANGE + return GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE + + coordinator = PassiveBluetoothProcessorCoordinator( + hass, + _LOGGER, + "aa:bb:cc:dd:ee:ff", + BluetoothScanningMode.ACTIVE, + _mock_update_method, + ) + assert coordinator.available is False # no data yet + + processor = PassiveBluetoothDataProcessor(_async_generate_mock_data) + + unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() + + entity_key = PassiveBluetoothEntityKey("temperature", None) + entity_key_events = [] + all_events = [] + mock_entity = MagicMock() + mock_add_entities = MagicMock() + + def _async_entity_key_listener(data: PassiveBluetoothDataUpdate | None) -> None: + """Mock entity key listener.""" + entity_key_events.append(data) + + cancel_async_add_entity_key_listener = processor.async_add_entity_key_listener( + _async_entity_key_listener, + entity_key, + ) + + def _all_listener(data: PassiveBluetoothDataUpdate | None) -> None: + """Mock an all listener.""" + all_events.append(data) + + cancel_listener = processor.async_add_listener( + _all_listener, + ) + + cancel_async_add_entities_listener = processor.async_add_entities_listener( + mock_entity, + mock_add_entities, + ) + + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + + # Each listener should receive the same data + # since both match + assert len(entity_key_events) == 1 + assert len(all_events) == 1 + + # There should be 4 calls to create entities + assert len(mock_entity.mock_calls) == 2 + + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) + + # Both listeners should receive the new data + # since temperature IS in the new data assert len(entity_key_events) == 2 assert len(all_events) == 2 + + # On the second, the entities should already be created + # so the mock should not be called again + assert len(mock_entity.mock_calls) == 2 + + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + + # All listeners should receive the data since + # the device name changed + assert len(entity_key_events) == 3 + assert len(all_events) == 3 + + # On the second, the entities should already be created + # so the mock should not be called again + assert len(mock_entity.mock_calls) == 2 + + cancel_async_add_entity_key_listener() + cancel_listener() + cancel_async_add_entities_listener() + + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) + + # Each listener should not trigger any more now + # that they were cancelled + assert len(entity_key_events) == 3 + assert len(all_events) == 3 assert len(mock_entity.mock_calls) == 2 assert coordinator.available is True @@ -897,9 +1078,9 @@ async def test_integration_with_entity( # Forth call with both primary and remote sensor entities does not add them again assert len(mock_add_entities.mock_calls) == 2 - # should not have triggered the entity key listener since there - # there is an update with the entity key - assert len(entity_key_events) == 3 + # should not have triggered the entity key listener humidity + # is not in the update + assert len(entity_key_events) == 2 entities = [ *mock_add_entities.mock_calls[0][1][0],