diff --git a/starpilot/system/bluetooth/daemon.py b/starpilot/system/bluetooth/daemon.py index c1f62ad56..98923dc10 100644 --- a/starpilot/system/bluetooth/daemon.py +++ b/starpilot/system/bluetooth/daemon.py @@ -73,32 +73,34 @@ class BluetoothController: return self.params.get_bool("IsOffroad") def status(self) -> dict[str, Any]: - result = { - "available": self._radio.available, - "enabled": self.params.get_bool("BluetoothEnabled"), - "powered": False, - "discovering": False, - "offroad": self._offroad(), - "selected_audio": self.params.get("BluetoothAudioAddress", encoding="utf-8") or "", - "devices": [], - "prompt": None, - "error": self._pairing_error, - "pairing_address": self._pairing_address, - } - if not result["enabled"]: + # Status lazily initializes the radio, so serialize it with power changes. + with self._lock: + result = { + "available": self._radio.available, + "enabled": self.params.get_bool("BluetoothEnabled"), + "powered": False, + "discovering": False, + "offroad": self._offroad(), + "selected_audio": self.params.get("BluetoothAudioAddress", encoding="utf-8") or "", + "devices": [], + "prompt": None, + "error": self._pairing_error, + "pairing_address": self._pairing_address, + } + if not result["enabled"]: + return result + try: + result.update(self._client().status()) + result["available"] = True + prompt = result.get("prompt") + if prompt is not None and self._pairing_address: + prompt["address"] = self._pairing_address + device = next((item for item in result["devices"] if item["address"].upper() == self._pairing_address.upper()), None) + prompt["name"] = device["name"] if device else self._pairing_address + except Exception as error: + result["error"] = str(error) + self._reset_client() return result - try: - result.update(self._client().status()) - result["available"] = True - prompt = result.get("prompt") - if prompt is not None and self._pairing_address: - prompt["address"] = self._pairing_address - device = next((item for item in result["devices"] if item["address"].upper() == self._pairing_address.upper()), None) - prompt["name"] = device["name"] if device else self._pairing_address - except Exception as error: - result["error"] = str(error) - self._reset_client() - return result def _require_offroad(self, command: str) -> None: if command in OFFROAD_COMMANDS and not self._offroad(): @@ -143,30 +145,30 @@ class BluetoothController: address = str(request.get("address", "")) if command == "set_power": enabled = bool(request.get("enabled", False)) - if enabled: - try: - self.params.put_bool("BluetoothEnabled", True) - self._client() - except Exception: - self.params.put_bool("BluetoothEnabled", False) - self._reset_client() + with self._lock: + if enabled: try: - self._radio.stop() + self.params.put_bool("BluetoothEnabled", True) + self._client() except Exception: - pass - raise - else: - try: - with self._lock: + self.params.put_bool("BluetoothEnabled", False) + self._reset_client() + try: + self._radio.stop() + except Exception: + pass + raise + else: + try: client = self._bluez - if client is not None: - client.set_powered(False) - finally: - self._reset_client() - self._radio.stop() - self.params.remove("BluetoothAudioAddress") - self.params.put_bool("BluetoothEnabled", False) - self._scan_deadline = 0.0 + if client is not None: + client.set_powered(False) + finally: + self._reset_client() + self._radio.stop() + self.params.remove("BluetoothAudioAddress") + self.params.put_bool("BluetoothEnabled", False) + self._scan_deadline = 0.0 elif command == "start_scan": if not self.params.get_bool("BluetoothEnabled"): raise RuntimeError("Enable Bluetooth before scanning") diff --git a/starpilot/system/bluetooth/tests/test_bluetooth.py b/starpilot/system/bluetooth/tests/test_bluetooth.py index 03ef72ebf..1628a50cb 100644 --- a/starpilot/system/bluetooth/tests/test_bluetooth.py +++ b/starpilot/system/bluetooth/tests/test_bluetooth.py @@ -11,6 +11,7 @@ from openpilot.starpilot.system.bluetooth.daemon import BluetoothController from openpilot.starpilot.system.bluetooth.protocol import (A2DP_SINK_UUID, HID_UUID, BluetoothClient, BluetoothDevice, BluetoothStatus, device_capabilities, show_pairing_device) from openpilot.system import hardware +from openpilot.system.ui.lib.bluetooth_manager import BluetoothManager class FakeParams: @@ -106,6 +107,35 @@ class FakeRadio: self.stops += 1 +class BlockingStopRadio(FakeRadio): + def __init__(self): + super().__init__() + self.stop_started = threading.Event() + self.allow_stop = threading.Event() + + def stop(self): + self.stops += 1 + self.stop_started.set() + self.allow_stop.wait() + + +class BlockingPowerClient: + def __init__(self): + self.power_entered = threading.Event() + self.allow_power = threading.Event() + self.power_finished = threading.Event() + self.status_calls = 0 + + def set_power(self, _enabled): + self.power_entered.set() + self.allow_power.wait() + self.power_finished.set() + + def status(self): + self.status_calls += 1 + return BluetoothStatus() + + class FakeProcess: def __init__(self): self.stdin = io.BytesIO() @@ -235,6 +265,81 @@ def test_power_pair_audio_and_offroad_enforcement(): assert not params.get_bool("BluetoothEnabled") and radio.stops == 1 and clients[0].closed +def test_status_does_not_restart_radio_during_disable(): + params = FakeParams(IsOffroad=True, BluetoothEnabled=True) + radio = BlockingStopRadio() + client = FakeBlueZ() + controller = BluetoothController(params, lambda: client, radio) + controller._bluez = client + + errors = [] + def disable(): + try: + controller.handle({"command": "set_power", "enabled": False}) + except Exception as error: + errors.append(error) + + status_started = threading.Event() + status_done = threading.Event() + status_result = [] + + def read_status(): + status_started.set() + status_result.append(controller.status()) + status_done.set() + + worker = threading.Thread(target=disable, daemon=True) + worker.start() + assert radio.stop_started.wait(timeout=1.0) + + status_worker = threading.Thread(target=read_status, daemon=True) + status_worker.start() + try: + assert status_started.wait(timeout=1.0) + assert not status_done.wait(timeout=0.1) + finally: + radio.allow_stop.set() + + worker.join(timeout=1.0) + status_worker.join(timeout=1.0) + + assert not worker.is_alive() + assert not status_worker.is_alive() + assert errors == [] + assert radio.starts == 0 + assert radio.stops == 1 + status = status_result[0] + assert not status["enabled"] + assert not params.get_bool("BluetoothEnabled") + + +def test_status_poll_does_not_overlap_power_transition(): + client = BlockingPowerClient() + manager = object.__new__(BluetoothManager) + manager._client = client + manager._lock = threading.Lock() + manager._client_lock = threading.Lock() + manager._status = BluetoothStatus() + manager._active = True + manager._exit = False + manager._operation_error = "" + manager._operations = {} + manager._power_pending = False + manager._audio_test_deadline = 0.0 + + manager.set_power(True) + assert client.power_entered.wait(timeout=1.0) + poller = threading.Thread(target=manager._poll_status) + poller.start() + poller.join(timeout=1.0) + + client.allow_power.set() + assert client.power_finished.wait(timeout=1.0) + + assert not poller.is_alive() + assert client.status_calls == 0 + + def test_audio_uses_soundd_engage_alert_and_cleans_up(): params = FakeParams(IsOffroad=True, BluetoothEnabled=True) params_memory = FakeParams() diff --git a/system/ui/lib/bluetooth_manager.py b/system/ui/lib/bluetooth_manager.py index 44d037f4b..9be2e5ce4 100644 --- a/system/ui/lib/bluetooth_manager.py +++ b/system/ui/lib/bluetooth_manager.py @@ -9,10 +9,12 @@ class BluetoothManager: def __init__(self): self._client = BluetoothClient(timeout=5.0) self._lock = threading.Lock() + self._client_lock = threading.Lock() self._status = BluetoothStatus() self._active = False self._exit = False self._operation_error = "" + self._power_pending = False self._operations = {} self._audio_test_deadline = 0.0 self._thread = threading.Thread(target=self._poll, daemon=True) @@ -54,15 +56,28 @@ class BluetoothManager: def _poll(self) -> None: while not self._exit: if self._active: - try: - status = self._client.status() - with self._lock: - self._status = status - except Exception as error: - with self._lock: - self._status = BluetoothStatus(error=str(error)) + self._poll_status() time.sleep(1.0 if self._active else 2.0) + def _poll_status(self) -> None: + # Power-on bootstraps the daemon before its RPC completes; do not report a + # transient status timeout while that transition owns the client. + with self._lock: + if self._power_pending: + return + + with self._client_lock: + with self._lock: + if self._power_pending: + return + try: + status = self._client.status() + with self._lock: + self._status = status + except Exception as error: + with self._lock: + self._status = BluetoothStatus(error=str(error)) + def _run(self, fn, *args, operation: str = "", address: str = "") -> None: normalized_address = address.upper() if normalized_address: @@ -83,7 +98,23 @@ class BluetoothManager: threading.Thread(target=worker, daemon=True).start() def set_power(self, enabled: bool) -> None: - self._run(self._client.set_power, enabled) + with self._lock: + if self._power_pending: + return + self._power_pending = True + + def worker(): + try: + with self._client_lock: + self._client.set_power(enabled) + except Exception as error: + with self._lock: + self._operation_error = str(error) + finally: + with self._lock: + self._power_pending = False + + threading.Thread(target=worker, daemon=True).start() def set_scanning(self, scanning: bool) -> None: self._run(self._client.start_scan if scanning else self._client.stop_scan)