Fix/async bt connect#1
Conversation
IOBluetoothDevice.openConnection() is synchronous and blocks the calling thread until the baseband link is established or times out (up to several seconds). connect() ran on the main actor and rescheduled its verification poll on DispatchQueue.main, so a slow or failing AirPods connect could stall the menu-bar app and delay processing of other audio events. Move the whole connect/poll cycle onto a dedicated serial background queue. connect() now returns immediately; no behavior change beyond not blocking the main thread. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds an opt-in yieldToOtherSource setting. When enabled, PodSwitch stops fighting for the target device after another source has taken it: once the target's A2DP link to this phone drops (a CONNECTED->DISCONNECTED transition seen by the new TargetConnectionMonitor) it is treated as yielded, and media starting on the phone will not auto-steal it back. Normal switching resumes when the target reconnects to the phone on its own, when playback fully stops, or when the user explicitly accepts a switch. Limitation: Android exposes no way to observe what a remote host streams through a shared sink, so 'busy elsewhere' is inferred from the target's own A2DP link to this phone, not from the other source's playback. This works for single-link setups (another device grabbing the headphones shows up as a disconnect); with multipoint the link may persist and the existing already-active guard applies instead. Engine + Coordinator changes covered by new unit tests. NOT compiled here: no Android SDK/JDK in this environment — needs an on-device/CI build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'yield to other source' feature on Android to prevent reclaiming the Bluetooth connection when another device takes over, supported by a new TargetConnectionMonitor and corresponding UI and settings updates. On macOS, it moves Bluetooth connection work to a background queue to prevent blocking the main thread. Key feedback includes addressing thread-safety issues with IOBluetooth on macOS by avoiding background queue execution, ensuring the Android broadcast receiver is registered as RECEIVER_EXPORTED to properly receive Bluetooth events, using IntentCompat for backward compatibility, and avoiding main-thread blocking disk reads in SwitchService by caching the target device ID.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| public func connect(deviceId: String) { | ||
| guard let device = pairedDevice(matching: deviceId) else { | ||
| Self.log.error("connect: device not found among paired devices") | ||
| return | ||
| } | ||
| if device.isConnected() && isActiveOutput(deviceId: deviceId) { | ||
| return | ||
| Self.queue.async { | ||
| guard let device = pairedDevice(matching: deviceId) else { | ||
| Self.log.error("connect: device not found among paired devices") | ||
| return | ||
| } | ||
| if device.isConnected() && isActiveOutput(deviceId: deviceId) { | ||
| return | ||
| } | ||
| attemptConnect(deviceId: deviceId, attempt: 1) | ||
| } | ||
| attemptConnect(deviceId: deviceId, attempt: 1) | ||
| } |
There was a problem hiding this comment.
IOBluetooth is not thread-safe. Calling its APIs (such as IOBluetoothDevice.pairedDevices(), device.isConnected(), and device.openConnection()) on a background queue can lead to race conditions, memory corruption, or crashes (e.g., EXC_BAD_ACCESS).
To safely connect asynchronously without blocking the main thread, consider using the asynchronous C API IOBluetoothDeviceAsyncConnectionCreate on the main thread, or ensure all IOBluetooth interactions are dispatched back to the main thread.
| ContextCompat.registerReceiver( | ||
| context, | ||
| callback, | ||
| IntentFilter(BluetoothA2dp.ACTION_CONNECTION_STATE_CHANGED), | ||
| ContextCompat.RECEIVER_NOT_EXPORTED, | ||
| ) |
There was a problem hiding this comment.
On Android 13+ (API 33+), registering a receiver with RECEIVER_NOT_EXPORTED prevents it from receiving broadcasts from other UIDs/processes. Since the Bluetooth stack (com.android.bluetooth) runs under a separate system UID (not UID 1000), registering this receiver as RECEIVER_NOT_EXPORTED will prevent it from receiving ACTION_CONNECTION_STATE_CHANGED broadcasts on many devices.
Since BluetoothA2dp.ACTION_CONNECTION_STATE_CHANGED is a protected system broadcast that cannot be spoofed by third-party apps, it is safe and necessary to register this receiver as RECEIVER_EXPORTED.
| ContextCompat.registerReceiver( | |
| context, | |
| callback, | |
| IntentFilter(BluetoothA2dp.ACTION_CONNECTION_STATE_CHANGED), | |
| ContextCompat.RECEIVER_NOT_EXPORTED, | |
| ) | |
| ContextCompat.registerReceiver( | |
| context, | |
| callback, | |
| IntentFilter(BluetoothA2dp.ACTION_CONNECTION_STATE_CHANGED), | |
| ContextCompat.RECEIVER_EXPORTED, | |
| ) |
| @Suppress("DEPRECATION") | ||
| private fun deviceFrom(intent: Intent): BluetoothDevice? = | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE, BluetoothDevice::class.java) | ||
| } else { | ||
| intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE) | ||
| } |
There was a problem hiding this comment.
Instead of manually checking the SDK version and suppressing deprecation warnings, you can use IntentCompat.getParcelableExtra from androidx.core.content.IntentCompat. This provides a safe, backward-compatible way to retrieve parcelable extras.
private fun deviceFrom(intent: Intent): BluetoothDevice? =
androidx.core.content.IntentCompat.getParcelableExtra(
intent,
BluetoothDevice.EXTRA_DEVICE,
BluetoothDevice::class.java
)| connectionMonitor = TargetConnectionMonitor( | ||
| context = applicationContext, | ||
| targetAddress = { settings.currentConfig().targetDeviceId }, | ||
| onChanged = { connected -> | ||
| coordinator.handle(SwitchEvent.TargetConnectionChanged(connected)) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Calling settings.currentConfig() inside the targetAddress lambda triggers runBlocking on the main thread every time a Bluetooth connection state broadcast is received. Since DataStore reads from disk, this can cause UI jank or ANRs (Application Not Responding).
Since SwitchService is a LifecycleService, we can collect settings.configFlow within the lifecycleScope to maintain a cached targetDeviceId in memory, completely avoiding blocking the main thread.
var targetDeviceId: String? = null
androidx.lifecycle.lifecycleScope.launch {
settings.configFlow.collect { config ->
targetDeviceId = config.targetDeviceId
}
}
connectionMonitor = TargetConnectionMonitor(
context = applicationContext,
targetAddress = { targetDeviceId },
onChanged = { connected ->
coordinator.handle(SwitchEvent.TargetConnectionChanged(connected))
},
)1 participant
No description provided.