Accept advertisements from alternate scanners when a scanner stops scanning (#82448)

This commit is contained in:
J. Nick Koston 2022-11-21 20:23:07 -06:00 committed by GitHub
parent dfed57ed4d
commit a7caa038be
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 275 additions and 68 deletions

View file

@ -51,6 +51,7 @@ from .api import (
async_rediscover_address,
async_register_callback,
async_register_scanner,
async_scanner_by_source,
async_scanner_count,
async_track_unavailable,
)
@ -86,6 +87,7 @@ __all__ = [
"async_register_callback",
"async_register_scanner",
"async_track_unavailable",
"async_scanner_by_source",
"async_scanner_count",
"BaseHaScanner",
"BaseHaRemoteScanner",

View file

@ -44,6 +44,19 @@ def async_get_scanner(hass: HomeAssistant) -> HaBleakScannerWrapper:
return HaBleakScannerWrapper()
@hass_callback
def async_scanner_by_source(hass: HomeAssistant, source: str) -> BaseHaScanner | None:
"""Return a scanner for a given source.
This method is only intended to be used by integrations that implement
a bluetooth client and need to interact with a scanner directly.
It is not intended to be used by integrations that need to interact
with a device.
"""
return _get_manager(hass).async_scanner_by_source(source)
@hass_callback
def async_scanner_count(hass: HomeAssistant, connectable: bool = True) -> int:
"""Return the number of scanners currently in use."""

View file

@ -2,7 +2,8 @@
from __future__ import annotations
from abc import abstractmethod
from collections.abc import Callable
from collections.abc import Callable, Generator
from contextlib import contextmanager
import datetime
from datetime import timedelta
from typing import Any, Final
@ -10,6 +11,7 @@ from typing import Any, Final
from bleak.backends.device import BLEDevice
from bleak.backends.scanner import AdvertisementData
from bleak_retry_connector import NO_RSSI_VALUE
from bluetooth_adapters import adapter_human_name
from home_assistant_bluetooth import BluetoothServiceInfoBleak
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback as hass_callback
@ -28,10 +30,26 @@ MONOTONIC_TIME: Final = monotonic_time_coarse
class BaseHaScanner:
"""Base class for Ha Scanners."""
def __init__(self, hass: HomeAssistant, source: str) -> None:
__slots__ = ("hass", "source", "_connecting", "name", "scanning")
def __init__(self, hass: HomeAssistant, source: str, adapter: str) -> None:
"""Initialize the scanner."""
self.hass = hass
self.source = source
self._connecting = 0
self.name = adapter_human_name(adapter, source) if adapter != source else source
self.scanning = True
@contextmanager
def connecting(self) -> Generator[None, None, None]:
"""Context manager to track connecting state."""
self._connecting += 1
self.scanning = not self._connecting
try:
yield
finally:
self._connecting -= 1
self.scanning = not self._connecting
@property
@abstractmethod
@ -62,16 +80,27 @@ class BaseHaScanner:
class BaseHaRemoteScanner(BaseHaScanner):
"""Base class for a Home Assistant remote BLE scanner."""
__slots__ = (
"_new_info_callback",
"_discovered_device_advertisement_datas",
"_discovered_device_timestamps",
"_connector",
"_connectable",
"_details",
"_expire_seconds",
)
def __init__(
self,
hass: HomeAssistant,
scanner_id: str,
name: str,
new_info_callback: Callable[[BluetoothServiceInfoBleak], None],
connector: HaBluetoothConnector,
connectable: bool,
) -> None:
"""Initialize the scanner."""
super().__init__(hass, scanner_id)
super().__init__(hass, scanner_id, name)
self._new_info_callback = new_info_callback
self._discovered_device_advertisement_datas: dict[
str, tuple[BLEDevice, AdvertisementData]

View file

@ -126,7 +126,7 @@ class BluetoothManager:
self._non_connectable_scanners: list[BaseHaScanner] = []
self._connectable_scanners: list[BaseHaScanner] = []
self._adapters: dict[str, AdapterDetails] = {}
self._sources: set[str] = set()
self._sources: dict[str, BaseHaScanner] = {}
self._bluetooth_adapters = bluetooth_adapters
@property
@ -169,6 +169,11 @@ class BluetoothManager:
return adapter
return None
@hass_callback
def async_scanner_by_source(self, source: str) -> BaseHaScanner | None:
"""Return the scanner for a source."""
return self._sources.get(source)
async def async_get_bluetooth_adapters(
self, cached: bool = True
) -> dict[str, AdapterDetails]:
@ -302,6 +307,7 @@ class BluetoothManager:
self,
old: BluetoothServiceInfoBleak,
new: BluetoothServiceInfoBleak,
debug: bool,
) -> bool:
"""Prefer previous advertisement from a different source if it is better."""
if new.time - old.time > (
@ -310,34 +316,32 @@ class BluetoothManager:
)
):
# If the old advertisement is stale, any new advertisement is preferred
_LOGGER.debug(
"%s (%s): Switching from %s[%s] to %s[%s] (time elapsed:%s > stale seconds:%s)",
new.name,
new.address,
old.source,
old.connectable,
new.source,
new.connectable,
new.time - old.time,
stale_seconds,
)
if debug:
_LOGGER.debug(
"%s (%s): Switching from %s to %s (time elapsed:%s > stale seconds:%s)",
new.name,
new.address,
self._async_describe_source(old),
self._async_describe_source(new),
new.time - old.time,
stale_seconds,
)
return False
if (new.rssi or NO_RSSI_VALUE) - RSSI_SWITCH_THRESHOLD > (
old.rssi or NO_RSSI_VALUE
):
# If new advertisement is RSSI_SWITCH_THRESHOLD more, the new one is preferred
_LOGGER.debug(
"%s (%s): Switching from %s[%s] to %s[%s] (new rssi:%s - threshold:%s > old rssi:%s)",
new.name,
new.address,
old.source,
old.connectable,
new.source,
new.connectable,
new.rssi,
RSSI_SWITCH_THRESHOLD,
old.rssi,
)
if debug:
_LOGGER.debug(
"%s (%s): Switching from %s to %s (new rssi:%s - threshold:%s > old rssi:%s)",
new.name,
new.address,
self._async_describe_source(old),
self._async_describe_source(new),
new.rssi,
RSSI_SWITCH_THRESHOLD,
old.rssi,
)
return False
return True
@ -367,6 +371,7 @@ class BluetoothManager:
connectable_history = self._connectable_history
source = service_info.source
debug = _LOGGER.isEnabledFor(logging.DEBUG)
# This logic is complex due to the many combinations of scanners that are supported.
#
# We need to handle multiple connectable and non-connectable scanners
@ -384,9 +389,10 @@ class BluetoothManager:
if (
(old_service_info := all_history.get(address))
and source != old_service_info.source
and old_service_info.source in self._sources
and (scanner := self._sources.get(old_service_info.source))
and scanner.scanning
and self._prefer_previous_adv_from_different_source(
old_service_info, service_info
old_service_info, service_info, debug
)
):
# If we are rejecting the new advertisement and the device is connectable
@ -404,9 +410,14 @@ class BluetoothManager:
# the old connectable advertisement
or (
source != old_connectable_service_info.source
and old_connectable_service_info.source in self._sources
and (
connectable_scanner := self._sources.get(
old_connectable_service_info.source
)
)
and connectable_scanner.scanning
and self._prefer_previous_adv_from_different_source(
old_connectable_service_info, service_info
old_connectable_service_info, service_info, debug
)
)
):
@ -461,15 +472,14 @@ class BluetoothManager:
)
matched_domains = self._integration_matcher.match_domains(service_info)
_LOGGER.debug(
"%s: %s %s connectable: %s match: %s rssi: %s",
source,
address,
advertisement_data,
connectable,
matched_domains,
advertisement_data.rssi,
)
if debug:
_LOGGER.debug(
"%s: %s %s match: %s",
self._async_describe_source(service_info),
address,
advertisement_data,
matched_domains,
)
if is_connectable_by_any_source:
# Bleak callbacks must get a connectable device
@ -491,6 +501,17 @@ class BluetoothManager:
service_info,
)
@hass_callback
def _async_describe_source(self, service_info: BluetoothServiceInfoBleak) -> str:
"""Describe a source."""
if scanner := self._sources.get(service_info.source):
description = scanner.name
else:
description = service_info.source
if service_info.connectable:
description += " [connectable]"
return description
@hass_callback
def async_track_unavailable(
self,
@ -611,15 +632,17 @@ class BluetoothManager:
self, scanner: BaseHaScanner, connectable: bool
) -> CALLBACK_TYPE:
"""Register a new scanner."""
_LOGGER.debug("Registering scanner %s", scanner.name)
scanners = self._get_scanners_by_type(connectable)
def _unregister_scanner() -> None:
_LOGGER.debug("Unregistering scanner %s", scanner.name)
self._advertisement_tracker.async_remove_source(scanner.source)
scanners.remove(scanner)
self._sources.remove(scanner.source)
del self._sources[scanner.source]
scanners.append(scanner)
self._sources.add(scanner.source)
self._sources[scanner.source] = scanner
return _unregister_scanner
@hass_callback

