diff --git a/packages/pb-cli/src/pb_cli/commands/hub.py b/packages/pb-cli/src/pb_cli/commands/hub.py index 5211b98..88a2241 100644 --- a/packages/pb-cli/src/pb_cli/commands/hub.py +++ b/packages/pb-cli/src/pb_cli/commands/hub.py @@ -14,16 +14,19 @@ from __future__ import annotations import logging -from collections.abc import AsyncIterator, Mapping +import sys +from collections.abc import AsyncGenerator, AsyncIterator, Callable, Mapping +from contextlib import AbstractAsyncContextManager, asynccontextmanager from typing import Annotated, Final +import anyio import cyclopts from rich.console import Console, RenderableType import pb_client from pb_cli import errors, keys, picker, ui from pb_cli.app import HUB_COMMANDS, Verbose, app, configure_logging -from pb_cli.errors import EXIT_OK +from pb_cli.errors import EXIT_OK, PERMISSION_NAME from pb_core.jsonrpc import ErrorCode logger = logging.getLogger(__name__) @@ -93,6 +96,63 @@ def _connect_hints(transport: str | None) -> Mapping[int, str]: return {ErrorCode.HUB_NOT_FOUND: hint} +def _asked(platform: str = sys.platform) -> str: + """Say that the system is asking whether the service may use Bluetooth. + + Args: + platform: What to say it for. Only macOS asks, today. + + Returns: + The sentence, with markup. + """ + who = "macOS" if platform == "darwin" else "The system" + return ( + f"{who} is asking whether [bold]{PERMISSION_NAME}[/] may use " + "Bluetooth. Answer the dialog to carry on." + ) + + +@asynccontextmanager +async def _telling_when_asked( + service: pb_client.Service, say: Callable[[bool], None] +) -> AsyncGenerator[None, None]: + """Say when the system starts or stops asking for Bluetooth, while hubs are sought. + + Open it before the scan or connection it is for, so that nothing it reports + is missed. The search itself carries on once the user answers, so this only + has to tell them to; what changes is whether they look for the dialog now + or are told to answer it once the search has given up. + + Args: + service: Where the search runs. + say: Called with True when the system starts asking, and with False if + it stops before the search ends. + + Yields: + Nothing. The block is the search. + """ + + async def follow( + changes: AsyncIterator[tuple[pb_client.UnusableAdapter, ...]], + ) -> None: + asked = False + + async for unusable in changes: + now = any(adapter.awaiting_permission for adapter in unusable) + + if now != asked: + asked = now + say(asked) + + async with service.watching_adapters() as changes, anyio.create_task_group() as tg: + tg.start_soon(follow, changes) + + try: + yield + finally: + tg.cancel_scope.cancel() + + def _looks_like_id(value: str) -> bool: """Whether a string is a hub id rather than a hub name.""" return value.startswith(_ID_PREFIXES) @@ -134,32 +194,55 @@ async def _scan(seconds: float, name: str | None, transport: str | None) -> int: seen: dict[str, pb_client.DeviceInfo] = {} caption = f"Scanning for {seconds:.0f}s..." - async with ( - pb_client.connect() as service, - service.scanning(seconds=seconds, name=name, transport=transport) as devices, - ): + def scanning( + service: pb_client.Service, + ) -> AbstractAsyncContextManager[AsyncIterator[pb_client.DeviceInfo]]: + return service.scanning(seconds=seconds, name=name, transport=transport) + + async with pb_client.connect() as service: if out.is_terminal: # Redrawn in place as advertisements arrive, so a hub appears the # moment it is heard and its signal moves as it is carried about. with ui.live(ui.scan_table((), caption), out) as display: - async for device in devices: - seen[device.id] = device + + def say(asked: bool) -> None: + # In the caption, not printed: this display is on stdout, + # and a line on stderr would land in the middle of it. + nonlocal caption + caption = _asked() if asked else f"Scanning for {seconds:.0f}s..." display.update(ui.scan_table(seen.values(), caption), refresh=True) + async with ( + _telling_when_asked(service, say), + scanning(service) as devices, + ): + async for device in devices: + seen[device.id] = device + display.update( + ui.scan_table(seen.values(), caption), refresh=True + ) + display.update(ui.scan_table(seen.values(), f"Found {len(seen)}.")) else: - # Piped: one line per hub, printed the moment it is first heard, so - # that "pb scan | head -1" answers as soon as there is an answer. - async for device in devices: - if device.id not in seen: - line = ( - f"{device.name}\t{device.id}" - if ui.SHOW_HUB_IDS - else device.name - ) - ui.plain(out, line) - seen[device.id] = device + def tell(asked: bool) -> None: + if asked: + err.print(_asked()) + + async with _telling_when_asked(service, tell), scanning(service) as devices: + # Piped: one line per hub, printed the moment it is first + # heard, so that "pb scan | head -1" answers as soon as there + # is an answer. + async for device in devices: + if device.id not in seen: + line = ( + f"{device.name}\t{device.id}" + if ui.SHOW_HUB_IDS + else device.name + ) + ui.plain(out, line) + + seen[device.id] = device if not seen: if transport == "rfcomm": @@ -225,11 +308,20 @@ async def connect_cmd( async def _connect(hub: str | None, scan: bool, transport: str | None) -> int: err = ui.console(err=True) + def tell(asked: bool) -> None: + # Printed above the spinner or the chooser, both of which are drawn + # on this console and so make room for it. + if asked: + err.print(_asked()) + async with pb_client.connect() as service: if scan: # No time limit: the hub may still be in the next room. The scan # ends when the block does, whether the user chose or gave up. - async with service.scanning(seconds=None, transport=transport) as devices: + async with ( + _telling_when_asked(service, tell), + service.scanning(seconds=None, transport=transport) as devices, + ): device = await picker.choose_hub(err, devices, ui.chooser_table) if device is None: @@ -243,11 +335,12 @@ async def _connect(hub: str | None, scan: bool, transport: str | None) -> int: # chosen the route, since each row is one. else: with ui.spinner(err, "Connecting..."): - info = await service.connect_hub( - hub_id=hub if hub and _looks_like_id(hub) else None, - name=hub if hub and not _looks_like_id(hub) else None, - transport=transport, - ) + async with _telling_when_asked(service, tell): + info = await service.connect_hub( + hub_id=hub if hub and _looks_like_id(hub) else None, + name=hub if hub and not _looks_like_id(hub) else None, + transport=transport, + ) # The protocol version belongs to pb status: it explains why a command was # refused, but on a successful connection it is one more number to read diff --git a/packages/pb-cli/src/pb_cli/errors.py b/packages/pb-cli/src/pb_cli/errors.py index e455d6f..e2d6701 100644 --- a/packages/pb-cli/src/pb_cli/errors.py +++ b/packages/pb-cli/src/pb_cli/errors.py @@ -52,7 +52,9 @@ "EXIT_INTERRUPTED", "EXIT_NOTHING_CONNECTED", "EXIT_OK", + "PERMISSION_NAME", "Problem", + "bluetooth_advice", "explain", "find", "flatten", @@ -154,6 +156,70 @@ def serial_port_advice(platform: str = sys.platform) -> str: ) +PERMISSION_NAME = "Pybricks RFCOMM Service" +"""What macOS calls this program when it asks about Bluetooth. + +pb-service's ``PERMISSION_NAME``, which it passes to aio-rfcomm's +``prompt_under_name()``, and so the one in the dialog and under Privacy & +Security. Repeated rather than imported, so that explaining an error does not +load the service; a test holds the two together. +""" + +_BLUETOOTH = ( + "Switch Bluetooth on, or plug in an adapter, and check that this " + "computer lets pb use Bluetooth." +) +"""What to suggest when the service could not say why Bluetooth is unusable.""" + + +def bluetooth_advice(availability: str | None, platform: str = sys.platform) -> str: + """Say what to do about Bluetooth that cannot be used. + + Args: + availability: Why, as the service names it -- one word per member of + aio-rfcomm's ``Availability`` -- or ``None`` if it did not say. + platform: What to advise for. An argument for the same reason as in + :func:`serial_port_advice`. + + Returns: + What to suggest. + """ + mac = platform == "darwin" + + match availability: + case "switched_off": + return "Switch Bluetooth on, then try again." + case "no_adapter": + return "Plug in a Bluetooth adapter, then try again." + case "awaiting_permission": + # Only once the service has given up waiting: until then the CLI + # says this as it happens, and carries on when it is answered. + what = "the dialog" if mac else "the system's question" + return ( + f"Answer {what} asking whether [bold]{PERMISSION_NAME}[/] may " + "use Bluetooth, then try again." + ) + case "not_allowed" if mac: + # Conditional, because a profile or parental controls refuse it + # in the same words, and those are not the user's to undo. + return ( + f"If you refused it for [bold]{PERMISSION_NAME}[/], allow it " + "in System Settings under Privacy & Security > Bluetooth, " + "then try again." + ) + case "not_allowed": + return "Check that this computer lets pb use Bluetooth." + case "service_unavailable": + return ( + "The system's Bluetooth service usually restarts by itself. " + "Wait a moment, then try again." + ) + case "unsupported": + return "If the hub has a USB port, connect it with a cable instead." + case _: + return _BLUETOOTH + + _RULES: Mapping[int, _Rule] = { # Hubs. The service's own wording is specific -- "no connected hub named # 'Spare Hub'" -- so it is usually kept as the headline and only the @@ -202,12 +268,9 @@ def serial_port_advice(platform: str = sys.platform) -> str: ), # Transport and lifecycle. # The service repeats what the system said, which is what tells a radio - # that is switched off from one this program may not use. - ErrorCode.BLUETOOTH_UNAVAILABLE: _Rule( - "Switch Bluetooth on, or plug in an adapter, and check that this " - "computer lets pb use Bluetooth.", - exit_code=EXIT_ERROR, - ), + # that is switched off from one this program may not use, and names it in + # a word, which is what chooses the suggestion (see bluetooth_advice). + ErrorCode.BLUETOOTH_UNAVAILABLE: _Rule(_BLUETOOTH, exit_code=EXIT_ERROR), ErrorCode.SCAN_TIMEOUT: _Rule( "Switch the hub on and press its button so that its light flashes." ), @@ -373,6 +436,24 @@ def _slot_hint(exc: JsonRpcError, default: str | None) -> str | None: return _ONE_SLOT if data.get("slots") == 1 else default +def _bluetooth_hint(exc: JsonRpcError) -> str: + """Suggest what to do about Bluetooth, by the reason the service gave. + + Args: + exc: The error the service sent. + + Returns: + The suggestion to print. + """ + payload: object = exc.data + if not isinstance(payload, Mapping): + return bluetooth_advice(None) + + data = cast("Mapping[str, object]", payload) + availability = data.get("availability") + return bluetooth_advice(availability if isinstance(availability, str) else None) + + def _describe_rpc(exc: JsonRpcError, hints: Mapping[int, str] | None) -> Problem | None: """Present an error the service reported, or ``None`` if the code is unknown.""" rule = _RULES.get(exc.code) @@ -391,6 +472,9 @@ def _describe_rpc(exc: JsonRpcError, hints: Mapping[int, str] | None) -> Problem if exc.code == ErrorCode.PROGRAM_SLOT_NOT_SELECTED: hint = _slot_hint(exc, hint) + if exc.code == ErrorCode.BLUETOOTH_UNAVAILABLE: + hint = _bluetooth_hint(exc) + if exc.code == ErrorCode.UNSUPPORTED_PROTOCOL_VERSION: message, hint = _version_mismatch(exc, message, hint) diff --git a/packages/pb-cli/tests/test_errors.py b/packages/pb-cli/tests/test_errors.py index c08b9a4..b1490f3 100644 --- a/packages/pb-cli/tests/test_errors.py +++ b/packages/pb-cli/tests/test_errors.py @@ -11,6 +11,7 @@ import asyncio import io +import sys from pathlib import Path import anyio @@ -439,6 +440,57 @@ def test_unusable_bluetooth_keeps_the_systems_words() -> None: assert "Low Energy" not in problems[0].hint +@pytest.mark.parametrize( + ("availability", "expected"), + [ + ("switched_off", "Switch Bluetooth on"), + ("no_adapter", "Plug in a Bluetooth adapter"), + # The two only macOS reports, worded for it there. + ( + "not_allowed", + "Privacy & Security" if sys.platform == "darwin" else "lets pb use", + ), + ("awaiting_permission", "Answer the"), + ("service_unavailable", "Wait a moment"), + ("unsupported", "USB"), + (None, "Switch Bluetooth on, or plug in an adapter"), + ("something_newer", "Switch Bluetooth on, or plug in an adapter"), + ], +) +def test_unusable_bluetooth_gets_the_advice_for_its_reason( + availability: str | None, expected: str +) -> None: + # One suggestion covering every reason sent somebody whose permission + # dialog was open off to switch on a radio that was already on. + exc = JsonRpcError( + ErrorCode.BLUETOOTH_UNAVAILABLE, + "Bluetooth cannot be used: whatever the system said", + {"availability": availability} if availability else None, + ) + problems = errors.explain(exc) + + assert problems is not None + assert problems[0].hint is not None + assert expected in problems[0].hint + assert "whatever the system said" in problems[0].message + + +def test_the_name_is_the_one_the_service_asks_under() -> None: + from pb_service.backend import PERMISSION_NAME + + assert errors.PERMISSION_NAME == PERMISSION_NAME + + +def test_the_permission_advice_names_what_macos_shows() -> None: + # The dialog and System Settings name the service, not pb or Python. + for availability in ("awaiting_permission", "not_allowed"): + advice = errors.bluetooth_advice(availability, "darwin") + assert errors.PERMISSION_NAME in advice + + # Elsewhere there is no System Settings to send anybody to. + assert "System Settings" not in errors.bluetooth_advice("not_allowed", "linux") + + @pytest.mark.parametrize( ("platform", "expected"), [ diff --git a/packages/pb-client/src/pb_client/__init__.py b/packages/pb-client/src/pb_client/__init__.py index 25a4d57..de87568 100644 --- a/packages/pb-client/src/pb_client/__init__.py +++ b/packages/pb-client/src/pb_client/__init__.py @@ -34,6 +34,7 @@ Downloaded, HubInfo, ServiceInfo, + UnusableAdapter, ) from pb_client.service import Service, connect from pb_client.stdio import REPL_PROGRAM_ID, HubEvent, Output, State, Status @@ -59,6 +60,7 @@ "ServiceUnavailable", "State", "Status", + "UnusableAdapter", "__version__", "compile_program", "compile_script", diff --git a/packages/pb-client/src/pb_client/models.py b/packages/pb-client/src/pb_client/models.py index baa0e63..0245922 100644 --- a/packages/pb-client/src/pb_client/models.py +++ b/packages/pb-client/src/pb_client/models.py @@ -16,7 +16,14 @@ from dataclasses import dataclass from typing import Any, cast -__all__ = ["Choice", "DeviceInfo", "Downloaded", "HubInfo", "ServiceInfo"] +__all__ = [ + "Choice", + "DeviceInfo", + "Downloaded", + "HubInfo", + "ServiceInfo", + "UnusableAdapter", +] @dataclass(frozen=True, slots=True) @@ -90,6 +97,49 @@ def from_wire(cls, payload: dict[str, Any]) -> DeviceInfo: ) +@dataclass(frozen=True, slots=True) +class UnusableAdapter: + """A Bluetooth adapter a scan could not use, and why.""" + + address: str + """Which adapter. ``"bluetooth"`` where the system has only the one.""" + + reason: str + """The system's own words, which differ between platforms.""" + + availability: str | None = None + """Why, as one word to choose advice by, where the service knows it: + ``switched_off``, ``no_adapter``, ``not_allowed``, + ``awaiting_permission``, ``service_unavailable`` or ``unsupported``.""" + + @property + def awaiting_permission(self) -> bool: + """Whether the system is asking the user to let the service in. + + On macOS, a dialog asking whether the service may use Bluetooth, by + the name it asks under. It ends by itself when the user answers, and + a scan carries on then. + """ + return self.availability == "awaiting_permission" + + @classmethod + def from_wire(cls, payload: dict[str, Any]) -> UnusableAdapter: + """Build from one entry of a ``hub.adapters`` notification. + + Args: + payload: The entry. + + Returns: + The parsed adapter. + """ + availability = payload.get("availability") + return cls( + address=str(payload.get("address", "")), + reason=str(payload.get("reason", "")), + availability=str(availability) if availability else None, + ) + + @dataclass(frozen=True, slots=True) class Choice: """The hub that commands act on when none is named.""" diff --git a/packages/pb-client/src/pb_client/service.py b/packages/pb-client/src/pb_client/service.py index c391101..4c01008 100644 --- a/packages/pb-client/src/pb_client/service.py +++ b/packages/pb-client/src/pb_client/service.py @@ -31,12 +31,14 @@ Downloaded, HubInfo, ServiceInfo, + UnusableAdapter, ) from pb_client.stdio import HubEvent from pb_core import auth, ipc from pb_core.ipc import Endpoint from pb_core.jsonrpc import JsonRpcPeer, Request, connect_peer, encode_binary from pb_core.protocol import ( + HUB_ADAPTERS, HUB_FOUND, HUB_STATE, HUB_STATUS, @@ -311,6 +313,54 @@ async def scan_until_done() -> None: # method started outlives the block. tg.cancel_scope.cancel() + @asynccontextmanager + async def watching_adapters( + self, + ) -> AsyncGenerator[AsyncIterator[tuple[UnusableAdapter, ...]], None]: + """Follow which Bluetooth adapters cannot be used, while hubs are sought. + + Only a scan or a connection looking for a hub reports this, and only on + a change, so open this around one of those. What it is for: on macOS + the first use of Bluetooth Classic makes the system ask the user + whether the service may use Bluetooth, and nothing is found until they + answer. Telling them as soon as that happens, rather than when the + search gives up, is the difference between a dialog answered and one + nobody noticed. Meanwhile the service holds the search's time limit + still, and once the dialog is answered the search carries on. + + Yields: + Each change: every adapter that cannot be used, or an empty tuple + once all can again. Ends when the block is left. + + Raises: + RuntimeError: If this connection has no notification router, which + would leave the iterator silently empty. + """ + if self._notifications is None: + raise RuntimeError("this connection does not receive notifications") + + # Small and lossy, as with every subscriber: these are levels, so the + # next one corrects a dropped one. + sender, receiver = anyio.create_memory_object_stream[ + tuple[str, dict[str, Any]] + ](16) + self._notifications.add(HUB_ADAPTERS, sender) + + async def changes() -> AsyncGenerator[tuple[UnusableAdapter, ...], None]: + async with receiver: + async for _method, payload in receiver: + entries = cast("list[dict[str, Any]]", payload.get("unusable", [])) + yield tuple(UnusableAdapter.from_wire(e) for e in entries) + + stream = changes() + + try: + async with aclosing(stream): + yield stream + finally: + self._notifications.discard(HUB_ADAPTERS, sender) + sender.close() + async def connect_hub( self, *, diff --git a/packages/pb-core/src/pb_core/protocol.py b/packages/pb-core/src/pb_core/protocol.py index a21676e..51b1899 100644 --- a/packages/pb-core/src/pb_core/protocol.py +++ b/packages/pb-core/src/pb_core/protocol.py @@ -32,6 +32,7 @@ from typing import Final __all__ = [ + "HUB_ADAPTERS", "HUB_FOUND", "HUB_STATE", "HUB_STATUS", @@ -78,6 +79,23 @@ anywhere -- it simply stops arriving, which is the worst way to find out. """ +HUB_ADAPTERS: Final = "hub.adapters" +"""Notification sent to a session that is scanning or connecting when the +Bluetooth adapters it cannot use change. + +Its parameter ``unusable`` lists each such adapter as an ``address``, the +system's ``reason`` and, where the transport knows it, an ``availability`` +word such as ``switched_off`` or ``awaiting_permission``. An empty list means +everything is usable again. Sent only on a change, and not at all while +everything is usable, so most scans never send one. + +It exists for ``awaiting_permission``: macOS asking the user whether the +service may use Bluetooth. That ends by itself when the user answers, and the +search carries on, so it is said while the search is still running rather than +as an error at the end -- by which time the user would have had to notice a +dialog without being told to look for one. +""" + HUB_STDOUT: Final = "hub.stdout" """Notification carrying one chunk of a hub's program output. diff --git a/packages/pb-service/pyproject.toml b/packages/pb-service/pyproject.toml index 037bd77..44f3981 100644 --- a/packages/pb-service/pyproject.toml +++ b/packages/pb-service/pyproject.toml @@ -23,7 +23,7 @@ dependencies = [ "bleak>=1.1.0", "packaging>=22", "serialx>=1.9", - "aio-rfcomm>=1.0", + "aio-rfcomm>=1.1", "semver>=3", "typing-extensions>=4.15", ] diff --git a/packages/pb-service/src/pb_service/backend.py b/packages/pb-service/src/pb_service/backend.py index 6a7dc39..fe25f11 100644 --- a/packages/pb-service/src/pb_service/backend.py +++ b/packages/pb-service/src/pb_service/backend.py @@ -24,7 +24,12 @@ logger = logging.getLogger(__name__) -__all__ = ["HUB_BACKEND_ENV_VAR", "get_hub_transport", "open_hub_transport"] +__all__ = [ + "HUB_BACKEND_ENV_VAR", + "PERMISSION_NAME", + "get_hub_transport", + "open_hub_transport", +] HUB_BACKEND_ENV_VAR = "PB_HUB_BACKEND" """Selects the transport. @@ -35,6 +40,21 @@ """ +PERMISSION_NAME = "Pybricks RFCOMM Service" +"""What macOS calls this service when it asks about Bluetooth. + +Shown in the permission dialog and under Privacy & Security, and what the +answer is remembered against, so changing it asks every user again. pb-cli +repeats it in what it tells the user, and a test holds the two together. + +Not plain "Pybricks": among the other entries in Privacy & Security that +reads as the Pybricks app or Pybricks Code, and says nothing about what is +asking or why. This is the part of pb-service that reaches EV3 and NXT hubs +over Bluetooth Classic, and only that part is asked about: BLE is bleak's, +inside the service itself. +""" + + def _usb() -> HubTransport: from pb_service.hub.usb import UsbHubTransport @@ -145,7 +165,7 @@ async def open_hub_transport() -> AsyncGenerator[HubTransport, None]: # so without this it is refused rather than asked. The answer is # remembered under the name, so changing it asks the user again. aio_rfcomm.macos.prompt_under_name( - name="Pybricks", reason="to connect to LEGO hubs over Bluetooth" + name=PERMISSION_NAME, reason="to connect to LEGO hubs over Bluetooth" ) async with anyio.create_task_group() as tg: diff --git a/packages/pb-service/src/pb_service/hub/manager.py b/packages/pb-service/src/pb_service/hub/manager.py index b4c40e9..7045b27 100644 --- a/packages/pb-service/src/pb_service/hub/manager.py +++ b/packages/pb-service/src/pb_service/hub/manager.py @@ -171,7 +171,10 @@ def resolve(self, name_or_id: str) -> HubConnection: # ------------------------------------------------------------------ scan async def discover( - self, scan_filter: ScanFilter | None = None + self, + scan_filter: ScanFilter | None = None, + *, + started: anyio.Event | None = None, ) -> AsyncGenerator[DeviceInfo, None]: """Report hubs as they are seen, until the caller stops listening. @@ -180,12 +183,18 @@ async def discover( Args: scan_filter: Which hubs to report. + started: Set once this scan has the radio, rather than waiting + behind another. Until then, what the transports report is the + other scan's. Yields: Hubs as their advertisements arrive. """ async with self._scan_lock: + if started is not None: + started.set() + logger.info("scanning for hubs") seen = 0 diff --git a/packages/pb-service/src/pb_service/hub/rfcomm/fake.py b/packages/pb-service/src/pb_service/hub/rfcomm/fake.py index d92c678..54405b6 100644 --- a/packages/pb-service/src/pb_service/hub/rfcomm/fake.py +++ b/packages/pb-service/src/pb_service/hub/rfcomm/fake.py @@ -26,6 +26,7 @@ import aio_rfcomm import anyio +from aio_rfcomm import UnavailableReason from aio_rfcomm.errors import ( BluetoothUnavailableError, DeviceNotFoundError, @@ -130,9 +131,15 @@ async def __anext__(self) -> aio_rfcomm.Availability: @final class _FakeBluetooth: - def __init__(self, hubs: Sequence[FakePairedHub], unavailable: str | None) -> None: + def __init__( + self, + hubs: Sequence[FakePairedHub], + unavailable: str | None, + reason: UnavailableReason, + ) -> None: self._hubs = hubs self._unavailable = unavailable + self._reason: UnavailableReason = reason self.changed = anyio.Event() @property @@ -140,10 +147,11 @@ def availability(self) -> aio_rfcomm.Availability: if self._unavailable is None: return aio_rfcomm.Availability.USABLE - return aio_rfcomm.Availability.SWITCHED_OFF + return self._reason - def switch(self, unavailable: str | None) -> None: + def switch(self, unavailable: str | None, reason: UnavailableReason) -> None: self._unavailable = unavailable + self._reason = reason self.changed.set() self.changed = anyio.Event() @@ -152,9 +160,7 @@ def watch_availability(self) -> _FakeWatcher: def _check(self) -> None: if self._unavailable is not None: - raise BluetoothUnavailableError( - aio_rfcomm.Availability.SWITCHED_OFF, self._unavailable - ) + raise BluetoothUnavailableError(self._reason, self._unavailable) async def list_known_devices( self, *, service: UUID | Collection[UUID] | None = None @@ -186,7 +192,11 @@ async def open_service( port.close() -def radio(*hubs: FakePairedHub, unavailable: str | None = None) -> aio_rfcomm.Bluetooth: +def radio( + *hubs: FakePairedHub, + unavailable: str | None = None, + reason: UnavailableReason = aio_rfcomm.Availability.SWITCHED_OFF, +) -> aio_rfcomm.Bluetooth: """Build Bluetooth with these devices paired to it. Args: @@ -195,22 +205,30 @@ def radio(*hubs: FakePairedHub, unavailable: str | None = None) -> aio_rfcomm.Bl the switched-off or not-allowed case, which the transport has to report without stopping the other transports. Opening Bluetooth succeeds regardless, so it is every call that fails. + reason: Which of those it is, while it is unusable. Each calls for + different advice, and on macOS one of them -- a permission dialog + still open -- ends by itself when the user answers. Returns: Something to hand :class:`~pb_service.hub.rfcomm.transport. RfcommHubTransport`. Cast, because the real one is a class rather than a protocol and nothing structural can stand in for it. """ - return cast("aio_rfcomm.Bluetooth", _FakeBluetooth(hubs, unavailable)) + return cast("aio_rfcomm.Bluetooth", _FakeBluetooth(hubs, unavailable, reason)) -def switch(bluetooth: aio_rfcomm.Bluetooth, unavailable: str | None) -> None: +def switch( + bluetooth: aio_rfcomm.Bluetooth, + unavailable: str | None, + reason: UnavailableReason = aio_rfcomm.Availability.SWITCHED_OFF, +) -> None: """Make Bluetooth usable or unusable while it is in use. Args: bluetooth: What :func:`radio` built. unavailable: Why Bluetooth cannot be used from now on, or None for usable again. Watchers hear of it at once, as they would of the - user flipping the switch. + user flipping the switch, or answering the permission dialog. + reason: Which kind of unusable it is, as for :func:`radio`. """ - cast("_FakeBluetooth", bluetooth).switch(unavailable) + cast("_FakeBluetooth", bluetooth).switch(unavailable, reason) diff --git a/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py b/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py index fd086de..f4ac438 100644 --- a/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py +++ b/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py @@ -3,12 +3,16 @@ """Bluetooth Classic, opened the first time RFCOMM needs it. -Opening it is not free and may not finish. On macOS it starts aio-rfcomm's -helper, which may ask the user for permission, and it does not return until the -system has said whether Bluetooth is usable -- which, on a Mac where nobody is -there to answer the prompt, is never. Done as the service started, that held up -USB and BLE as well. So it is done in a task of the transport's own scope, the -first time a scan or a connection asks for it, as bleak does for BLE. +Opening it is not free. On macOS it starts aio-rfcomm's helper, which takes +about a second, and the first time it makes macOS ask the user whether +pb-service may use Bluetooth, by the name in +:data:`~pb_service.backend.PERMISSION_NAME`. The open does not wait for that +answer: the availability says ``AWAITING_PERMISSION`` until it comes, and +every call is refused meanwhile with that reason, which the transport reports +and the client turns into "answer the dialog". Done as the service started, the open would +still hold up USB and BLE, and ask a question about a radio nobody had asked +for. So it is done in a task of the transport's own scope, the first time a +scan or a connection asks for it, as bleak does for BLE. """ from __future__ import annotations @@ -21,6 +25,7 @@ import aio_rfcomm import anyio +from aio_rfcomm import UnavailableReason from aio_rfcomm.errors import BluetoothUnavailableError, RfcommError from anyio.abc import TaskGroup @@ -48,7 +53,7 @@ def __init__(self, group: TaskGroup) -> None: self._asked = False self._settled = anyio.Event() self._bluetooth: aio_rfcomm.Bluetooth | None = None - self._failure: str | None = None + self._failure: tuple[UnavailableReason, str] | None = None async def _get(self) -> aio_rfcomm.Bluetooth: """Wait for Bluetooth to be open, opening it on the first call. @@ -66,9 +71,10 @@ async def _get(self) -> aio_rfcomm.Bluetooth: await self._settled.wait() if self._bluetooth is None: - raise BluetoothUnavailableError( - aio_rfcomm.Availability.UNSUPPORTED, self._failure - ) + assert self._failure is not None + # A new one each time: raising the same object from every caller + # would pile each one's traceback onto it. + raise BluetoothUnavailableError(*self._failure) return self._bluetooth @@ -83,7 +89,14 @@ async def _hold(self) -> None: # Reported by each call from now on, as Bluetooth that cannot be # used, which is what RFCOMM already tells the user about. logger.warning("RFCOMM hubs will not be found: %s", exc) - self._failure = str(exc) + # With its reason, if it said why, so that what the user is told + # to do still fits. + self._failure = ( + exc.reason + if isinstance(exc, BluetoothUnavailableError) + else aio_rfcomm.Availability.UNSUPPORTED, + str(exc), + ) self._settled.set() async def list_known_devices( @@ -143,12 +156,12 @@ async def __anext__(self) -> aio_rfcomm.Availability: if self._watching is None: try: bluetooth = await self._get() - except BluetoothUnavailableError: + except BluetoothUnavailableError as exc: # Said once, as a watcher says the state now. It never changes # after, since nothing will open Bluetooth again. if not self._reported: self._reported = True - return aio_rfcomm.Availability.UNSUPPORTED + return exc.reason await anyio.sleep_forever() raise AssertionError("sleep_forever returned") from None diff --git a/packages/pb-service/src/pb_service/hub/rfcomm/transport.py b/packages/pb-service/src/pb_service/hub/rfcomm/transport.py index c6f58f9..b58222d 100644 --- a/packages/pb-service/src/pb_service/hub/rfcomm/transport.py +++ b/packages/pb-service/src/pb_service/hub/rfcomm/transport.py @@ -82,7 +82,7 @@ def __init__(self, bluetooth: aio_rfcomm.Bluetooth) -> None: """ self._bluetooth = bluetooth self.on_disconnect: Callable[[str], None] | None = None - self._unavailable: dict[str, str] = {} + self._unavailable: tuple[UnavailableDevice, ...] = () # The device as listed and the service it answered on, so that opening # it uses the one that found it rather than guessing. self._found: dict[str, tuple[aio_rfcomm.RfcommDeviceInfo, UUID]] = {} @@ -99,10 +99,7 @@ def unavailable(self) -> tuple[UnavailableDevice, ...]: paired set cannot be read at all in either case, so the one entry here stands for the adapter. """ - return tuple( - UnavailableDevice(address, reason, kind="adapter") - for address, reason in self._unavailable.items() - ) + return self._unavailable async def discover( self, scan_filter: ScanFilter @@ -124,11 +121,17 @@ async def discover( # whose error carries what the platform said, such as how to allow # this program. changes = self._bluetooth.watch_availability() + # Before waiting on anything: what the last scan found is no answer + # for this one, and opening Bluetooth the first time can take a while. + self._unavailable = () # The state now, which the first pass is about to read anyway. await anext(changes) while True: - self._unavailable.clear() + # Replaced at the end of the pass rather than emptied at its start, + # so that someone watching it mid-pass -- a client being told a + # permission dialog is open -- does not see it flicker. + problems: tuple[UnavailableDevice, ...] = () try: seen: set[str] = set() @@ -170,9 +173,21 @@ async def discover( except BluetoothUnavailableError as exc: # The library's own words, kept rather than paraphrased: they # are what tells "turn Bluetooth on" from "allow this program". - self._unavailable["bluetooth"] = str(exc) + # The reason as a word as well, which is what a client picks + # its advice by -- and on macOS what says a permission dialog + # is open, and that this pass is only waiting for an answer. + problems = ( + UnavailableDevice( + "bluetooth", + str(exc), + kind="adapter", + availability=exc.reason.name.lower(), + ), + ) logger.debug("cannot read the paired set: %s", exc) + self._unavailable = problems + with anyio.move_on_after(POLL_SECONDS): await anext(changes) diff --git a/packages/pb-service/src/pb_service/hub/transport.py b/packages/pb-service/src/pb_service/hub/transport.py index 5dd2241..b0eaf27 100644 --- a/packages/pb-service/src/pb_service/hub/transport.py +++ b/packages/pb-service/src/pb_service/hub/transport.py @@ -150,6 +150,21 @@ class UnavailableDevice: reach, while an adapter that cannot be used hides every hub behind it. """ + availability: str | None = None + """For an adapter, which kind of unusable it is, if the transport knows. + + One word a client can choose its advice by, since :attr:`reason` is the + system's prose and differs between platforms: ``switched_off``, + ``no_adapter``, ``not_allowed``, ``awaiting_permission``, + ``service_unavailable`` or ``unsupported``. ``awaiting_permission`` is the + one that ends by itself, when the user answers the system's question. + """ + + @property + def awaiting_permission(self) -> bool: + """Whether the system is asking the user to let this program in.""" + return self.availability == "awaiting_permission" + TRANSPORT_PREFERENCE: Final = ("usb", "ble", "rfcomm", "fake") """How transports rank when a hub is reachable more than one way, best first. diff --git a/packages/pb-service/src/pb_service/methods/hub.py b/packages/pb-service/src/pb_service/methods/hub.py index bfa90b2..3f28a57 100644 --- a/packages/pb-service/src/pb_service/methods/hub.py +++ b/packages/pb-service/src/pb_service/methods/hub.py @@ -6,13 +6,14 @@ from __future__ import annotations import logging +from collections.abc import Callable from contextlib import aclosing from typing import Any import anyio from pb_core.jsonrpc import ErrorCode, JsonRpcError -from pb_core.protocol import HUB_FOUND +from pb_core.protocol import HUB_ADAPTERS, HUB_FOUND from pb_service.errors import HUB_FAILURES, as_wire_error from pb_service.hub import protocol as p from pb_service.hub.connection import HubConnection, HubDisconnectedError @@ -25,6 +26,7 @@ from pb_service.hub.transport import ( DeviceInfo, ScanFilter, + UnavailableDevice, looks_like_id, preference_rank, ) @@ -83,13 +85,108 @@ def _found_nothing(manager: HubManager) -> JsonRpcError: if adapters: reasons = ", ".join(d.reason for d in adapters) + # The reason as a word too, for the client to choose its advice by: + # the sentence is the system's and differs between platforms. + known = [d.availability for d in adapters if d.availability] return JsonRpcError( - ErrorCode.BLUETOOTH_UNAVAILABLE, f"Bluetooth cannot be used: {reasons}" + ErrorCode.BLUETOOTH_UNAVAILABLE, + f"Bluetooth cannot be used: {reasons}", + {"availability": known[0]} if known else None, ) return JsonRpcError(ErrorCode.HUB_NOT_FOUND, "no hub found") +PERMISSION_SECONDS = 120.0 +"""How much longer a search may run while the system asks for permission. + +On macOS the first use of Bluetooth Classic asks the user whether the service +may use it, by the name in :data:`~pb_service.backend.PERMISSION_NAME`, and +nothing can be found until they answer. Their time reading the +dialog is not counted against the search's own limit -- a ten-second scan would +otherwise run out while they read -- but not without end either: a Mac where +nobody is there to answer leaves the question open for good. +""" + +FOLLOW_SECONDS = 0.1 +"""How often a search looks at what its transports cannot use. + +A poll, because transports keep that as state rather than announcing it, and +only for as long as one request is looking. +""" + + +def _adapters(manager: HubManager) -> tuple[UnavailableDevice, ...]: + return tuple(d for d in manager.transport.unavailable if d.kind == "adapter") + + +async def _follow_adapters( + manager: HubManager, + session: Session, + looking: anyio.CancelScope, + waiting: Callable[[], bool], + started: anyio.Event, +) -> None: + """Keep the caller told about unusable adapters while a search runs. + + Only adapters, not refused ports. A port is a hub that was found, which + the result or the error at the end says. An adapter is what decides + whether anything can be found at all, and one of its states -- a + permission dialog that is open -- is something the user has to act on + now, while the search is still running. + + Args: + manager: Whose transports are looking. + session: Who to tell, as :data:`~pb_core.protocol.HUB_ADAPTERS`. + looking: The search's deadline, held still while the system is asking + the user for permission, up to :data:`PERMISSION_SECONDS`. + waiting: Whether nothing has been found yet. Once something has, the + permission no longer holds the search open. + started: Set when this search's scan has the radio. A search queued + behind another must not report that one's adapters, or have its + deadline held by that one's permission dialog. + """ + await started.wait() + told: tuple[UnavailableDevice, ...] = () + held = 0.0 + then = anyio.current_time() + + while True: + # Before the first look, so that the transports have started this + # search: until then they still hold what the last one found, even + # once it has the radio. + await anyio.sleep(FOLLOW_SECONDS) + now = anyio.current_time() + adapters = _adapters(manager) + + if adapters != told: + told = adapters + + if session.peer is not None: + session.peer.notify( + HUB_ADAPTERS, + { + "unusable": [ + { + "address": d.address, + "reason": d.reason, + "availability": d.availability, + } + for d in adapters + ] + }, + ) + + asked = any(d.awaiting_permission for d in adapters) + + if asked and waiting() and held < PERMISSION_SECONDS: + # The clock stands still while the user is being asked. + looking.deadline += now - then + held += now - then + + then = now + + def _device_json(device: DeviceInfo) -> dict[str, Any]: return { "id": device.id, @@ -249,7 +346,10 @@ def _hub_json(connection: HubConnection) -> dict[str, Any]: "hub.scan", "Look for hubs that are advertising. Each advertisement is also sent to " "the caller as a hub.found notification while the scan runs, so a client " - "can show hubs as they appear instead of waiting for the whole scan.", + "can show hubs as they appear instead of waiting for the whole scan. A " + "Bluetooth adapter that cannot be used is reported as hub.adapters when " + "that changes; while the system asks the user for permission, the time " + "limit stands still.", params={ "timeout_s": ( "How long to look. Omit for 10 seconds; pass null to look until " @@ -286,24 +386,41 @@ async def hub_scan(session: Session, params: dict[str, Any]) -> dict[str, Any]: # A cancel scope rather than a timeout argument, which also means that # whatever was seen before the deadline is still returned. - with anyio.move_on_after(seconds): - async with aclosing(manager.discover(scan_filter)) as stream: - async for device in stream: - found[device.id] = device - - # Every advertisement, not only the first from each hub: the - # signal strength is what changes between them, and watching - # it move is how someone finds out which hub is which. The - # result still carries the whole list, so a caller that - # ignores the notifications loses nothing. - if session.peer is not None: - session.peer.notify(HUB_FOUND, _device_json(device)) + started = anyio.Event() + + async with anyio.create_task_group() as tg: + with anyio.move_on_after(seconds) as looking: + tg.start_soon( + _follow_adapters, + manager, + session, + looking, + lambda: not found, + started, + ) + + async with aclosing( + manager.discover(scan_filter, started=started) + ) as stream: + async for device in stream: + found[device.id] = device + + # Every advertisement, not only the first from each hub: + # the signal strength is what changes between them, and + # watching it move is how someone finds out which hub is + # which. The result still carries the whole list, so a + # caller that ignores the notifications loses nothing. + if session.peer is not None: + session.peer.notify(HUB_FOUND, _device_json(device)) + + tg.cancel_scope.cancel() # A scan that finds nothing is an ordinary answer and not an error, so it # returns an empty list -- unless a hub was there and would not open, or # Bluetooth could not be used, which scanning again will not fix and which - # the user can do something about. - if not found and manager.transport.unavailable: + # the user can do something about. Only if it scanned at all: one whose + # time ran out waiting behind another would be reporting that one's. + if not found and started.is_set() and manager.transport.unavailable: raise _found_nothing(manager) return {"devices": [_device_json(device) for device in found.values()]} @@ -311,7 +428,8 @@ async def hub_scan(session: Session, params: dict[str, Any]) -> dict[str, Any]: @method( "hub.connect", - "Connect to a hub and keep it connected.", + "Connect to a hub and keep it connected. While looking for it, an " + "unusable Bluetooth adapter is reported as hub.adapters, as by hub.scan.", params={ "id": "The hub's id, from hub.scan.", "name": "Connect to the first hub with this name instead.", @@ -354,30 +472,51 @@ async def hub_connect(session: Session, params: dict[str, Any]) -> dict[str, Any # it: put it inside and cancelling it unwinds into the generator's own # scopes, which anyio reports as exiting a cancel scope that is not the # current one -- measured, as a service that died whenever USB was in play. - with anyio.move_on_after( - float(params.get("timeout_s") or DEFAULT_SCAN_SECONDS) - ) as looking: - async with aclosing(manager.discover(scan_filter)) as stream: - async for candidate in stream: - if wanted_id and candidate.id != wanted_id: - continue - - if device is None or preference_rank( - candidate.transport - ) < preference_rank(device.transport): - device = candidate - - if preference_rank(device.transport) == 0: - # Nothing can beat it, so do not wait out the grace. - break - - # Something matched, so stop waiting for a hub and start - # waiting for a better route to the one already found. - looking.deadline = min( - looking.deadline, anyio.current_time() + PREFERENCE_SECONDS - ) + started = anyio.Event() + + async with anyio.create_task_group() as tg: + with anyio.move_on_after( + float(params.get("timeout_s") or DEFAULT_SCAN_SECONDS) + ) as looking: + tg.start_soon( + _follow_adapters, + manager, + session, + looking, + lambda: device is None, + started, + ) + + async with aclosing( + manager.discover(scan_filter, started=started) + ) as stream: + async for candidate in stream: + if wanted_id and candidate.id != wanted_id: + continue + + if device is None or preference_rank( + candidate.transport + ) < preference_rank(device.transport): + device = candidate + + if preference_rank(device.transport) == 0: + # Nothing can beat it, so do not wait out the grace. + break + + # Something matched, so stop waiting for a hub and start + # waiting for a better route to the one already found. + looking.deadline = min( + looking.deadline, anyio.current_time() + PREFERENCE_SECONDS + ) + + tg.cancel_scope.cancel() if device is None: + # As for a scan: what the transports say is another search's unless + # this one got the radio. + if not started.is_set(): + raise JsonRpcError(ErrorCode.HUB_NOT_FOUND, "no hub found") + raise _found_nothing(manager) try: diff --git a/packages/pb-service/tests/test_rfcomm_lazy.py b/packages/pb-service/tests/test_rfcomm_lazy.py index 021ec2a..e942b51 100644 --- a/packages/pb-service/tests/test_rfcomm_lazy.py +++ b/packages/pb-service/tests/test_rfcomm_lazy.py @@ -18,7 +18,7 @@ import pytest from aio_rfcomm.errors import BluetoothUnavailableError, RfcommError -from pb_service.hub.rfcomm.fake import FakePairedHub, radio +from pb_service.hub.rfcomm.fake import FakePairedHub, radio, switch from pb_service.hub.rfcomm.lazy import LazyBluetooth pytestmark = pytest.mark.anyio @@ -35,7 +35,9 @@ class _Opener: """Stands in for ``aio_rfcomm.open_bluetooth``, counting the opens.""" def __init__( - self, bluetooth: aio_rfcomm.Bluetooth | None, failure: str | None = None + self, + bluetooth: aio_rfcomm.Bluetooth | None, + failure: str | RfcommError | None = None, ) -> None: self.opens = 0 self.open = anyio.Event() @@ -46,6 +48,9 @@ def __init__( async def __call__(self) -> AsyncGenerator[aio_rfcomm.Bluetooth, None]: self.opens += 1 + if isinstance(self._failure, RfcommError): + raise self._failure + if self._failure is not None: raise RfcommError(self._failure) @@ -137,3 +142,55 @@ async def test_failing_to_open_is_bluetooth_that_cannot_be_used( assert waited.cancelled_caught tg.cancel_scope.cancel() + + +async def test_failing_to_open_for_a_reason_keeps_the_reason( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The reason is what the user is told to do about it, so a refused + # permission must not turn into "this system has no Bluetooth". + refused = BluetoothUnavailableError( + aio_rfcomm.Availability.NOT_ALLOWED, "Bluetooth was refused" + ) + monkeypatch.setattr(aio_rfcomm, "open_bluetooth", _Opener(None, refused)) + + async with anyio.create_task_group() as tg: + lazy = LazyBluetooth(tg) + + with pytest.raises(BluetoothUnavailableError, match="refused") as caught: + await lazy.list_known_devices() + + assert caught.value.reason == aio_rfcomm.Availability.NOT_ALLOWED + watcher = lazy.watch_availability() + assert await anext(watcher) == aio_rfcomm.Availability.NOT_ALLOWED + tg.cancel_scope.cancel() + + +async def test_an_open_permission_dialog_is_waited_out( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # aio-rfcomm 1.1 opens without waiting for the user's answer on macOS: + # calls are refused until it comes, and the watcher says when it has. + bluetooth = radio( + FakePairedHub(address=EV3), + unavailable="answer the permission dialog", + reason=aio_rfcomm.Availability.AWAITING_PERMISSION, + ) + monkeypatch.setattr(aio_rfcomm, "open_bluetooth", _Opener(bluetooth)) + + async with anyio.create_task_group() as tg: + lazy = LazyBluetooth(tg) + + with pytest.raises(BluetoothUnavailableError) as caught: + await lazy.list_known_devices() + + assert caught.value.reason == aio_rfcomm.Availability.AWAITING_PERMISSION + watcher = lazy.watch_availability() + + with anyio.fail_after(5): + assert await anext(watcher) == aio_rfcomm.Availability.AWAITING_PERMISSION + switch(bluetooth, None) + assert await anext(watcher) == aio_rfcomm.Availability.USABLE + + assert [d.address for d in await lazy.list_known_devices()] == [EV3] + tg.cancel_scope.cancel() diff --git a/packages/pb-service/tests/test_rfcomm_transport.py b/packages/pb-service/tests/test_rfcomm_transport.py index 9638479..197db0e 100644 --- a/packages/pb-service/tests/test_rfcomm_transport.py +++ b/packages/pb-service/tests/test_rfcomm_transport.py @@ -14,8 +14,10 @@ from collections.abc import Callable from contextlib import aclosing +import aio_rfcomm import anyio import pytest +from aio_rfcomm import UnavailableReason from pb_service.hub import protocol as p from pb_service.hub.rfcomm.fake import FakePairedHub, radio, switch @@ -190,6 +192,61 @@ async def until(condition: Callable[[], object]) -> None: tg.cancel_scope.cancel() +@pytest.mark.parametrize( + ("reason", "word"), + [ + (aio_rfcomm.Availability.SWITCHED_OFF, "switched_off"), + (aio_rfcomm.Availability.NOT_ALLOWED, "not_allowed"), + (aio_rfcomm.Availability.AWAITING_PERMISSION, "awaiting_permission"), + (aio_rfcomm.Availability.UNSUPPORTED, "unsupported"), + ], +) +async def test_why_bluetooth_cannot_be_used_is_said_in_a_word( + reason: UnavailableReason, word: str +) -> None: + # The system's prose differs between platforms, so the client picks what + # to suggest by this instead. + transport = RfcommHubTransport(radio(unavailable="no", reason=reason)) + + assert await found(transport) == [] + assert [u.availability for u in transport.unavailable] == [word] + + +async def test_a_scan_carries_on_once_the_permission_dialog_is_answered() -> None: + # aio-rfcomm 1.1 on macOS: Bluetooth opens before the user has answered, + # and says so. The scan reports that until the answer comes, and then + # lists the hub without being asked again. + bluetooth = radio( + FakePairedHub(address=EV3), + unavailable="answer the permission dialog", + reason=aio_rfcomm.Availability.AWAITING_PERMISSION, + ) + transport = RfcommHubTransport(bluetooth) + seen: list[DeviceInfo] = [] + + async def scan() -> None: + async with aclosing(transport.discover(ScanFilter())) as hubs: + async for hub in hubs: + seen.append(hub) + + async with anyio.create_task_group() as tg: + tg.start_soon(scan) + + with anyio.fail_after(POLL_SECONDS / 4): + while not transport.unavailable: # noqa: ASYNC110 + await anyio.sleep(0.005) + + assert transport.unavailable[0].awaiting_permission + switch(bluetooth, None) + + with anyio.fail_after(POLL_SECONDS / 4): + while not seen: # noqa: ASYNC110 + await anyio.sleep(0.005) + + assert [hub.address for hub in seen] == [EV3] + tg.cancel_scope.cancel() + + async def test_a_hub_that_is_switched_off_refuses_rather_than_crashing() -> None: # The whole reason this file exists. A paired hub is listed whether or not # it is switched on, so this is the ordinary way connecting fails, and it diff --git a/tests/e2e/test_commands.py b/tests/e2e/test_commands.py index ea85068..5b70a9a 100644 --- a/tests/e2e/test_commands.py +++ b/tests/e2e/test_commands.py @@ -21,15 +21,16 @@ from collections.abc import AsyncGenerator from pathlib import Path +import aio_rfcomm import anyio import pytest -import pb_service.methods.hub # pyright: ignore[reportUnusedImport] +import pb_service.methods.hub import pb_service.methods.program # pyright: ignore[reportUnusedImport] # Imported to register the RPC methods. import pb_service.methods.service # pyright: ignore[reportUnusedImport] -import pb_service.methods.stdio # noqa: F401 # pyright: ignore[reportUnusedImport] +import pb_service.methods.stdio # pyright: ignore[reportUnusedImport] from pb_cli import errors from pb_cli.commands import hub as hub_commands from pb_cli.commands import program as program_commands @@ -38,6 +39,8 @@ from pb_service import service as service_module from pb_service.hub import protocol as p from pb_service.hub.fake import FakeHub, FakeHubTransport +from pb_service.hub.rfcomm.fake import FakePairedHub, radio, switch +from pb_service.hub.rfcomm.transport import RfcommHubTransport pytestmark = pytest.mark.anyio @@ -65,18 +68,16 @@ def endpoint(short_tmp_path: Path) -> Endpoint: @pytest.fixture -async def service( +def private( endpoint: Endpoint, - hub: FakeHub, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, short_tmp_path: Path, -) -> AsyncGenerator[FakeHub, None]: - """Run a service the commands will find, without touching the real one. +) -> Endpoint: + """Point the commands at a service of the test's own, not the real one. - In this very event loop. The commands are ``async def`` and cyclopts runs - them, so nothing here starts a loop of its own and there is no thread or - blocking portal between the test and what it is testing. + Returns: + Where that service should listen. """ # Short, not tmp_path: the service's socket goes in here, and on macOS a # path under tmp_path does not fit in sun_path. See short_tmp_path. @@ -94,9 +95,19 @@ async def service( # console script does, so this is what points them at the private service # rather than at whatever is running on the developer's machine. monkeypatch.setenv("PB_ENDPOINT", str(endpoint)) + return endpoint + +@pytest.fixture +async def service(private: Endpoint, hub: FakeHub) -> AsyncGenerator[FakeHub, None]: + """Run a service the commands will find, without touching the real one. + + In this very event loop. The commands are ``async def`` and cyclopts runs + them, so nothing here starts a loop of its own and there is no thread or + blocking portal between the test and what it is testing. + """ async with service_module.run_service( - endpoint, idle_timeout=0, transport=FakeHubTransport(hub) + private, idle_timeout=0, transport=FakeHubTransport(hub) ): yield hub @@ -248,6 +259,98 @@ async def test_scan_says_so_when_it_hears_nothing( assert captured.out == "" +@pytest.fixture +async def asking(private: Endpoint) -> AsyncGenerator[aio_rfcomm.Bluetooth, None]: + """Run a service whose Bluetooth is waiting for the user's permission. + + What the first use of Bluetooth Classic on a Mac looks like with + aio-rfcomm 1.1: the dialog is open and nothing can be listed until it is + answered. + + Yields: + The radio, to answer with :func:`~pb_service.hub.rfcomm.fake.switch`. + """ + bluetooth = radio( + FakePairedHub(name="Ostrich (MINDSTORMS EV3)"), + unavailable="macOS is asking whether this program may use Bluetooth", + reason=aio_rfcomm.Availability.AWAITING_PERMISSION, + ) + + async with service_module.run_service( + private, idle_timeout=0, transport=RfcommHubTransport(bluetooth) + ): + yield bluetooth + + +async def _answer_later(bluetooth: aio_rfcomm.Bluetooth) -> None: + # Later than the command would otherwise have given up. + await anyio.sleep(1) + switch(bluetooth, None) + + +async def test_scan_says_to_answer_the_dialog_and_then_lists_the_hub( + asking: aio_rfcomm.Bluetooth, capsys: pytest.CaptureFixture[str] +) -> None: + # Not "is the hub paired?", which is what an empty listing used to say: + # it is, and nothing about pairing will help. + async with anyio.create_task_group() as tg: + tg.start_soon(_answer_later, asking) + assert ( + await hub_commands.scan_cmd(timeout=0.3, transport="rfcomm") + == errors.EXIT_OK + ) + + captured = capsys.readouterr() + # As one line, wherever rich wrapped it for the terminal width. + said = " ".join(captured.err.split()) + assert "Answer the dialog" in said + assert errors.PERMISSION_NAME in said, "not the name the dialog shows" + assert captured.out.splitlines() == ["Ostrich"] + + +async def test_connect_says_to_answer_the_dialog_and_then_connects( + asking: aio_rfcomm.Bluetooth, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(pb_service.methods.hub, "DEFAULT_SCAN_SECONDS", 0.3) + + async with anyio.create_task_group() as tg: + tg.start_soon(_answer_later, asking) + assert ( + await hub_commands.connect_cmd(hub="Ostrich", transport="rfcomm") + == errors.EXIT_OK + ) + + captured = capsys.readouterr() + # As one line, wherever rich wrapped it for the terminal width. + said = " ".join(captured.err.split()) + assert "Answer the dialog" in said + assert "Connected to" in said + assert "pair" not in said + + +async def test_a_dialog_left_unanswered_is_what_the_failure_names( + asking: aio_rfcomm.Bluetooth, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(pb_service.methods.hub, "DEFAULT_SCAN_SECONDS", 0.2) + monkeypatch.setattr(pb_service.methods.hub, "PERMISSION_SECONDS", 0.2) + + assert ( + await hub_commands.connect_cmd(hub="Ostrich", transport="rfcomm") + == errors.EXIT_ERROR + ) + + captured = capsys.readouterr() + # As one line, wherever rich wrapped it for the terminal width. + said = " ".join(captured.err.split()) + assert "may use Bluetooth, then try again" in said + assert "Switch Bluetooth on" not in said + assert "pair" not in said + + async def test_disconnect_then_nothing_is_connected( service: FakeHub, capsys: pytest.CaptureFixture[str] ) -> None: diff --git a/tests/e2e/test_hub_lifecycle.py b/tests/e2e/test_hub_lifecycle.py index 5925ace..22411bb 100644 --- a/tests/e2e/test_hub_lifecycle.py +++ b/tests/e2e/test_hub_lifecycle.py @@ -15,6 +15,7 @@ from collections.abc import AsyncGenerator, Iterator from pathlib import Path +import aio_rfcomm import anyio import pytest @@ -29,7 +30,10 @@ from pb_core.jsonrpc import ErrorCode, JsonRpcError from pb_service import service as service_module from pb_service.hub import protocol as p +from pb_service.hub.combined import CombinedTransport from pb_service.hub.fake import FakeHub, FakeHubTransport +from pb_service.hub.rfcomm.fake import FakePairedHub, radio, switch +from pb_service.hub.rfcomm.transport import RfcommHubTransport from pb_service.hub.transport import UnavailableDevice pytestmark = pytest.mark.anyio @@ -37,6 +41,7 @@ FIRST = "Pybricks Hub" SECOND = "Spare Hub" PORT = "/dev/ttyACM0" +EV3 = "00:16:53:4A:75:15" @pytest.fixture(scope="session") @@ -180,7 +185,12 @@ async def bluetooth_off_at(endpoint: Endpoint) -> AsyncGenerator[Endpoint, None] transport = FakeHubTransport() transport.hubs = [] transport.unavailable = ( - UnavailableDevice("bluetooth", "Bluetooth is switched off", kind="adapter"), + UnavailableDevice( + "bluetooth", + "Bluetooth is switched off", + kind="adapter", + availability="switched_off", + ), ) async with service_module.run_service( @@ -200,6 +210,168 @@ async def test_bluetooth_that_cannot_be_used_is_not_reported_as_a_port( assert caught.value.code == ErrorCode.BLUETOOTH_UNAVAILABLE assert "switched off" in str(caught.value), "the system's words are lost" + # And in a word, which is what the client chooses its advice by. + assert caught.value.data == {"availability": "switched_off"} + + +@pytest.fixture +async def asking_at( + endpoint: Endpoint, +) -> AsyncGenerator[tuple[Endpoint, aio_rfcomm.Bluetooth], None]: + """A service whose only hub is behind a permission dialog nobody has answered. + + What aio-rfcomm 1.1 gives on a Mac the first time pb-service uses + Bluetooth Classic: it opens straight away and says it is waiting for the + user. The real RFCOMM transport, on the simulated radio, so that what the + user does -- answering -- is one call. + + Args: + endpoint: Where the service should listen. + + Yields: + That endpoint, with the service running, and the radio to answer with + :func:`~pb_service.hub.rfcomm.fake.switch`. + """ + bluetooth = radio( + FakePairedHub(address=EV3), + unavailable="macOS is asking whether this program may use Bluetooth", + reason=aio_rfcomm.Availability.AWAITING_PERMISSION, + ) + + async with service_module.run_service( + endpoint, idle_timeout=0, transport=RfcommHubTransport(bluetooth) + ): + yield endpoint, bluetooth + + +async def test_a_scan_waits_while_the_user_is_asked_and_then_carries_on( + asking_at: tuple[Endpoint, aio_rfcomm.Bluetooth], +) -> None: + # Told at once, so that they look for the dialog; given the time to read + # it, rather than a scan that runs out meanwhile; and the hub listed once + # they have answered, without being asked to scan again. + endpoint, bluetooth = asking_at + found: list[pb_client.DeviceInfo] = [] + + async with ( + pb_client.connect(endpoint, autostart=False) as service, + service.watching_adapters() as changes, + anyio.create_task_group() as tg, + ): + + async def scan() -> None: + found.extend(await service.scan(seconds=0.3)) + + tg.start_soon(scan) + + with anyio.fail_after(5): + (asked,) = await anext(changes) + + assert asked.awaiting_permission + assert "asking" in asked.reason, "the system's words are lost" + + # Well past the scan's own limit. + await anyio.sleep(1) + switch(bluetooth, None) + + with anyio.fail_after(5): + assert await anext(changes) == () + + assert [device.address for device in found] == [EV3] + + +async def test_connecting_carries_on_once_the_user_answers( + asking_at: tuple[Endpoint, aio_rfcomm.Bluetooth], + monkeypatch: pytest.MonkeyPatch, +) -> None: + # pb connect is what somebody types the first time, so it is where the + # dialog first appears. + monkeypatch.setattr(pb_service.methods.hub, "DEFAULT_SCAN_SECONDS", 0.3) + endpoint, bluetooth = asking_at + connected: list[pb_client.HubInfo] = [] + + async with ( + pb_client.connect(endpoint, autostart=False) as service, + service.watching_adapters() as changes, + anyio.create_task_group() as tg, + ): + + async def connect() -> None: + connected.append(await service.connect_hub(transport="rfcomm")) + + tg.start_soon(connect) + + with anyio.fail_after(5): + (asked,) = await anext(changes) + + assert asked.awaiting_permission + await anyio.sleep(1) + switch(bluetooth, None) + + assert [hub.address for hub in connected] == [EV3] + + +async def test_a_search_queued_behind_one_held_by_the_dialog_is_not_held_too( + endpoint: Endpoint, monkeypatch: pytest.MonkeyPatch +) -> None: + # One scan at a time, so a search waits for the radio behind any other. + # Until it has the radio, what the transports report is the other + # search's: a USB user must not be told about a Bluetooth dialog, or kept + # waiting while somebody else reads it. + monkeypatch.setattr(pb_service.methods.hub, "PERMISSION_SECONDS", 1.0) + bluetooth = radio( + FakePairedHub(address=EV3), + unavailable="macOS is asking whether this program may use Bluetooth", + reason=aio_rfcomm.Availability.AWAITING_PERMISSION, + ) + transport = CombinedTransport(RfcommHubTransport(bluetooth), FakeHubTransport()) + told: list[tuple[pb_client.UnusableAdapter, ...]] = [] + took = float("inf") + + async with ( + service_module.run_service(endpoint, idle_timeout=0, transport=transport), + pb_client.connect(endpoint, autostart=False) as asked, + pb_client.connect(endpoint, autostart=False) as queued, + anyio.create_task_group() as tg, + ): + + async def held() -> None: + with pytest.raises(JsonRpcError): + await asked.scan(seconds=0.2, transport="rfcomm") + + async def watch() -> None: + async with queued.watching_adapters() as changes: + async for change in changes: + told.append(change) + + tg.start_soon(held) + tg.start_soon(watch) + # Asked second, so that it queues behind the scan the dialog holds. + await anyio.sleep(0.05) + began = anyio.current_time() + await queued.scan(seconds=0.3, transport="fake") + took = anyio.current_time() - began + tg.cancel_scope.cancel() + + assert told == [], "told about another search's Bluetooth dialog" + assert took < 0.9, "held by another search's Bluetooth dialog" + + +async def test_a_question_nobody_answers_is_not_waited_on_for_ever( + asking_at: tuple[Endpoint, aio_rfcomm.Bluetooth], + monkeypatch: pytest.MonkeyPatch, +) -> None: + # A Mac with nobody at it leaves the dialog open for good. The search + # gives up in the end, and says what it was waiting for. + monkeypatch.setattr(pb_service.methods.hub, "PERMISSION_SECONDS", 0.3) + endpoint, _ = asking_at + + async with pb_client.connect(endpoint, autostart=False) as service: + with anyio.fail_after(5), pytest.raises(JsonRpcError) as caught: + await service.scan(seconds=0.2) + + assert caught.value.code == ErrorCode.BLUETOOTH_UNAVAILABLE + assert caught.value.data == {"availability": "awaiting_permission"} @pytest.fixture diff --git a/uv.lock b/uv.lock index e7c79b3..4c8d7c3 100644 --- a/uv.lock +++ b/uv.lock @@ -44,7 +44,7 @@ typecheck = [{ name = "pyright", specifier = ">=1.1.400" }] [[package]] name = "aio-rfcomm" -version = "1.0.0" +version = "1.1.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "dbus-fast", marker = "sys_platform == 'linux'" }, @@ -58,9 +58,9 @@ dependencies = [ { name = "winrt-windows-foundation-collections", marker = "sys_platform == 'win32'" }, { name = "winrt-windows-storage-streams", marker = "sys_platform == 'win32'" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ff/f9/d0123d34731da8b544376ff29bef60842ef0c587223b240ff11fc276ad68/aio_rfcomm-1.0.0.tar.gz", hash = "sha256:3d4458519ecbe437967623b6ad72815294bbff7213a6d18152087cb98f233651", size = 108909, upload-time = "2026-10-18T04:38:40.416Z" } +sdist = { url = "https://files.pythonhosted.org/packages/ae/80/7f0e6deed493c5402bf0b6d23c3107008cf74c621bdb7469dc5becf0e3f0/aio_rfcomm-1.1.0.tar.gz", hash = "sha256:8f3bff8e508c6d7b7e05381fbc4a6169e69e2c9a3df5e7d59f4ee641fbd515fd", size = 113091, upload-time = "2026-10-18T22:17:00.494Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/6a/28/85ff0a652fa0609f894c9afbbe17f02a168e40e9e403ac574b3ab3bcd565/aio_rfcomm-1.0.0-py3-none-any.whl", hash = "sha256:88e6a12f3023cc6e46b74a306d0b501185f6eefc6faa53844a2d62acac5f7f95", size = 138379, upload-time = "2026-10-18T04:38:38.862Z" }, + { url = "https://files.pythonhosted.org/packages/a7/4f/6a8c420806612345afdfda4f36b05cf3f80d6e0211590a0caef25850e77d/aio_rfcomm-1.1.0-py3-none-any.whl", hash = "sha256:66d4fcc3132382cf83a89b6822e65e765ce162d71a3c2d79ebd795652a3b21b7", size = 142451, upload-time = "2026-10-18T22:16:59.016Z" }, ] [[package]] @@ -971,7 +971,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "aio-rfcomm", specifier = ">=1.0" }, + { name = "aio-rfcomm", specifier = ">=1.1" }, { name = "anyio", specifier = ">=4.14" }, { name = "bleak", specifier = ">=1.1.0" }, { name = "packaging", specifier = ">=22" },