Big Tooth 2

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")

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()

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)