View file

@ -16,7 +16,7 @@ from bleak.backends.bluezdbus.advertisement_monitor import OrPattern
from bleak.backends.bluezdbus.scanner import BlueZScannerArgs
from bleak.backends.device import BLEDevice
from bleak.backends.scanner import AdvertisementData, AdvertisementDataCallback
from bluetooth_adapters import DEFAULT_ADDRESS, adapter_human_name
from bluetooth_adapters import DEFAULT_ADDRESS
from dbus_fast import InvalidMessageError
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback as hass_callback
@ -130,7 +130,7 @@ class HaScanner(BaseHaScanner):
) -> None:
"""Init bluetooth discovery."""
source = address if address != DEFAULT_ADDRESS else adapter or SOURCE_LOCAL
super().__init__(hass, source)
super().__init__(hass, source, adapter)
self.mode = mode
self.adapter = adapter
self._start_stop_lock = asyncio.Lock()
@ -138,7 +138,7 @@ class HaScanner(BaseHaScanner):
self._last_detection = 0.0
self._start_time = 0.0
self._new_info_callback = new_info_callback
self.name = adapter_human_name(adapter, address)
self.scanning = False
@property
def discovered_devices(self) -> list[BLEDevice]:
@ -312,6 +312,7 @@ class HaScanner(BaseHaScanner):
# Everything is fine, break out of the loop
break
self.scanning = True
self._async_setup_scanner_watchdog()
@hass_callback
@ -385,6 +386,7 @@ class HaScanner(BaseHaScanner):
async def _async_stop_scanner(self) -> None:
"""Stop bluetooth discovery under the lock."""
self.scanning = False
_LOGGER.debug("%s: Stopping bluetooth discovery", self.name)
try:
await self.scanner.stop() # type: ignore[no-untyped-call]

