Skip to content

Commit

Permalink
feat: speed up stopping the scanner when its stuck setting up (#37)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco authored Apr 17, 2024
1 parent e613943 commit bba8b51
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 2 deletions.
13 changes: 12 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ bleak-retry-connector = ">=3.3.0"
bluetooth-data-tools = ">=1.16.0"
bluetooth-adapters = ">=0.16.1"
bluetooth-auto-recovery = ">=1.2.3"
async-interrupt = ">=1.1.1"

[tool.poetry.group.dev.dependencies]
pytest = "^7.0"
Expand Down
1 change: 1 addition & 0 deletions src/habluetooth/scanner.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ cdef class HaScanner(BaseHaScanner):
cdef public object _start_stop_lock
cdef public object _background_tasks
cdef public object scanner
cdef public object _start_future

cpdef void _async_detection_callback(
self,
Expand Down
32 changes: 31 additions & 1 deletion src/habluetooth/scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import platform
from typing import Any, Coroutine, Iterable

import async_interrupt
import bleak
from bleak import BleakError
from bleak.assigned_numbers import AdvertisementDataType
Expand Down Expand Up @@ -83,6 +84,10 @@
)


class _AbortStartError(Exception):
"""Error to indicate that the start should be aborted."""


class ScannerStartError(Exception):
"""Error to indicate that the scanner failed to start."""

Expand Down Expand Up @@ -156,6 +161,7 @@ def __init__(
self.scanning = False
self._background_tasks: set[asyncio.Task[Any]] = set()
self.scanner: bleak.BleakScanner | None = None
self._start_future: asyncio.Future[None] | None = None

def _create_background_task(self, coro: Coroutine[Any, Any, None]) -> None:
"""Create a background task and add it to the background tasks set."""
Expand Down Expand Up @@ -264,6 +270,10 @@ async def _async_on_successful_start(self) -> None:

async def _async_start_attempt(self, attempt: int) -> bool:
"""Start the scanner and handle errors."""
assert ( # noqa: S101
self._loop is not None
), "Loop is not set, call async_setup first"

mode = self.mode
# 1st attempt - no auto reset
# 2nd attempt - try to reset the adapter and wait a bit
Expand All @@ -288,9 +298,15 @@ async def _async_start_attempt(self, attempt: int) -> bool:
self._async_detection_callback, mode, self.adapter
)
self._log_start_attempt(attempt)
self._start_future = self._loop.create_future()
try:
async with asyncio.timeout(START_TIMEOUT):
async with asyncio.timeout(START_TIMEOUT), async_interrupt.interrupt(
self._start_future, _AbortStartError, None
):
await self.scanner.start()
except _AbortStartError as ex:
await self._async_stop_scanner()
self._raise_for_abort_start(ex)
except InvalidMessageError as ex:
await self._async_stop_scanner()
self._raise_for_invalid_dbus_message(ex)
Expand Down Expand Up @@ -335,6 +351,8 @@ async def _async_start_attempt(self, attempt: int) -> bool:
except BaseException:
await self._async_stop_scanner()
raise
finally:
self._start_future = None

self._log_start_success(attempt)
return True
Expand Down Expand Up @@ -381,6 +399,16 @@ def _log_start_attempt(self, attempt: int) -> None:
START_ATTEMPTS,
)

def _raise_for_abort_start(self, ex: _AbortStartError) -> None:
_LOGGER.debug(
"%s: Starting bluetooth scanner aborted: %s",
self.name,
ex,
exc_info=True,
)
msg = f"{self.name}: Starting bluetooth scanner aborted"
raise ScannerStartError(msg) from ex

def _raise_for_file_not_found_error(self, ex: FileNotFoundError) -> None:
_LOGGER.debug(
"%s: FileNotFoundError while starting bluetooth: %s",
Expand Down Expand Up @@ -482,6 +510,8 @@ async def _async_reset_adapter(self) -> None:

async def async_stop(self) -> None:
"""Stop bluetooth scanner."""
if self._start_future is not None and not self._start_future.done():
self._start_future.set_exception(_AbortStartError())
async with self._start_stop_lock:
self._async_stop_scanner_watchdog()
await self._async_stop_scanner()
Expand Down
33 changes: 33 additions & 0 deletions tests/test_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ async def test_dbus_socket_missing_in_container(
),
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
await scanner.async_start()
assert mock_stop.called
await scanner.async_stop()
Expand All @@ -109,6 +110,7 @@ async def test_dbus_socket_missing(caplog: pytest.LogCaptureFixture) -> None:
),
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
await scanner.async_start()
assert mock_stop.called
await scanner.async_stop()
Expand All @@ -129,11 +131,39 @@ async def test_handle_cancellation(caplog: pytest.LogCaptureFixture) -> None:
) as mock_stop,
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
with pytest.raises(asyncio.CancelledError):
await scanner.async_start()
assert mock_stop.called


@pytest.mark.asyncio
@pytest.mark.skipif("platform.system() != 'Linux'")
async def test_handle_stop_while_starting(caplog: pytest.LogCaptureFixture) -> None:
"""Test stop while starting."""

async def _start(*args, **kwargs):
await asyncio.sleep(1000)

with (
patch("habluetooth.scanner.is_docker_env", return_value=False),
patch("habluetooth.scanner.OriginalBleakScanner.start", _start),
patch(
"habluetooth.scanner.OriginalBleakScanner.stop",
) as mock_stop,
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
task = asyncio.create_task(scanner.async_start())
await asyncio.sleep(0)
await scanner.async_stop()
with pytest.raises(
ScannerStartError, match="Starting bluetooth scanner aborted"
):
await task
assert mock_stop.called


@pytest.mark.asyncio
@pytest.mark.skipif("platform.system() != 'Linux'")
async def test_dbus_broken_pipe_in_container(caplog: pytest.LogCaptureFixture) -> None:
Expand All @@ -150,6 +180,7 @@ async def test_dbus_broken_pipe_in_container(caplog: pytest.LogCaptureFixture) -
pytest.raises(ScannerStartError, match="DBus connection broken"),
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
await scanner.async_start()
assert mock_stop.called
await scanner.async_stop()
Expand All @@ -171,6 +202,7 @@ async def test_dbus_broken_pipe(caplog: pytest.LogCaptureFixture) -> None:
pytest.raises(ScannerStartError, match="DBus connection broken:"),
):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
await scanner.async_start()
assert mock_stop.called
await scanner.async_stop()
Expand All @@ -185,6 +217,7 @@ async def test_invalid_dbus_message(caplog: pytest.LogCaptureFixture) -> None:
side_effect=InvalidMessageError,
), pytest.raises(ScannerStartError, match="Invalid DBus message received"):
scanner = HaScanner(BluetoothScanningMode.ACTIVE, "hci0", "AA:BB:CC:DD:EE:FF")
scanner.async_setup()
await scanner.async_start()
await scanner.async_stop()

Expand Down

0 comments on commit bba8b51

Please sign in to comment.