From 55e8eafa70eb3a93f4ef6acc30c408c9b0c15101 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 18 Oct 2026 17:19:44 -0500 Subject: [PATCH 1/4] pb-service: require aio-rfcomm 1.1 1.1 stops waiting for the answer to macOS's Bluetooth permission dialog and reports AWAITING_PERMISSION meanwhile, says NOT_ALLOWED rather than UNSUPPORTED for a refused permission, and names the program in both messages. What follows depends on the first: until now a scan started while the dialog was open sat waiting and then told the user to pair the hub. --- packages/pb-service/pyproject.toml | 2 +- uv.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) 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/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" }, From 8bdcf2613c7fed8e4cecfbc04e6978aa4d16bec4 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 18 Oct 2026 17:21:30 -0500 Subject: [PATCH 2/4] pb-service: say a Bluetooth permission dialog is open, and wait for its answer On macOS the first use of Bluetooth Classic asks the user whether the service may use it, and with aio-rfcomm 1.1 nothing can be listed until they answer. A scan or connect started meanwhile ran out after ten seconds and told the user to pair a hub that was paired. Transports now name why an adapter cannot be used in a word, one per aio-rfcomm Availability, alongside the system's sentence. While a search runs, the service reports the adapters it cannot use as hub.adapters, and while permission is being asked and nothing has been found it holds the search's time limit still, for up to two minutes. Bluetooth errors carry the word too. pb-client follows those with watching_adapters(). pb scan and pb connect say at once that macOS is asking, under the name the dialog shows, and carry on when the dialog is answered. When Bluetooth cannot be used, the suggestion now fits the reason rather than covering every reason at once. A failed open keeps its reason instead of always reading unsupported. The RFCOMM transport replaces what it reports at the end of each pass, so a client watching does not see it flicker, and starts each scan without the last one's. --- packages/pb-cli/src/pb_cli/commands/hub.py | 143 +++++++++++--- packages/pb-cli/src/pb_cli/errors.py | 96 ++++++++- packages/pb-cli/tests/test_errors.py | 51 +++++ packages/pb-client/src/pb_client/__init__.py | 2 + packages/pb-client/src/pb_client/models.py | 51 ++++- packages/pb-client/src/pb_client/service.py | 50 +++++ packages/pb-core/src/pb_core/protocol.py | 18 ++ packages/pb-service/src/pb_service/backend.py | 18 +- .../src/pb_service/hub/rfcomm/fake.py | 40 ++-- .../src/pb_service/hub/rfcomm/lazy.py | 38 ++-- .../src/pb_service/hub/rfcomm/transport.py | 29 ++- .../src/pb_service/hub/transport.py | 15 ++ .../pb-service/src/pb_service/methods/hub.py | 184 ++++++++++++++---- packages/pb-service/tests/test_rfcomm_lazy.py | 61 +++++- .../pb-service/tests/test_rfcomm_transport.py | 57 ++++++ tests/e2e/test_commands.py | 123 +++++++++++- tests/e2e/test_hub_lifecycle.py | 127 +++++++++++- 17 files changed, 987 insertions(+), 116 deletions(-) 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..3a33298 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" +"""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..e93fc3e 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,56 @@ 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 say "Pybricks", not pb or Python. + for availability in ("awaiting_permission", "not_allowed"): + assert "Pybricks" in errors.bluetooth_advice(availability, "darwin") + + # 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..82d3e28 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,48 @@ 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 "Pybricks" may use Bluetooth. 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..d161f5e 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 "Pybricks" 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/src/pb_service/backend.py b/packages/pb-service/src/pb_service/backend.py index 6a7dc39..d60b026 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,15 @@ """ +PERMISSION_NAME = "Pybricks" +"""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. +""" + + def _usb() -> HubTransport: from pb_service.hub.usb import UsbHubTransport @@ -145,7 +159,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/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..244be55 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,15 @@ """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 +"Pybricks" may use Bluetooth. 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 +24,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 +52,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 +70,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 +88,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 +155,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..13767e0 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,101 @@ 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 "Pybricks" +may use it, 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], +) -> 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. + """ + 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. + 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 +339,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,18 +379,25 @@ 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)) + 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 + ) + + 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)) + + 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 @@ -311,7 +411,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,28 +455,35 @@ 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 - ) + 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 + ) + + 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 + ) + + tg.cancel_scope.cancel() if device is None: raise _found_nothing(manager) 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..b864ce4 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 "Pybricks" 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..a3c6823 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 @@ -30,6 +31,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 from pb_service.hub.transport import UnavailableDevice pytestmark = pytest.mark.anyio @@ -37,6 +40,7 @@ FIRST = "Pybricks Hub" SECOND = "Spare Hub" PORT = "/dev/ttyACM0" +EV3 = "00:16:53:4A:75:15" @pytest.fixture(scope="session") @@ -180,7 +184,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 +209,122 @@ 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_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 From 4d247749d515cf840ba3996dc6285dba0eb6f7e7 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 18 Oct 2026 17:22:58 -0500 Subject: [PATCH 3/4] pb-service: ask for Bluetooth as Pybricks RFCOMM Service macOS shows this name in the permission dialog and under Privacy & Security. Plain "Pybricks" reads there as the Pybricks app or Pybricks Code, and says nothing about what is asking or why. What asks is the part of the service that reaches EV3 and NXT hubs over Bluetooth Classic. macOS remembers the answer against the name, so everyone is asked once more, and the old "Pybricks" entry stays in Privacy & Security until it is removed there. --- packages/pb-cli/src/pb_cli/errors.py | 2 +- packages/pb-cli/tests/test_errors.py | 5 +++-- packages/pb-client/src/pb_client/models.py | 5 +++-- packages/pb-client/src/pb_client/service.py | 2 +- packages/pb-service/src/pb_service/backend.py | 8 +++++++- packages/pb-service/src/pb_service/hub/rfcomm/lazy.py | 9 +++++---- packages/pb-service/src/pb_service/methods/hub.py | 5 +++-- tests/e2e/test_commands.py | 2 +- 8 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/pb-cli/src/pb_cli/errors.py b/packages/pb-cli/src/pb_cli/errors.py index 3a33298..e2d6701 100644 --- a/packages/pb-cli/src/pb_cli/errors.py +++ b/packages/pb-cli/src/pb_cli/errors.py @@ -156,7 +156,7 @@ def serial_port_advice(platform: str = sys.platform) -> str: ) -PERMISSION_NAME = "Pybricks" +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 diff --git a/packages/pb-cli/tests/test_errors.py b/packages/pb-cli/tests/test_errors.py index e93fc3e..b1490f3 100644 --- a/packages/pb-cli/tests/test_errors.py +++ b/packages/pb-cli/tests/test_errors.py @@ -482,9 +482,10 @@ def test_the_name_is_the_one_the_service_asks_under() -> None: def test_the_permission_advice_names_what_macos_shows() -> None: - # The dialog and System Settings say "Pybricks", not pb or Python. + # The dialog and System Settings name the service, not pb or Python. for availability in ("awaiting_permission", "not_allowed"): - assert "Pybricks" in errors.bluetooth_advice(availability, "darwin") + 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") diff --git a/packages/pb-client/src/pb_client/models.py b/packages/pb-client/src/pb_client/models.py index 82d3e28..0245922 100644 --- a/packages/pb-client/src/pb_client/models.py +++ b/packages/pb-client/src/pb_client/models.py @@ -116,8 +116,9 @@ class UnusableAdapter: def awaiting_permission(self) -> bool: """Whether the system is asking the user to let the service in. - On macOS, a dialog asking whether "Pybricks" may use Bluetooth. It - ends by itself when the user answers, and a scan carries on then. + 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" diff --git a/packages/pb-client/src/pb_client/service.py b/packages/pb-client/src/pb_client/service.py index d161f5e..4c01008 100644 --- a/packages/pb-client/src/pb_client/service.py +++ b/packages/pb-client/src/pb_client/service.py @@ -322,7 +322,7 @@ async def watching_adapters( 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 "Pybricks" may use Bluetooth, and nothing is found until they + 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 diff --git a/packages/pb-service/src/pb_service/backend.py b/packages/pb-service/src/pb_service/backend.py index d60b026..fe25f11 100644 --- a/packages/pb-service/src/pb_service/backend.py +++ b/packages/pb-service/src/pb_service/backend.py @@ -40,12 +40,18 @@ """ -PERMISSION_NAME = "Pybricks" +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. """ 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 244be55..f4ac438 100644 --- a/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py +++ b/packages/pb-service/src/pb_service/hub/rfcomm/lazy.py @@ -5,10 +5,11 @@ 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 -"Pybricks" may use Bluetooth. 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 +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. diff --git a/packages/pb-service/src/pb_service/methods/hub.py b/packages/pb-service/src/pb_service/methods/hub.py index 13767e0..31de745 100644 --- a/packages/pb-service/src/pb_service/methods/hub.py +++ b/packages/pb-service/src/pb_service/methods/hub.py @@ -100,8 +100,9 @@ def _found_nothing(manager: HubManager) -> JsonRpcError: 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 "Pybricks" -may use it, and nothing can be found until they answer. Their time reading the +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. diff --git a/tests/e2e/test_commands.py b/tests/e2e/test_commands.py index b864ce4..5b70a9a 100644 --- a/tests/e2e/test_commands.py +++ b/tests/e2e/test_commands.py @@ -304,7 +304,7 @@ async def test_scan_says_to_answer_the_dialog_and_then_lists_the_hub( # As one line, wherever rich wrapped it for the terminal width. said = " ".join(captured.err.split()) assert "Answer the dialog" in said - assert "Pybricks" in said, "not the name the dialog shows" + assert errors.PERMISSION_NAME in said, "not the name the dialog shows" assert captured.out.splitlines() == ["Ostrich"] From c89e04da75e7dce02376c05e975198a539cbc4ec Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 18 Oct 2026 17:37:57 -0500 Subject: [PATCH 4/4] pb-service: a search waiting for the radio reports only its own adapters Only one scan runs at a time, so a search arriving during another waits for the radio. Until it has it, what the transports report is the other search's. A USB-only search queued behind an RFCOMM one held by the permission dialog was told about that dialog, had its own time limit held by it, and, if its time ran out in the queue, failed with that search's Bluetooth error. HubManager.discover() now says when a scan has the radio. Until then the search follows nothing, holds nothing, and explains an empty result only as nothing found. --- .../pb-service/src/pb_service/hub/manager.py | 11 ++++- .../pb-service/src/pb_service/methods/hub.py | 44 ++++++++++++++--- tests/e2e/test_hub_lifecycle.py | 47 +++++++++++++++++++ 3 files changed, 94 insertions(+), 8 deletions(-) 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/methods/hub.py b/packages/pb-service/src/pb_service/methods/hub.py index 31de745..3f28a57 100644 --- a/packages/pb-service/src/pb_service/methods/hub.py +++ b/packages/pb-service/src/pb_service/methods/hub.py @@ -125,6 +125,7 @@ async def _follow_adapters( session: Session, looking: anyio.CancelScope, waiting: Callable[[], bool], + started: anyio.Event, ) -> None: """Keep the caller told about unusable adapters while a search runs. @@ -141,14 +142,19 @@ async def _follow_adapters( 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. + # 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) @@ -380,13 +386,22 @@ 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. + 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 + _follow_adapters, + manager, + session, + looking, + lambda: not found, + started, ) - async with aclosing(manager.discover(scan_filter)) as stream: + async with aclosing( + manager.discover(scan_filter, started=started) + ) as stream: async for device in stream: found[device.id] = device @@ -403,8 +418,9 @@ async def hub_scan(session: Session, params: dict[str, Any]) -> dict[str, Any]: # 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()]} @@ -456,15 +472,24 @@ 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. + 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 + _follow_adapters, + manager, + session, + looking, + lambda: device is None, + started, ) - async with aclosing(manager.discover(scan_filter)) as stream: + 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 @@ -487,6 +512,11 @@ async def hub_connect(session: Session, params: dict[str, Any]) -> dict[str, Any 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/tests/e2e/test_hub_lifecycle.py b/tests/e2e/test_hub_lifecycle.py index a3c6823..22411bb 100644 --- a/tests/e2e/test_hub_lifecycle.py +++ b/tests/e2e/test_hub_lifecycle.py @@ -30,6 +30,7 @@ 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 @@ -310,6 +311,52 @@ async def connect() -> 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,