View file

@ -67,7 +67,9 @@ async def async_connect_scanner(
source=source,
can_connect=_async_can_connect_factory(entry_data, source),
)
scanner = ESPHomeScanner(hass, source, new_info_callback, connector, connectable)
scanner = ESPHomeScanner(
hass, source, entry.title, new_info_callback, connector, connectable
)
unload_callbacks = [
async_register_scanner(hass, scanner, connectable),
scanner.async_setup(),

View file

@ -21,6 +21,7 @@ from bleak.backends.device import BLEDevice
from bleak.backends.service import BleakGATTServiceCollection
from bleak.exc import BleakError
from homeassistant.components.bluetooth import async_scanner_by_source
from homeassistant.core import CALLBACK_TYPE
from ..domain_data import DomainData
@ -119,11 +120,12 @@ class ESPHomeClient(BaseBleakClient):
"""Initialize the ESPHomeClient."""
assert isinstance(address_or_ble_device, BLEDevice)
super().__init__(address_or_ble_device, *args, **kwargs)
self._hass = kwargs["hass"]
self._ble_device = address_or_ble_device
self._address_as_int = mac_to_int(self._ble_device.address)
assert self._ble_device.details is not None
self._source = self._ble_device.details["source"]
self.domain_data = DomainData.get(kwargs["hass"])
self.domain_data = DomainData.get(self._hass)
config_entry = self.domain_data.get_by_unique_id(self._source)
self.entry_data = self.domain_data.get_entry_data(config_entry)
self._client = self.entry_data.client
@ -257,12 +259,15 @@ class ESPHomeClient(BaseBleakClient):
connected_future.set_result(connected)
timeout = kwargs.get("timeout", self._timeout)
self._cancel_connection_state = await self._client.bluetooth_device_connect(
self._address_as_int,
_on_bluetooth_connection_state,
timeout=timeout,
)
await connected_future
if not (scanner := async_scanner_by_source(self._hass, self._source)):
raise BleakError("Scanner disappeared for {self._source}")
with scanner.connecting():
self._cancel_connection_state = await self._client.bluetooth_device_connect(
self._address_as_int,
_on_bluetooth_connection_state,
timeout=timeout,
)
await connected_future
await self.get_services(dangerous_use_bleak_cache=dangerous_use_bleak_cache)
self._disconnected_event = asyncio.Event()
return True

View file

@ -34,6 +34,7 @@ async def async_connect_scanner(
) -> CALLBACK_TYPE:
"""Connect scanner."""
device = coordinator.device
entry = coordinator.entry
source = format_mac(coordinator.mac).upper()
new_info_callback = async_get_advertisement_callback(hass)
connector = HaBluetoothConnector(
@ -42,7 +43,9 @@ async def async_connect_scanner(
source=source,
can_connect=lambda: False,
)
scanner = ShellyBLEScanner(hass, source, new_info_callback, connector, False)
scanner = ShellyBLEScanner(
hass, source, entry.title, new_info_callback, connector, False
)
unload_callbacks = [
async_register_scanner(hass, scanner, False),
scanner.async_setup(),

View file

@ -314,7 +314,7 @@ async def test_advertisment_interval_longer_than_adapter_stack_timeout_adapter_c
"""Return a list of discovered devices."""
return {}
scanner = FakeScanner(hass, "new")
scanner = FakeScanner(hass, "new", "fake_adapter")
cancel_scanner = async_register_scanner(hass, scanner, False)
@callback

View file

@ -0,0 +1,16 @@
"""Tests for the Bluetooth integration API."""
from homeassistant.components import bluetooth
from homeassistant.components.bluetooth import BaseHaScanner, async_scanner_by_source
async def test_scanner_by_source(hass, enable_bluetooth):
"""Test we can get a scanner by source."""
hci2_scanner = BaseHaScanner(hass, "hci2", "hci2")
cancel_hci2 = bluetooth.async_register_scanner(hass, hci2_scanner, True)
assert async_scanner_by_source(hass, "hci2") is hci2_scanner
cancel_hci2()
assert async_scanner_by_source(hass, "hci2") is None

View file

@ -20,7 +20,7 @@ from . import MockBleakClient, _get_manager, generate_advertisement_data
from tests.common import async_fire_time_changed
async def test_remote_scanner(hass):
async def test_remote_scanner(hass, enable_bluetooth):
"""Test the remote scanner base class merges advertisement_data."""
manager = _get_manager()
@ -70,7 +70,7 @@ async def test_remote_scanner(hass):
connector = (
HaBluetoothConnector(MockBleakClient, "mock_bleak_client", lambda: False),
)
scanner = FakeScanner(hass, "esp32", new_info_callback, connector, True)
scanner = FakeScanner(hass, "esp32", "esp32", new_info_callback, connector, True)
scanner.async_setup()
cancel = manager.async_register_scanner(scanner, True)
@ -104,7 +104,7 @@ async def test_remote_scanner(hass):
cancel()
async def test_remote_scanner_expires_connectable(hass):
async def test_remote_scanner_expires_connectable(hass, enable_bluetooth):
"""Test the remote scanner expires stale connectable data."""
manager = _get_manager()
@ -140,7 +140,7 @@ async def test_remote_scanner_expires_connectable(hass):
connector = (
HaBluetoothConnector(MockBleakClient, "mock_bleak_client", lambda: False),
)
scanner = FakeScanner(hass, "esp32", new_info_callback, connector, True)
scanner = FakeScanner(hass, "esp32", "esp32", new_info_callback, connector, True)
scanner.async_setup()
cancel = manager.async_register_scanner(scanner, True)
@ -174,7 +174,7 @@ async def test_remote_scanner_expires_connectable(hass):
cancel()
async def test_remote_scanner_expires_non_connectable(hass):
async def test_remote_scanner_expires_non_connectable(hass, enable_bluetooth):
"""Test the remote scanner expires stale non connectable data."""
manager = _get_manager()
@ -210,7 +210,7 @@ async def test_remote_scanner_expires_non_connectable(hass):
connector = (
HaBluetoothConnector(MockBleakClient, "mock_bleak_client", lambda: False),
)
scanner = FakeScanner(hass, "esp32", new_info_callback, connector, False)
scanner = FakeScanner(hass, "esp32", "esp32", new_info_callback, connector, False)
scanner.async_setup()
cancel = manager.async_register_scanner(scanner, True)
@ -265,3 +265,61 @@ async def test_remote_scanner_expires_non_connectable(hass):
assert len(scanner.discovered_devices_and_advertisement_data) == 0
cancel()
async def test_base_scanner_connecting_behavior(hass, enable_bluetooth):
"""Test that the default behavior is to mark the scanner as not scanning when connecting."""
manager = _get_manager()
switchbot_device = BLEDevice(
"44:44:33:11:23:45",
"wohand",
{},
rssi=-100,
)
switchbot_device_adv = generate_advertisement_data(
local_name="wohand",
service_uuids=[],
manufacturer_data={1: b"\x01"},
rssi=-100,
)
class FakeScanner(BaseHaRemoteScanner):
def inject_advertisement(
self, device: BLEDevice, advertisement_data: AdvertisementData
) -> None:
"""Inject an advertisement."""
self._async_on_advertisement(
device.address,
advertisement_data.rssi,
device.name,
advertisement_data.service_uuids,
advertisement_data.service_data,
advertisement_data.manufacturer_data,
advertisement_data.tx_power,
)
new_info_callback = manager.scanner_adv_received
connector = (
HaBluetoothConnector(MockBleakClient, "mock_bleak_client", lambda: False),
)
scanner = FakeScanner(hass, "esp32", "esp32", new_info_callback, connector, False)
scanner.async_setup()
cancel = manager.async_register_scanner(scanner, True)
with scanner.connecting():
assert scanner.scanning is False
# We should still accept new advertisements while connecting
# since advertisements are delivered asynchronously and
# we don't want to miss any even when we are willing to
# accept advertisements from another scanner in the brief window
# between when we start connecting and when we stop scanning
scanner.inject_advertisement(switchbot_device, switchbot_device_adv)
devices = scanner.discovered_devices
assert len(scanner.discovered_devices) == 1
assert len(scanner.discovered_devices_and_advertisement_data) == 1
assert devices[0].name == "wohand"
cancel()

View file

@ -2649,7 +2649,7 @@ async def test_getting_the_scanner_returns_the_wrapped_instance(hass, enable_blu
async def test_scanner_count_connectable(hass, enable_bluetooth):
"""Test getting the connectable scanner count."""
scanner = BaseHaScanner(hass, "any")
scanner = BaseHaScanner(hass, "any", "any")
cancel = bluetooth.async_register_scanner(hass, scanner, False)
assert bluetooth.async_scanner_count(hass, connectable=True) == 1
cancel()
@ -2657,7 +2657,7 @@ async def test_scanner_count_connectable(hass, enable_bluetooth):
async def test_scanner_count(hass, enable_bluetooth):
"""Test getting the connectable and non-connectable scanner count."""
scanner = BaseHaScanner(hass, "any")
scanner = BaseHaScanner(hass, "any", "any")
cancel = bluetooth.async_register_scanner(hass, scanner, False)
assert bluetooth.async_scanner_count(hass, connectable=False) == 2
cancel()

View file

@ -26,7 +26,8 @@ from . import (
@pytest.fixture
def register_hci0_scanner(hass: HomeAssistant) -> None:
"""Register an hci0 scanner."""
cancel = bluetooth.async_register_scanner(hass, BaseHaScanner(hass, "hci0"), True)
hci0_scanner = BaseHaScanner(hass, "hci0", "hci0")
cancel = bluetooth.async_register_scanner(hass, hci0_scanner, True)
yield
cancel()
@ -34,7 +35,8 @@ def register_hci0_scanner(hass: HomeAssistant) -> None:
@pytest.fixture
def register_hci1_scanner(hass: HomeAssistant) -> None:
"""Register an hci1 scanner."""
cancel = bluetooth.async_register_scanner(hass, BaseHaScanner(hass, "hci1"), True)
hci1_scanner = BaseHaScanner(hass, "hci1", "hci1")
cancel = bluetooth.async_register_scanner(hass, hci1_scanner, True)
yield
cancel()
@ -416,7 +418,7 @@ async def test_switching_adapters_when_one_goes_away(
):
"""Test switching adapters when one goes away."""
cancel_hci2 = bluetooth.async_register_scanner(
hass, BaseHaScanner(hass, "hci2"), True
hass, BaseHaScanner(hass, "hci2", "hci2"), True
)
address = "44:44:33:11:23:45"
@ -460,3 +462,55 @@ async def test_switching_adapters_when_one_goes_away(
bluetooth.async_ble_device_from_address(hass, address)
is switchbot_device_poor_signal
)
async def test_switching_adapters_when_one_stop_scanning(
hass, enable_bluetooth, register_hci0_scanner
):
"""Test switching adapters when stops scanning."""
hci2_scanner = BaseHaScanner(hass, "hci2", "hci2")
cancel_hci2 = bluetooth.async_register_scanner(hass, hci2_scanner, True)
address = "44:44:33:11:23:45"
switchbot_device_good_signal = BLEDevice(address, "wohand_good_signal")
switchbot_adv_good_signal = generate_advertisement_data(
local_name="wohand_good_signal", service_uuids=[], rssi=-60
)
inject_advertisement_with_source(
hass, switchbot_device_good_signal, switchbot_adv_good_signal, "hci2"
)
assert (
bluetooth.async_ble_device_from_address(hass, address)
is switchbot_device_good_signal
)
switchbot_device_poor_signal = BLEDevice(address, "wohand_poor_signal")
switchbot_adv_poor_signal = generate_advertisement_data(
local_name="wohand_poor_signal", service_uuids=[], rssi=-100
)
inject_advertisement_with_source(
hass, switchbot_device_poor_signal, switchbot_adv_poor_signal, "hci0"
)
# We want to prefer the good signal when we have options
assert (
bluetooth.async_ble_device_from_address(hass, address)
is switchbot_device_good_signal
)
hci2_scanner.scanning = False
inject_advertisement_with_source(
hass, switchbot_device_poor_signal, switchbot_adv_poor_signal, "hci0"
)
# Now that hci2 has stopped scanning, we should prefer the poor signal
# since poor signal is better than no signal
assert (
bluetooth.async_ble_device_from_address(hass, address)
is switchbot_device_poor_signal
)
cancel_hci2()

View file

@ -200,7 +200,7 @@ async def test_ble_device_with_proxy_client_out_of_connections_uses_best_availab
return switchbot_proxy_device_has_connection_slot
return None
scanner = FakeScanner(hass, "esp32")
scanner = FakeScanner(hass, "esp32", "esp32")
cancel = manager.async_register_scanner(scanner, True)
assert manager.async_discovered_devices(True) == [
switchbot_proxy_device_no_connection_slot
@ -306,7 +306,7 @@ async def test_ble_device_with_proxy_client_out_of_connections_uses_best_availab
return switchbot_proxy_device_has_connection_slot
return None
scanner = FakeScanner(hass, "esp32")
scanner = FakeScanner(hass, "esp32", "esp32")
cancel = manager.async_register_scanner(scanner, True)
assert manager.async_discovered_devices(True) == [
switchbot_proxy_device_no_connection_slot