From 63273a307a04a0bb666de6ad81f49fa45c9c4c26 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 4 Sep 2023 19:42:05 -0500 Subject: [PATCH] Fix Bluetooth passive update processor dispatching updates to unchanged entities (#99527) * Fix passive update processor dispatching updates to unchanged entities * adjust tests * coverage * fix * Update homeassistant/components/bluetooth/update_coordinator.py --- .../bluetooth/passive_update_coordinator.py | 1 + .../bluetooth/passive_update_processor.py | 35 +++++++++++++++---- .../bluetooth/update_coordinator.py | 14 ++------ .../bluetooth/test_active_update_processor.py | 10 +++--- .../test_passive_update_processor.py | 28 +++++++++++++++ 5 files changed, 65 insertions(+), 23 deletions(-) diff --git a/homeassistant/components/bluetooth/passive_update_coordinator.py b/homeassistant/components/bluetooth/passive_update_coordinator.py index 6f1749aeef2..fcf6fcdf255 100644 --- a/homeassistant/components/bluetooth/passive_update_coordinator.py +++ b/homeassistant/components/bluetooth/passive_update_coordinator.py @@ -85,6 +85,7 @@ class PassiveBluetoothDataUpdateCoordinator( change: BluetoothChange, ) -> None: """Handle a Bluetooth event.""" + self._available = True self.async_update_listeners() diff --git a/homeassistant/components/bluetooth/passive_update_processor.py b/homeassistant/components/bluetooth/passive_update_processor.py index 6d0621fa4f6..7294d55f912 100644 --- a/homeassistant/components/bluetooth/passive_update_processor.py +++ b/homeassistant/components/bluetooth/passive_update_processor.py @@ -341,6 +341,8 @@ class PassiveBluetoothProcessorCoordinator( change: BluetoothChange, ) -> None: """Handle a Bluetooth event.""" + was_available = self._available + self._available = True if self.hass.is_stopping: return @@ -358,7 +360,7 @@ class PassiveBluetoothProcessorCoordinator( self.logger.info("Coordinator %s recovered", self.name) for processor in self._processors: - processor.async_handle_update(update) + processor.async_handle_update(update, was_available) _PassiveBluetoothDataProcessorT = TypeVar( @@ -515,20 +517,39 @@ class PassiveBluetoothDataProcessor(Generic[_T]): @callback def async_update_listeners( - self, data: PassiveBluetoothDataUpdate[_T] | None + self, + data: PassiveBluetoothDataUpdate[_T] | None, + was_available: bool | None = None, ) -> None: """Update all registered listeners.""" + if was_available is None: + was_available = self.coordinator.available + # Dispatch to listeners without a filter key for update_callback in self._listeners: update_callback(data) + if not was_available or data is None: + # When data is None, or was_available is False, + # dispatch to all listeners as it means the device + # is flipping between available and unavailable + for listeners in self._entity_key_listeners.values(): + for update_callback in listeners: + update_callback(data) + return + # Dispatch to listeners with a filter key - for listeners in self._entity_key_listeners.values(): - for update_callback in listeners: - update_callback(data) + # if the key is in the data + entity_key_listeners = self._entity_key_listeners + for entity_key in data.entity_data: + if maybe_listener := entity_key_listeners.get(entity_key): + for update_callback in maybe_listener: + update_callback(data) @callback - def async_handle_update(self, update: _T) -> None: + def async_handle_update( + self, update: _T, was_available: bool | None = None + ) -> None: """Handle a Bluetooth event.""" try: new_data = self.update_method(update) @@ -553,7 +574,7 @@ class PassiveBluetoothDataProcessor(Generic[_T]): ) self.data.update(new_data) - self.async_update_listeners(new_data) + self.async_update_listeners(new_data, was_available) class PassiveBluetoothProcessorEntity(Entity, Generic[_PassiveBluetoothDataProcessorT]): diff --git a/homeassistant/components/bluetooth/update_coordinator.py b/homeassistant/components/bluetooth/update_coordinator.py index 9c38bf2f520..12bff3be645 100644 --- a/homeassistant/components/bluetooth/update_coordinator.py +++ b/homeassistant/components/bluetooth/update_coordinator.py @@ -39,6 +39,8 @@ class BasePassiveBluetoothCoordinator(ABC): self.mode = mode self._last_unavailable_time = 0.0 self._last_name = address + # Subclasses are responsible for setting _available to True + # when the abstractmethod _async_handle_bluetooth_event is called. self._available = async_address_present(hass, address, connectable) @callback @@ -88,23 +90,13 @@ class BasePassiveBluetoothCoordinator(ABC): """Return if the device is available.""" return self._available - @callback - def _async_handle_bluetooth_event_internal( - self, - service_info: BluetoothServiceInfoBleak, - change: BluetoothChange, - ) -> None: - """Handle a bluetooth event.""" - self._available = True - self._async_handle_bluetooth_event(service_info, change) - @callback def _async_start(self) -> None: """Start the callbacks.""" self._on_stop.append( async_register_callback( self.hass, - self._async_handle_bluetooth_event_internal, + self._async_handle_bluetooth_event, BluetoothCallbackMatcher( address=self.address, connectable=self.connectable ), diff --git a/tests/components/bluetooth/test_active_update_processor.py b/tests/components/bluetooth/test_active_update_processor.py index 83ad809016a..fba86223a2d 100644 --- a/tests/components/bluetooth/test_active_update_processor.py +++ b/tests/components/bluetooth/test_active_update_processor.py @@ -91,7 +91,7 @@ async def test_basic_usage( # The first time, it was passed the data from parsing the advertisement # The second time, it was passed the data from polling assert len(async_handle_update.mock_calls) == 2 - assert async_handle_update.mock_calls[0] == call({"testdata": 0}) + assert async_handle_update.mock_calls[0] == call({"testdata": 0}, False) assert async_handle_update.mock_calls[1] == call({"testdata": 1}) cancel() @@ -148,7 +148,7 @@ async def test_poll_can_be_skipped( inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) await hass.async_block_till_done() - assert async_handle_update.mock_calls[-1] == call({"testdata": None}) + assert async_handle_update.mock_calls[-1] == call({"testdata": None}, True) flag = True @@ -208,7 +208,7 @@ async def test_bleak_error_and_recover( # First poll fails inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) await hass.async_block_till_done() - assert async_handle_update.mock_calls[-1] == call({"testdata": None}) + assert async_handle_update.mock_calls[-1] == call({"testdata": None}, False) assert ( "aa:bb:cc:dd:ee:ff: Bluetooth error whilst polling: Connection was aborted" @@ -272,7 +272,7 @@ async def test_poll_failure_and_recover( # First poll fails inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) await hass.async_block_till_done() - assert async_handle_update.mock_calls[-1] == call({"testdata": None}) + assert async_handle_update.mock_calls[-1] == call({"testdata": None}, False) # Second poll works flag = False @@ -433,7 +433,7 @@ async def test_no_polling_after_stop_event( # The first time, it was passed the data from parsing the advertisement # The second time, it was passed the data from polling assert len(async_handle_update.mock_calls) == 2 - assert async_handle_update.mock_calls[0] == call({"testdata": 0}) + assert async_handle_update.mock_calls[0] == call({"testdata": 0}, False) assert async_handle_update.mock_calls[1] == call({"testdata": 1}) hass.state = CoreState.stopping diff --git a/tests/components/bluetooth/test_passive_update_processor.py b/tests/components/bluetooth/test_passive_update_processor.py index c96fbfbfc99..5baff65f29a 100644 --- a/tests/components/bluetooth/test_passive_update_processor.py +++ b/tests/components/bluetooth/test_passive_update_processor.py @@ -858,22 +858,49 @@ async def test_integration_with_entity( mock_add_entities, ) + entity_key_events = [] + + 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, + PassiveBluetoothEntityKey(key="humidity", device_id="primary"), + ) + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) # First call with just the remote sensor entities results in them being added assert len(mock_add_entities.mock_calls) == 1 + # should have triggered the entity key listener since the + # the device is becoming available + assert len(entity_key_events) == 1 + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) # Second call with just the remote sensor entities does not add them again assert len(mock_add_entities.mock_calls) == 1 + # should not have triggered the entity key listener since there + # there is no update with the entity key + assert len(entity_key_events) == 1 + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) # Third call with primary and remote sensor entities adds the primary sensor entities 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) == 2 + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) # 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 + entities = [ *mock_add_entities.mock_calls[0][1][0], *mock_add_entities.mock_calls[1][1][0], @@ -892,6 +919,7 @@ async def test_integration_with_entity( assert entity_one.entity_key == PassiveBluetoothEntityKey( key="temperature", device_id="remote" ) + cancel_async_add_entity_key_listener() cancel_coordinator()