From 98dd84f53543cdfd2351abf620006cb4ab659b90 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 17 Sep 2022 03:26:02 -0500 Subject: [PATCH] Ensure bluetooth callbacks are only fired when advertisement data changes (#78609) --- homeassistant/components/bluetooth/manager.py | 13 +++- .../test_active_update_coordinator.py | 19 +++-- tests/components/bluetooth/test_init.py | 75 +++++++++++++++---- .../test_passive_update_processor.py | 43 ++++++++--- 4 files changed, 120 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/bluetooth/manager.py b/homeassistant/components/bluetooth/manager.py index 47941c8d5c1..edc9bb1edde 100644 --- a/homeassistant/components/bluetooth/manager.py +++ b/homeassistant/components/bluetooth/manager.py @@ -323,12 +323,23 @@ class BluetoothManager: return self._history[address] = service_info - source = service_info.source if connectable: self._connectable_history[address] = service_info # Bleak callbacks must get a connectable device + # If the advertisement data is the same as the last time we saw it, we + # don't need to do anything else. + if old_service_info and not ( + service_info.manufacturer_data != old_service_info.manufacturer_data + or service_info.service_data != old_service_info.service_data + or service_info.service_uuids != old_service_info.service_uuids + ): + return + + source = service_info.source + if connectable: + # Bleak callbacks must get a connectable device for callback_filters in self._bleak_callbacks: _dispatch_bleak_callback(*callback_filters, device, advertisement_data) diff --git a/tests/components/bluetooth/test_active_update_coordinator.py b/tests/components/bluetooth/test_active_update_coordinator.py index ecec5f12171..4dbe32d69d2 100644 --- a/tests/components/bluetooth/test_active_update_coordinator.py +++ b/tests/components/bluetooth/test_active_update_coordinator.py @@ -36,6 +36,15 @@ GENERIC_BLUETOOTH_SERVICE_INFO = BluetoothServiceInfo( service_uuids=[], source="local", ) +GENERIC_BLUETOOTH_SERVICE_INFO_2 = BluetoothServiceInfo( + name="Generic", + address="aa:bb:cc:dd:ee:ff", + rssi=-95, + manufacturer_data={1: b"\x01\x01\x01\x01\x01\x01\x01\x01", 2: b"\x02"}, + service_data={}, + service_uuids=[], + source="local", +) async def test_basic_usage(hass: HomeAssistant, mock_bleak_scanner_start): @@ -128,7 +137,7 @@ async def test_poll_can_be_skipped(hass: HomeAssistant, mock_bleak_scanner_start flag = False - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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}) @@ -196,7 +205,7 @@ async def test_bleak_error_and_recover( # Second poll works flag = False - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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": False}) @@ -251,7 +260,7 @@ async def test_poll_failure_and_recover(hass: HomeAssistant, mock_bleak_scanner_ # Second poll works flag = False - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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": False}) @@ -297,7 +306,7 @@ async def test_second_poll_needed(hass: HomeAssistant, mock_bleak_scanner_start) # First poll gets queued inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) # Second poll gets stuck behind first poll - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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": 1}) @@ -343,7 +352,7 @@ async def test_rate_limit(hass: HomeAssistant, mock_bleak_scanner_start): # First poll gets queued inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) # Second poll gets stuck behind first poll - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) # Third poll gets stuck behind first poll doesn't get queued inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) diff --git a/tests/components/bluetooth/test_init.py b/tests/components/bluetooth/test_init.py index 527ab879164..33627f6a787 100644 --- a/tests/components/bluetooth/test_init.py +++ b/tests/components/bluetooth/test_init.py @@ -475,7 +475,9 @@ async def test_discovery_match_by_local_name( assert len(mock_config_flow.mock_calls) == 0 switchbot_device = BLEDevice("44:44:33:11:23:45", "wohand") - switchbot_adv = AdvertisementData(local_name="wohand", service_uuids=[]) + switchbot_adv = AdvertisementData( + local_name="wohand", service_uuids=[], manufacturer_data={1: b"\x01"} + ) inject_advertisement(hass, switchbot_device, switchbot_adv) await hass.async_block_till_done() @@ -875,7 +877,11 @@ async def test_rediscovery(hass, mock_bleak_scanner_start, enable_bluetooth): switchbot_adv = AdvertisementData( local_name="wohand", service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"] ) - + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={1: b"\x01"}, + ) inject_advertisement(hass, switchbot_device, switchbot_adv) await hass.async_block_till_done() @@ -887,7 +893,7 @@ async def test_rediscovery(hass, mock_bleak_scanner_start, enable_bluetooth): async_rediscover_address(hass, "44:44:33:11:23:45") - inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) await hass.async_block_till_done() assert len(mock_config_flow.mock_calls) == 2 @@ -1876,7 +1882,12 @@ async def test_register_callback_survives_reload( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, ) - + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["zba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, + ) inject_advertisement(hass, switchbot_device, switchbot_adv) assert len(callbacks) == 1 service_info: BluetoothServiceInfo = callbacks[0][0] @@ -1888,7 +1899,7 @@ async def test_register_callback_survives_reload( await hass.config_entries.async_reload(entry.entry_id) await hass.async_block_till_done() - inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) assert len(callbacks) == 2 service_info: BluetoothServiceInfo = callbacks[1][0] assert service_info.name == "wohand" @@ -1950,6 +1961,12 @@ async def test_process_advertisements_ignore_bad_advertisement( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fa": b""}, ) + adv2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51a"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fa": b""}, + ) def _callback(service_info: BluetoothServiceInfo) -> bool: done.set() @@ -1969,6 +1986,7 @@ async def test_process_advertisements_ignore_bad_advertisement( # callback that returns False while not done.is_set(): inject_advertisement(hass, device, adv) + inject_advertisement(hass, device, adv2) await asyncio.sleep(0) # Set the return value and mutate the advertisement @@ -1976,6 +1994,7 @@ async def test_process_advertisements_ignore_bad_advertisement( return_value.set() adv.service_data["00000d00-0000-1000-8000-00805f9b34fa"] = b"H\x10c" inject_advertisement(hass, device, adv) + inject_advertisement(hass, device, adv2) await asyncio.sleep(0) result = await handle @@ -2024,6 +2043,12 @@ async def test_wrapped_instance_with_filter( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, ) + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, + ) empty_device = BLEDevice("11:22:33:44:55:66", "empty") empty_adv = AdvertisementData(local_name="empty") @@ -2033,7 +2058,7 @@ async def test_wrapped_instance_with_filter( ) scanner.register_detection_callback(_device_detected) - inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) await hass.async_block_till_done() discovered = await scanner.discover(timeout=0) @@ -2090,6 +2115,12 @@ async def test_wrapped_instance_with_service_uuids( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, ) + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, + ) empty_device = BLEDevice("11:22:33:44:55:66", "empty") empty_adv = AdvertisementData(local_name="empty") @@ -2099,9 +2130,10 @@ async def test_wrapped_instance_with_service_uuids( ) scanner.register_detection_callback(_device_detected) - for _ in range(2): - inject_advertisement(hass, switchbot_device, switchbot_adv) - await hass.async_block_till_done() + inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) + + await hass.async_block_till_done() assert len(detected) == 2 @@ -2182,6 +2214,12 @@ async def test_wrapped_instance_changes_uuids( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, ) + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, + ) empty_device = BLEDevice("11:22:33:44:55:66", "empty") empty_adv = AdvertisementData(local_name="empty") @@ -2192,9 +2230,9 @@ async def test_wrapped_instance_changes_uuids( ) scanner.register_detection_callback(_device_detected) - for _ in range(2): - inject_advertisement(hass, switchbot_device, switchbot_adv) - await hass.async_block_till_done() + inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) + await hass.async_block_till_done() assert len(detected) == 2 @@ -2231,6 +2269,12 @@ async def test_wrapped_instance_changes_filters( manufacturer_data={89: b"\xd8.\xad\xcd\r\x85"}, service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, ) + switchbot_adv_2 = AdvertisementData( + local_name="wohand", + service_uuids=["cba20d00-224d-11e6-9fb8-0002a5d5c51b"], + manufacturer_data={89: b"\xd8.\xad\xcd\r\x84"}, + service_data={"00000d00-0000-1000-8000-00805f9b34fb": b"H\x10c"}, + ) empty_device = BLEDevice("11:22:33:44:55:62", "empty") empty_adv = AdvertisementData(local_name="empty") @@ -2241,9 +2285,10 @@ async def test_wrapped_instance_changes_filters( ) scanner.register_detection_callback(_device_detected) - for _ in range(2): - inject_advertisement(hass, switchbot_device, switchbot_adv) - await hass.async_block_till_done() + inject_advertisement(hass, switchbot_device, switchbot_adv) + inject_advertisement(hass, switchbot_device, switchbot_adv_2) + + await hass.async_block_till_done() assert len(detected) == 2 diff --git a/tests/components/bluetooth/test_passive_update_processor.py b/tests/components/bluetooth/test_passive_update_processor.py index 99b16131ddc..e7260d22e4b 100644 --- a/tests/components/bluetooth/test_passive_update_processor.py +++ b/tests/components/bluetooth/test_passive_update_processor.py @@ -56,6 +56,18 @@ GENERIC_BLUETOOTH_SERVICE_INFO = BluetoothServiceInfo( service_uuids=[], source="local", ) +GENERIC_BLUETOOTH_SERVICE_INFO_2 = BluetoothServiceInfo( + name="Generic", + address="aa:bb:cc:dd:ee:ff", + rssi=-95, + manufacturer_data={ + 1: b"\x01\x01\x01\x01\x01\x01\x01\x01", + 2: b"\x02", + }, + service_data={}, + service_uuids=[], + source="local", +) GENERIC_PASSIVE_BLUETOOTH_DATA_UPDATE = PassiveBluetoothDataUpdate( devices={ @@ -156,7 +168,7 @@ async def test_basic_usage(hass, mock_bleak_scanner_start): # There should be 4 calls to create entities assert len(mock_entity.mock_calls) == 2 - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) # Each listener should receive the same data # since both match @@ -325,7 +337,7 @@ async def test_no_updates_once_stopping(hass, mock_bleak_scanner_start): hass.state = CoreState.stopping # We should stop processing events once hass is stopping - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) assert len(all_events) == 1 unregister_processor() cancel_coordinator() @@ -383,7 +395,7 @@ async def test_exception_from_update_method(hass, caplog, mock_bleak_scanner_sta assert processor.available is True # We should go unavailable once we get an exception - saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO, BluetoothChange.ADVERTISEMENT) + saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO_2, BluetoothChange.ADVERTISEMENT) assert "Test exception" in caplog.text assert processor.available is False @@ -447,7 +459,7 @@ async def test_bad_data_from_update_method(hass, mock_bleak_scanner_start): # We should go unavailable once we get bad data with pytest.raises(ValueError): - saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO, BluetoothChange.ADVERTISEMENT) + saved_callback(GENERIC_BLUETOOTH_SERVICE_INFO_2, BluetoothChange.ADVERTISEMENT) assert processor.available is False @@ -796,7 +808,7 @@ async def test_integration_with_entity(hass, mock_bleak_scanner_start): # First call with just the remote sensor entities results in them being added assert len(mock_add_entities.mock_calls) == 1 - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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 @@ -804,7 +816,7 @@ async def test_integration_with_entity(hass, mock_bleak_scanner_start): # Third call with primary and remote sensor entities adds the primary sensor entities assert len(mock_add_entities.mock_calls) == 2 - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + 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 @@ -840,6 +852,19 @@ NO_DEVICES_BLUETOOTH_SERVICE_INFO = BluetoothServiceInfo( service_uuids=[], source="local", ) + +NO_DEVICES_BLUETOOTH_SERVICE_INFO_2 = BluetoothServiceInfo( + name="Generic", + address="aa:bb:cc:dd:ee:ff", + rssi=-95, + manufacturer_data={ + 1: b"\x01\x01\x01\x01\x01\x01\x01\x01", + 2: b"\x02", + }, + service_data={}, + service_uuids=[], + source="local", +) NO_DEVICES_PASSIVE_BLUETOOTH_DATA_UPDATE = PassiveBluetoothDataUpdate( devices={}, entity_data={ @@ -905,7 +930,7 @@ async def test_integration_with_entity_without_a_device(hass, mock_bleak_scanner # First call with just the remote sensor entities results in them being added assert len(mock_add_entities.mock_calls) == 1 - inject_bluetooth_service_info(hass, NO_DEVICES_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, NO_DEVICES_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 @@ -967,7 +992,7 @@ async def test_passive_bluetooth_entity_with_entity_platform( ) inject_bluetooth_service_info(hass, NO_DEVICES_BLUETOOTH_SERVICE_INFO) await hass.async_block_till_done() - inject_bluetooth_service_info(hass, NO_DEVICES_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, NO_DEVICES_BLUETOOTH_SERVICE_INFO_2) await hass.async_block_till_done() assert ( hass.states.get("test_domain.test_platform_aa_bb_cc_dd_ee_ff_temperature") @@ -1157,7 +1182,7 @@ async def test_exception_from_coordinator_update_method( assert processor.available is True # We should go unavailable once we get an exception - inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO) + inject_bluetooth_service_info(hass, GENERIC_BLUETOOTH_SERVICE_INFO_2) assert "Test exception" in caplog.text assert processor.available is False