From 1e85ddabfd0e59a0106ea8d50f1e6ee5d3f84bb3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 26 Jul 2022 09:29:23 -1000 Subject: [PATCH] Fix startup race in BLE integrations (#75780) --- .../bluetooth/passive_update_coordinator.py | 11 ----------- .../bluetooth/passive_update_processor.py | 11 ----------- .../components/bluetooth/update_coordinator.py | 11 +++++++++++ homeassistant/components/govee_ble/__init__.py | 5 ++++- homeassistant/components/govee_ble/sensor.py | 2 +- homeassistant/components/inkbird/__init__.py | 5 ++++- homeassistant/components/inkbird/sensor.py | 2 +- homeassistant/components/moat/__init__.py | 5 ++++- homeassistant/components/moat/sensor.py | 2 +- .../components/sensorpush/__init__.py | 5 ++++- homeassistant/components/sensorpush/sensor.py | 2 +- .../components/xiaomi_ble/__init__.py | 5 ++++- homeassistant/components/xiaomi_ble/sensor.py | 2 +- .../bluetooth/test_passive_update_processor.py | 18 ++++++++++++++++++ 14 files changed, 54 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/bluetooth/passive_update_coordinator.py b/homeassistant/components/bluetooth/passive_update_coordinator.py index 4a22f03449e..97e7ddc49ee 100644 --- a/homeassistant/components/bluetooth/passive_update_coordinator.py +++ b/homeassistant/components/bluetooth/passive_update_coordinator.py @@ -42,17 +42,6 @@ class PassiveBluetoothDataUpdateCoordinator(BasePassiveBluetoothCoordinator): super()._async_handle_unavailable(address) self.async_update_listeners() - @callback - def async_start(self) -> CALLBACK_TYPE: - """Start the data updater.""" - self._async_start() - - @callback - def _async_cancel() -> None: - self._async_stop() - - return _async_cancel - @callback def async_add_listener( self, update_callback: CALLBACK_TYPE, context: Any = None diff --git a/homeassistant/components/bluetooth/passive_update_processor.py b/homeassistant/components/bluetooth/passive_update_processor.py index 2e4118000bd..43467701879 100644 --- a/homeassistant/components/bluetooth/passive_update_processor.py +++ b/homeassistant/components/bluetooth/passive_update_processor.py @@ -78,21 +78,10 @@ class PassiveBluetoothProcessorCoordinator(BasePassiveBluetoothCoordinator): def remove_processor() -> None: """Remove a processor.""" self._processors.remove(processor) - self._async_handle_processors_changed() self._processors.append(processor) - self._async_handle_processors_changed() return remove_processor - @callback - def _async_handle_processors_changed(self) -> None: - """Handle processors changed.""" - running = bool(self._cancel_bluetooth_advertisements) - if running and not self._processors: - self._async_stop() - elif not running and self._processors: - self._async_start() - @callback def _async_handle_unavailable(self, address: str) -> None: """Handle the device going unavailable.""" diff --git a/homeassistant/components/bluetooth/update_coordinator.py b/homeassistant/components/bluetooth/update_coordinator.py index d45514ab9ab..b1cb2de1453 100644 --- a/homeassistant/components/bluetooth/update_coordinator.py +++ b/homeassistant/components/bluetooth/update_coordinator.py @@ -38,6 +38,17 @@ class BasePassiveBluetoothCoordinator: self._present = False self.last_seen = 0.0 + @callback + def async_start(self) -> CALLBACK_TYPE: + """Start the data updater.""" + self._async_start() + + @callback + def _async_cancel() -> None: + self._async_stop() + + return _async_cancel + @property def available(self) -> bool: """Return if the device is available.""" diff --git a/homeassistant/components/govee_ble/__init__.py b/homeassistant/components/govee_ble/__init__.py index 3cb9e4659d6..3099d401e9b 100644 --- a/homeassistant/components/govee_ble/__init__.py +++ b/homeassistant/components/govee_ble/__init__.py @@ -21,7 +21,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Govee BLE device from a config entry.""" address = entry.unique_id assert address is not None - hass.data.setdefault(DOMAIN, {})[ + coordinator = hass.data.setdefault(DOMAIN, {})[ entry.entry_id ] = PassiveBluetoothProcessorCoordinator( hass, @@ -29,6 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: address=address, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + entry.async_on_unload( + coordinator.async_start() + ) # only start after all platforms have had a chance to subscribe return True diff --git a/homeassistant/components/govee_ble/sensor.py b/homeassistant/components/govee_ble/sensor.py index b8af7df8fb9..d0b9447d9e3 100644 --- a/homeassistant/components/govee_ble/sensor.py +++ b/homeassistant/components/govee_ble/sensor.py @@ -135,12 +135,12 @@ async def async_setup_entry( data.update(service_info) ) ) - entry.async_on_unload(coordinator.async_register_processor(processor)) entry.async_on_unload( processor.async_add_entities_listener( GoveeBluetoothSensorEntity, async_add_entities ) ) + entry.async_on_unload(coordinator.async_register_processor(processor)) class GoveeBluetoothSensorEntity( diff --git a/homeassistant/components/inkbird/__init__.py b/homeassistant/components/inkbird/__init__.py index 574f2a23355..5553b1c6ded 100644 --- a/homeassistant/components/inkbird/__init__.py +++ b/homeassistant/components/inkbird/__init__.py @@ -21,7 +21,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up INKBIRD BLE device from a config entry.""" address = entry.unique_id assert address is not None - hass.data.setdefault(DOMAIN, {})[ + coordinator = hass.data.setdefault(DOMAIN, {})[ entry.entry_id ] = PassiveBluetoothProcessorCoordinator( hass, @@ -29,6 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: address=address, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + entry.async_on_unload( + coordinator.async_start() + ) # only start after all platforms have had a chance to subscribe return True diff --git a/homeassistant/components/inkbird/sensor.py b/homeassistant/components/inkbird/sensor.py index 6b472edefe9..0648ca80383 100644 --- a/homeassistant/components/inkbird/sensor.py +++ b/homeassistant/components/inkbird/sensor.py @@ -135,12 +135,12 @@ async def async_setup_entry( data.update(service_info) ) ) - entry.async_on_unload(coordinator.async_register_processor(processor)) entry.async_on_unload( processor.async_add_entities_listener( INKBIRDBluetoothSensorEntity, async_add_entities ) ) + entry.async_on_unload(coordinator.async_register_processor(processor)) class INKBIRDBluetoothSensorEntity( diff --git a/homeassistant/components/moat/__init__.py b/homeassistant/components/moat/__init__.py index bd32ef64a0f..259b6b66709 100644 --- a/homeassistant/components/moat/__init__.py +++ b/homeassistant/components/moat/__init__.py @@ -21,7 +21,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Moat BLE device from a config entry.""" address = entry.unique_id assert address is not None - hass.data.setdefault(DOMAIN, {})[ + coordinator = hass.data.setdefault(DOMAIN, {})[ entry.entry_id ] = PassiveBluetoothProcessorCoordinator( hass, @@ -29,6 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: address=address, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + entry.async_on_unload( + coordinator.async_start() + ) # only start after all platforms have had a chance to subscribe return True diff --git a/homeassistant/components/moat/sensor.py b/homeassistant/components/moat/sensor.py index f29111a3406..295e2877aed 100644 --- a/homeassistant/components/moat/sensor.py +++ b/homeassistant/components/moat/sensor.py @@ -142,12 +142,12 @@ async def async_setup_entry( data.update(service_info) ) ) - entry.async_on_unload(coordinator.async_register_processor(processor)) entry.async_on_unload( processor.async_add_entities_listener( MoatBluetoothSensorEntity, async_add_entities ) ) + entry.async_on_unload(coordinator.async_register_processor(processor)) class MoatBluetoothSensorEntity( diff --git a/homeassistant/components/sensorpush/__init__.py b/homeassistant/components/sensorpush/__init__.py index 0f0d4d4fe8f..0a9efcbc752 100644 --- a/homeassistant/components/sensorpush/__init__.py +++ b/homeassistant/components/sensorpush/__init__.py @@ -21,7 +21,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up SensorPush BLE device from a config entry.""" address = entry.unique_id assert address is not None - hass.data.setdefault(DOMAIN, {})[ + coordinator = hass.data.setdefault(DOMAIN, {})[ entry.entry_id ] = PassiveBluetoothProcessorCoordinator( hass, @@ -29,6 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: address=address, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + entry.async_on_unload( + coordinator.async_start() + ) # only start after all platforms have had a chance to subscribe return True diff --git a/homeassistant/components/sensorpush/sensor.py b/homeassistant/components/sensorpush/sensor.py index 3921cbe43dd..9bfa59e3876 100644 --- a/homeassistant/components/sensorpush/sensor.py +++ b/homeassistant/components/sensorpush/sensor.py @@ -136,12 +136,12 @@ async def async_setup_entry( data.update(service_info) ) ) - entry.async_on_unload(coordinator.async_register_processor(processor)) entry.async_on_unload( processor.async_add_entities_listener( SensorPushBluetoothSensorEntity, async_add_entities ) ) + entry.async_on_unload(coordinator.async_register_processor(processor)) class SensorPushBluetoothSensorEntity( diff --git a/homeassistant/components/xiaomi_ble/__init__.py b/homeassistant/components/xiaomi_ble/__init__.py index a40dc8995d1..4eb20dbd943 100644 --- a/homeassistant/components/xiaomi_ble/__init__.py +++ b/homeassistant/components/xiaomi_ble/__init__.py @@ -21,7 +21,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Xiaomi BLE device from a config entry.""" address = entry.unique_id assert address is not None - hass.data.setdefault(DOMAIN, {})[ + coordinator = hass.data.setdefault(DOMAIN, {})[ entry.entry_id ] = PassiveBluetoothProcessorCoordinator( hass, @@ -29,6 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: address=address, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + entry.async_on_unload( + coordinator.async_start() + ) # only start after all platforms have had a chance to subscribe return True diff --git a/homeassistant/components/xiaomi_ble/sensor.py b/homeassistant/components/xiaomi_ble/sensor.py index 9cdf661f5ad..f33c864fa4a 100644 --- a/homeassistant/components/xiaomi_ble/sensor.py +++ b/homeassistant/components/xiaomi_ble/sensor.py @@ -167,12 +167,12 @@ async def async_setup_entry( data.update(service_info) ) ) - entry.async_on_unload(coordinator.async_register_processor(processor)) entry.async_on_unload( processor.async_add_entities_listener( XiaomiBluetoothSensorEntity, async_add_entities ) ) + entry.async_on_unload(coordinator.async_register_processor(processor)) class XiaomiBluetoothSensorEntity( diff --git a/tests/components/bluetooth/test_passive_update_processor.py b/tests/components/bluetooth/test_passive_update_processor.py index e5f992eebb2..ebb69d7c7d0 100644 --- a/tests/components/bluetooth/test_passive_update_processor.py +++ b/tests/components/bluetooth/test_passive_update_processor.py @@ -107,6 +107,7 @@ async def test_basic_usage(hass, mock_bleak_scanner_start): _async_register_callback, ): unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() entity_key = PassiveBluetoothEntityKey("temperature", None) entity_key_events = [] @@ -171,6 +172,7 @@ async def test_basic_usage(hass, mock_bleak_scanner_start): assert coordinator.available is True unregister_processor() + cancel_coordinator() async def test_unavailable_after_no_data(hass, mock_bleak_scanner_start): @@ -206,6 +208,7 @@ async def test_unavailable_after_no_data(hass, mock_bleak_scanner_start): _async_register_callback, ): unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() mock_entity = MagicMock() mock_add_entities = MagicMock() @@ -259,6 +262,7 @@ async def test_unavailable_after_no_data(hass, mock_bleak_scanner_start): assert processor.available is False unregister_processor() + cancel_coordinator() async def test_no_updates_once_stopping(hass, mock_bleak_scanner_start): @@ -290,6 +294,7 @@ async def test_no_updates_once_stopping(hass, mock_bleak_scanner_start): _async_register_callback, ): unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() all_events = [] @@ -310,6 +315,7 @@ async def test_no_updates_once_stopping(hass, mock_bleak_scanner_start): saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO, BluetoothChange.ADVERTISEMENT) assert len(all_events) == 1 unregister_processor() + cancel_coordinator() async def test_exception_from_update_method(hass, caplog, mock_bleak_scanner_start): @@ -346,6 +352,7 @@ async def test_exception_from_update_method(hass, caplog, mock_bleak_scanner_sta _async_register_callback, ): unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() processor.async_add_listener(MagicMock()) @@ -361,6 +368,7 @@ async def test_exception_from_update_method(hass, caplog, mock_bleak_scanner_sta saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO, BluetoothChange.ADVERTISEMENT) assert processor.available is True unregister_processor() + cancel_coordinator() async def test_bad_data_from_update_method(hass, mock_bleak_scanner_start): @@ -397,6 +405,7 @@ async def test_bad_data_from_update_method(hass, mock_bleak_scanner_start): _async_register_callback, ): unregister_processor = coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() processor.async_add_listener(MagicMock()) @@ -413,6 +422,7 @@ async def test_bad_data_from_update_method(hass, mock_bleak_scanner_start): saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO, BluetoothChange.ADVERTISEMENT) assert processor.available is True unregister_processor() + cancel_coordinator() GOVEE_B5178_REMOTE_SERVICE_INFO = BluetoothServiceInfo( @@ -737,6 +747,7 @@ async def test_integration_with_entity(hass, mock_bleak_scanner_start): _async_register_callback, ): coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() processor.async_add_listener(MagicMock()) @@ -781,6 +792,7 @@ async def test_integration_with_entity(hass, mock_bleak_scanner_start): assert entity_one.entity_key == PassiveBluetoothEntityKey( key="temperature", device_id="remote" ) + cancel_coordinator() NO_DEVICES_BLUETOOTH_SERVICE_INFO = BluetoothServiceInfo( @@ -845,6 +857,7 @@ async def test_integration_with_entity_without_a_device(hass, mock_bleak_scanner _async_register_callback, ): coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() mock_add_entities = MagicMock() @@ -873,6 +886,7 @@ async def test_integration_with_entity_without_a_device(hass, mock_bleak_scanner assert entity_one.entity_key == PassiveBluetoothEntityKey( key="temperature", device_id=None ) + cancel_coordinator() async def test_passive_bluetooth_entity_with_entity_platform( @@ -907,6 +921,7 @@ async def test_passive_bluetooth_entity_with_entity_platform( _async_register_callback, ): coordinator.async_register_processor(processor) + cancel_coordinator = coordinator.async_start() processor.async_add_entities_listener( PassiveBluetoothProcessorEntity, @@ -926,6 +941,7 @@ async def test_passive_bluetooth_entity_with_entity_platform( hass.states.get("test_domain.test_platform_aa_bb_cc_dd_ee_ff_pressure") is not None ) + cancel_coordinator() SENSOR_PASSIVE_BLUETOOTH_DATA_UPDATE = PassiveBluetoothDataUpdate( @@ -999,6 +1015,7 @@ async def test_integration_multiple_entity_platforms(hass, mock_bleak_scanner_st ): coordinator.async_register_processor(binary_sensor_processor) coordinator.async_register_processor(sesnor_processor) + cancel_coordinator = coordinator.async_start() binary_sensor_processor.async_add_listener(MagicMock()) sesnor_processor.async_add_listener(MagicMock()) @@ -1056,3 +1073,4 @@ async def test_integration_multiple_entity_platforms(hass, mock_bleak_scanner_st assert binary_sensor_entity_one.entity_key == PassiveBluetoothEntityKey( key="motion", device_id=None ) + cancel_coordinator()