fix(android): support non-rooted OnePlus 12 / OxygenOS 16#453
Open
teng-lin wants to merge 15 commits intokavishdevar:mainfrom
Open
fix(android): support non-rooted OnePlus 12 / OxygenOS 16#453teng-lin wants to merge 15 commits intokavishdevar:mainfrom
teng-lin wants to merge 15 commits intokavishdevar:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
OxygenOS 16's QTI Bluetooth stack handles L2CAP natively without root hooks. This commit: - Detect OxygenOS/ColorOS 16+ (OnePlus/OPPO/Realme on SDK 36) and skip root/radare2 setup in RadareOffsetFinder - Start service via startForegroundService() so it survives activity lifecycle (onStop unbind no longer kills the service) - Auto-reconnect L2CAP in onStartCommand() when service restarts via START_STICKY with a saved MAC address - Guard lateinit connectionStatusReceiver/serviceConnection with isInitialized checks to prevent UninitializedPropertyAccessException - Skip BLUETOOTH_PRIVILEGED setBatteryMetadata() calls on non-rooted devices to eliminate SecurityException log spam Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The settings screen rendered nothing when airPodsService was null, causing a black screen on startup until the service bind completed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two callers (onStartCommand reconnect + BLE/A2DP callback) can race into connectToSocket simultaneously. The first wins the L2CAP channel; the second fails with "Message too long" and shows a spurious error notification. Add AtomicBoolean guard to serialize connection attempts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ions The foreground service notification (ID 1) cannot be cancelled via notificationManager.cancel(). Use ID 1 for both connected and disconnected states so the battery notification replaces the "Background Service Running" one instead of showing alongside it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use CopyOnWriteArrayList/ConcurrentHashMap for AACP control command collections to prevent ConcurrentModificationException - Wrap NoiseControlSettings BroadcastReceiver in DisposableEffect to properly unregister on composable disposal (IntentReceiverLeaked) - Reset isConnectedLocally and isConnecting on bytesRead==-1 disconnect so auto-reconnect can trigger via onStartCommand Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only Bluetooth and location permissions are required to proceed past the permission screen. Notification (POST_NOTIFICATIONS) and phone (READ_PHONE_STATE, ANSWER_PHONE_CALLS) permissions are still requested but no longer block the main settings screen. The foreground service notification is exempt from POST_NOTIFICATIONS on Android 13+. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When AirPods report audio source switched back to the local device, send OWNS_CONNECTION=0x01 to reclaim control. Previously the app only gave up control but never took it back, causing ANC/transparency switching to stop working after switching audio between devices. Also guard audio source checks with localMac.isNotEmpty(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When AirPods switch audio to another device (e.g. Mac), the L2CAP AACP socket gets dropped. When audio returns to the phone, the A2DP PLAYING_STATE_CHANGED broadcast fires but the bluetoothReceiver only handled ACL_CONNECTED. Now also handle PLAYING_STATE_CHANGED to re-trigger L2CAP connection when A2DP starts playing on the AirPods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
isConnectedLocally can be stale after a remote disconnect because connectionReceiver sets it true on ACL_CONNECTED before connectToSocket runs. Now verify the socket is actually alive by probing inputStream before skipping reconnection. If the socket is dead, reset the flag and proceed with a fresh connection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
inputStream.available() returns 0 on dead sockets instead of throwing, so it can't detect stale connections. Use aacpManager.connectedDevices which is cleared on disconnect and only populated after successful AACP handshake - a reliable indicator of actual socket health. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The service is already unbound in onStop(), so calling unbindService() again in onDestroy() causes "Service not registered" error. Remove the duplicate unbind call since onStop() is called before onDestroy(). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdates to Android app: lifecycle and permission handling, composable receiver lifecycle management, service start/bind pattern, socket connection guards and recovery, thread-safe collections for AACP, OxygenOS/ColorOS 16+ detection and README additions, plus a Changes
Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add documentation for the MAC address injection workaround needed on non-rooted SDK 36 devices where the system's bluetooth_address is not accessible to user apps. Include step-by-step instructions using adb. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/composables/NoiseControlSettings.kt`:
- Around line 179-183: The broadcast registration is currently using
Context.RECEIVER_EXPORTED for noiseControlReceiver which exposes internal
app-only actions; change the SDK >= TIRAMISU branch to use
Context.RECEIVER_NOT_EXPORTED instead (keep the else branch unchanged) so the
Intent actions handled by noiseControlIntentFilter
(me.kavishdevar.librepods.ANC_DATA and
me.kavishdevar.librepods.DISCONNECT_RECEIVERS) remain app-internal and cannot be
spoofed or triggered by other apps.
🧹 Nitpick comments (3)
android/app/src/main/java/me/kavishdevar/librepods/composables/NoiseControlSettings.kt (1)
174-175: KeyDisposableEffecttocontextto avoid stale registrations.
IfLocalContextchanges (e.g., configuration change), the receiver should be re-registered with the new context.♻️ Suggested change
- DisposableEffect(Unit) { + DisposableEffect(context) {android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
2178-2189: PreferBluetoothA2dp.STATE_PLAYINGover the magic number.
Improves readability and avoids future constant drift.♻️ Suggested change
+import android.bluetooth.BluetoothA2dp ... - if (state == 10) { // BluetoothA2dp.STATE_PLAYING + if (state == BluetoothA2dp.STATE_PLAYING) {android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
473-475: Move service start/bind into a side‑effect to avoid recomposition repeats.
Running these directly in composition can trigger multiple start/bind calls.♻️ Suggested change
- val serviceIntent = Intent(context, AirPodsService::class.java) - context.startForegroundService(serviceIntent) - context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE) + LaunchedEffect(Unit) { + val serviceIntent = Intent(context, AirPodsService::class.java) + context.startForegroundService(serviceIntent) + context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE) + }
Comment on lines
+179
to
+183
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter, Context.RECEIVER_EXPORTED) | ||
| } else { | ||
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd android && find . -name "NoiseControlSettings.kt" -type fRepository: kavishdevar/librepods
Length of output: 145
🏁 Script executed:
cd android && git ls-files | grep -i noiseRepository: kavishdevar/librepods
Length of output: 541
🏁 Script executed:
find . -name "NoiseControlSettings.kt" -type fRepository: kavishdevar/librepods
Length of output: 153
🏁 Script executed:
cat -n android/app/src/main/java/me/kavishdevar/librepods/composables/NoiseControlSettings.kt | head -200 | tail -50Repository: kavishdevar/librepods
Length of output: 2379
🏁 Script executed:
cat -n android/app/src/main/java/me/kavishdevar/librepods/composables/NoiseControlSettings.kt | head -100Repository: kavishdevar/librepods
Length of output: 5178
🏁 Script executed:
find . -name "AirPodsNotifications.kt" -type fRepository: kavishdevar/librepods
Length of output: 47
🏁 Script executed:
rg "ANC_DATA|DISCONNECT_RECEIVERS" --type ktRepository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
rg "sendBroadcast" --type kt -A 3 -B 3Repository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
find . -name "*AirPodsNotifications*" -type fRepository: kavishdevar/librepods
Length of output: 47
🏁 Script executed:
find . -name "*Notification*" -type f | grep -i airpodsRepository: kavishdevar/librepods
Length of output: 47
🏁 Script executed:
rg "ANC_DATA|DISCONNECT_RECEIVERS" -A 2 -B 2Repository: kavishdevar/librepods
Length of output: 14838
🏁 Script executed:
rg "sendBroadcast" -A 3 -B 3Repository: kavishdevar/librepods
Length of output: 20765
Use RECEIVER_NOT_EXPORTED for app-internal broadcasts.
These actions (me.kavishdevar.librepods.ANC_DATA and me.kavishdevar.librepods.DISCONNECT_RECEIVERS) are sent only from internal app code. Exporting lets other apps spoof ANC updates or force unregistration.
Suggested fix
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
- context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter, Context.RECEIVER_EXPORTED)
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
+ context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter, Context.RECEIVER_NOT_EXPORTED)
} else {
context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | |
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter, Context.RECEIVER_EXPORTED) | |
| } else { | |
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter) | |
| } | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | |
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter, Context.RECEIVER_NOT_EXPORTED) | |
| } else { | |
| context.registerReceiver(noiseControlReceiver, noiseControlIntentFilter) | |
| } |
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/composables/NoiseControlSettings.kt`
around lines 179 - 183, The broadcast registration is currently using
Context.RECEIVER_EXPORTED for noiseControlReceiver which exposes internal
app-only actions; change the SDK >= TIRAMISU branch to use
Context.RECEIVER_NOT_EXPORTED instead (keep the else branch unchanged) so the
Intent actions handled by noiseControlIntentFilter
(me.kavishdevar.librepods.ANC_DATA and
me.kavishdevar.librepods.DISCONNECT_RECEIVERS) remain app-internal and cannot be
spoofed or triggered by other apps.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 76-93: The README currently suggests using adb shell run-as
me.kavishdevar.librepods ... but run-as only works for debuggable builds and
will fail on typical release installs; update the OxygenOS/ColorOS 16 setup
section to state the prerequisite that the app must be an adb-installed
debuggable build (e.g., built with DEBUG=true or installed via adb from Android
Studio) when using run-as, and provide alternatives: (1) instruct users to
enable a one-time in-app setting to paste their Bluetooth MAC into
shared_prefs/settings.xml (string name="self_mac_address"), (2) for rooted
devices show using su to edit shared_prefs/settings.xml, or (3) suggest using
Device File Explorer in Android Studio to edit
me.kavishdevar.librepods/shared_prefs/settings.xml; ensure the README mentions
the package name me.kavishdevar.librepods and the exact adb command as an option
only for debuggable builds.
The socketActuallyAlive check required aacpManager.connectedDevices to be non-empty, but during the AACP handshake window (~seconds after socket.connect() succeeds) this list is still empty. A second connectToSocket call from the A2DP profile proxy callback would see the socket as "stale", tear it down, and fail to reconnect. - Add 10-second handshake grace period to socketActuallyAlive check - Remove premature isConnectedLocally=true from connectionReceiver and takeOver (connectToSocket sets it internally on success) - Wrap socket read loop in try/catch/finally to properly handle IOException from remote disconnect, preventing stale socket state
Address PR review comment: run-as only works on debuggable builds. Add prerequisite note directing users to install the debug/nightly APK. Remove the alternative adb settings method which also requires elevated privileges.
teng-lin
force-pushed
the
fix/android-oneplus12-oxygenos16
branch
from
d090728 to
aa3db92
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`:
- Around line 1463-1465: isConnectedLocally is a plain var accessed from
multiple threads which causes a data race; make it thread-safe by converting
isConnectedLocally to an AtomicBoolean (to match isConnecting) or mark it
`@Volatile` if you only need atomic reads/writes, then update all reads/writes in
AirPodsService to use the AtomicBoolean methods (get()/set()/compareAndSet) or
simple assignments for the volatile field so access from main thread, coroutine
IO dispatchers and broadcast receivers is consistent and free of stale reads;
keep isConnecting as-is for consistency if you choose AtomicBoolean.
- Around line 1051-1065: The callback onAudioSourceReceived can mis-evaluate
when aacpManager.audioSource is null; update onAudioSourceReceived to
early-return or explicitly treat null as AudioSourceType.NONE by first capturing
val source = aacpManager.audioSource and checking if source == null (or
source.type == AACPManager.Companion.AudioSourceType.NONE) before the existing
logic, then use that local source variable in the subsequent comparisons against
localMac and when calling aacpManager.sendControlCommand with
ControlCommandIdentifiers.OWNS_CONNECTION to avoid sending the relinquish (0x00)
command erroneously.
🧹 Nitpick comments (4)
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (4)
2178-2190: UseBluetoothA2dp.STATE_PLAYINGconstant instead of magic number10.Line 2183 uses a raw int
10with a comment. The constantandroid.bluetooth.BluetoothA2dp.STATE_PLAYINGis available and should be used directly for readability and correctness.- if (state == 10) { // BluetoothA2dp.STATE_PLAYING + if (state == android.bluetooth.BluetoothA2dp.STATE_PLAYING) {
2210-2222: Reconnect fires on everyonStartCommandinvocation, includingRECONNECT_AFTER_REVERSE.The
RECONNECT_AFTER_REVERSEhandler at line 2203 doesn'treturn, so it falls through to this block.takeOver()(line 2207) already callsconnectToSocketinternally, meaning the same socket connection is attempted twice. WhileisConnectingprevents the duplicate from proceeding, consider addingreturn START_STICKYafter the reverse handler to avoid the unnecessary attempt:Proposed fix
if (intent?.action == "me.kavishdevar.librepods.RECONNECT_AFTER_REVERSE") { Log.d(TAG, "reconnect after reversed received, taking over") disconnectedBecauseReversed = false otherDeviceTookOver = false takeOver("music", manualTakeOverAfterReversed = true) + return START_STICKY }
2486-2514: After exception inwithTimeout, execution falls through to the post-timeout check.When
socket.connect()throws (caught at line 2487),return@withTimeoutexits the lambda but the code continues at line 2502. SinceisConnectingis already reset at line 2496, the subsequent check at line 2502 will also try to reset and return. This is functionally harmless but could be simplified by throwing the exception to be caught by the outercatchat line 2594, or by tracking the success state.
2178-2190: Verify that frequent A2DP play-state changes don't cause log noise or unnecessary work.Every
STATE_PLAYINGevent for the saved device re-broadcastsAIRPODS_CONNECTION_DETECTED, which triggersconnectToSocket. WhileisConnectingand the liveness check prevent actual reconnection, this could produce repetitive log entries during normal playback. Consider adding a check forisConnectedLocallybefore re-triggering:Proposed improvement
if (state == 10) { // BluetoothA2dp.STATE_PLAYING - Log.d(TAG, "A2DP playing on AirPods, re-triggering connection") - val connectionIntent = Intent(AirPodsNotifications.AIRPODS_CONNECTION_DETECTED) - connectionIntent.putExtra("name", name) - connectionIntent.putExtra("device", bluetoothDevice) - context?.sendBroadcast(connectionIntent) + val service = ServiceManager.getService() + if (service == null || !service.isConnectedLocally) { + Log.d(TAG, "A2DP playing on AirPods, re-triggering connection") + val connectionIntent = Intent(AirPodsNotifications.AIRPODS_CONNECTION_DETECTED) + connectionIntent.putExtra("name", name) + connectionIntent.putExtra("device", bluetoothDevice) + context?.sendBroadcast(connectionIntent) + } }
Comment on lines
1051
to
1065
| override fun onAudioSourceReceived(audioSource: ByteArray) { | ||
| Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}") | ||
| if (aacpManager.audioSource?.type != AACPManager.Companion.AudioSourceType.NONE && aacpManager.audioSource?.mac != localMac) { | ||
| Log.d("AirPodsParser", "Audio source is another device, better to give up aacp control") | ||
| Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}, localMac: $localMac") | ||
| if (aacpManager.audioSource?.type != AACPManager.Companion.AudioSourceType.NONE && localMac.isNotEmpty() && aacpManager.audioSource?.mac != localMac) { | ||
| Log.d("AirPodsParser", "Audio source is another device, giving up AACP control") | ||
| aacpManager.sendControlCommand( | ||
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | ||
| byteArrayOf(0x00) | ||
| ) | ||
| // this also means that the other device has start playing the audio, and if that's true, we can again start listening for audio config changes | ||
| // Log.d(TAG, "Another device started playing audio, listening for audio config changes again") | ||
| // MediaController.pausedForOtherDevice = false | ||
| // future me: what the heck is this? this just means it will not be taking over again if audio source doesn't change??? | ||
| } else if (localMac.isNotEmpty() && aacpManager.audioSource?.mac == localMac) { | ||
| Log.d("AirPodsParser", "Audio source is local device, reclaiming AACP control") | ||
| aacpManager.sendControlCommand( | ||
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | ||
| byteArrayOf(0x01) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Potential null audioSource triggers false "give up" path.
If aacpManager.audioSource is null when this callback fires, the null-safe operators yield null != NONE → true and null != localMac → true, so the code will send OWNS_CONNECTION(0x00) (relinquish) even though there's no actual other device. Guard against null at the top:
Proposed fix
override fun onAudioSourceReceived(audioSource: ByteArray) {
+ val source = aacpManager.audioSource ?: return
Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}, localMac: $localMac")
- if (aacpManager.audioSource?.type != AACPManager.Companion.AudioSourceType.NONE && localMac.isNotEmpty() && aacpManager.audioSource?.mac != localMac) {
+ if (source.type != AACPManager.Companion.AudioSourceType.NONE && localMac.isNotEmpty() && source.mac != localMac) {
Log.d("AirPodsParser", "Audio source is another device, giving up AACP control")
aacpManager.sendControlCommand(
AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value,
byteArrayOf(0x00)
)
- } else if (localMac.isNotEmpty() && aacpManager.audioSource?.mac == localMac) {
+ } else if (localMac.isNotEmpty() && source.mac == localMac) {
Log.d("AirPodsParser", "Audio source is local device, reclaiming AACP control")
aacpManager.sendControlCommand(
AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value,
byteArrayOf(0x01)
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| override fun onAudioSourceReceived(audioSource: ByteArray) { | |
| Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}") | |
| if (aacpManager.audioSource?.type != AACPManager.Companion.AudioSourceType.NONE && aacpManager.audioSource?.mac != localMac) { | |
| Log.d("AirPodsParser", "Audio source is another device, better to give up aacp control") | |
| Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}, localMac: $localMac") | |
| if (aacpManager.audioSource?.type != AACPManager.Companion.AudioSourceType.NONE && localMac.isNotEmpty() && aacpManager.audioSource?.mac != localMac) { | |
| Log.d("AirPodsParser", "Audio source is another device, giving up AACP control") | |
| aacpManager.sendControlCommand( | |
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | |
| byteArrayOf(0x00) | |
| ) | |
| // this also means that the other device has start playing the audio, and if that's true, we can again start listening for audio config changes | |
| // Log.d(TAG, "Another device started playing audio, listening for audio config changes again") | |
| // MediaController.pausedForOtherDevice = false | |
| // future me: what the heck is this? this just means it will not be taking over again if audio source doesn't change??? | |
| } else if (localMac.isNotEmpty() && aacpManager.audioSource?.mac == localMac) { | |
| Log.d("AirPodsParser", "Audio source is local device, reclaiming AACP control") | |
| aacpManager.sendControlCommand( | |
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | |
| byteArrayOf(0x01) | |
| ) | |
| } | |
| override fun onAudioSourceReceived(audioSource: ByteArray) { | |
| val source = aacpManager.audioSource ?: return | |
| Log.d("AirPodsParser", "Audio source changed mac: ${aacpManager.audioSource?.mac}, type: ${aacpManager.audioSource?.type?.name}, localMac: $localMac") | |
| if (source.type != AACPManager.Companion.AudioSourceType.NONE && localMac.isNotEmpty() && source.mac != localMac) { | |
| Log.d("AirPodsParser", "Audio source is another device, giving up AACP control") | |
| aacpManager.sendControlCommand( | |
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | |
| byteArrayOf(0x00) | |
| ) | |
| } else if (localMac.isNotEmpty() && source.mac == localMac) { | |
| Log.d("AirPodsParser", "Audio source is local device, reclaiming AACP control") | |
| aacpManager.sendControlCommand( | |
| AACPManager.Companion.ControlCommandIdentifiers.OWNS_CONNECTION.value, | |
| byteArrayOf(0x01) | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 1051 - 1065, The callback onAudioSourceReceived can mis-evaluate
when aacpManager.audioSource is null; update onAudioSourceReceived to
early-return or explicitly treat null as AudioSourceType.NONE by first capturing
val source = aacpManager.audioSource and checking if source == null (or
source.type == AACPManager.Companion.AudioSourceType.NONE) before the existing
logic, then use that local source variable in the subsequent comparisons against
localMac and when calling aacpManager.sendControlCommand with
ControlCommandIdentifiers.OWNS_CONNECTION to avoid sending the relinquish (0x00)
command erroneously.
Comment on lines
1463
to
+1465
| var isConnectedLocally = false | ||
| private val isConnecting = java.util.concurrent.atomic.AtomicBoolean(false) | ||
| @Volatile private var socketConnectedAt: Long = 0 |
There was a problem hiding this comment.
isConnectedLocally is a plain var accessed from multiple threads without synchronization.
isConnecting is correctly an AtomicBoolean, but isConnectedLocally (line 1463) is a regular var read/written from the main thread, IO coroutine dispatchers, and broadcast receivers. This is a data race that can cause stale reads. Consider making it @Volatile at minimum, or an AtomicBoolean for consistency with isConnecting.
- var isConnectedLocally = false
+ `@Volatile` var isConnectedLocally = false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var isConnectedLocally = false | |
| private val isConnecting = java.util.concurrent.atomic.AtomicBoolean(false) | |
| @Volatile private var socketConnectedAt: Long = 0 | |
| `@Volatile` var isConnectedLocally = false | |
| private val isConnecting = java.util.concurrent.atomic.AtomicBoolean(false) | |
| `@Volatile` private var socketConnectedAt: Long = 0 |
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 1463 - 1465, isConnectedLocally is a plain var accessed from
multiple threads which causes a data race; make it thread-safe by converting
isConnectedLocally to an AtomicBoolean (to match isConnecting) or mark it
`@Volatile` if you only need atomic reads/writes, then update all reads/writes in
AirPodsService to use the AtomicBoolean methods (get()/set()/compareAndSet) or
simple assignments for the volatile field so access from main thread, coroutine
IO dispatchers and broadcast receivers is consistent and free of stale reads;
keep isConnecting as-is for consistency if you choose AtomicBoolean.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
2200-2224:⚠️ Potential issue | 🟠 MajorMissing
returnafter handlingRECONNECT_AFTER_REVERSEcauses redundant reconnect attempt.After the
RECONNECT_AFTER_REVERSEblock (lines 2203-2208), execution falls through to lines 2210-2222, which unconditionally attempts another L2CAP reconnect if!isConnectedLocally. SincetakeOver("music", ...)already callsconnectToSocket, this creates a duplicate connection attempt on every reverse-reconnect.Proposed fix
if (intent?.action == "me.kavishdevar.librepods.RECONNECT_AFTER_REVERSE") { Log.d(TAG, "reconnect after reversed received, taking over") disconnectedBecauseReversed = false otherDeviceTookOver = false takeOver("music", manualTakeOverAfterReversed = true) + return START_STICKY }
🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`:
- Around line 2210-2222: The call to BluetoothAdapter.getRemoteDevice(savedMac)
can throw IllegalArgumentException for an invalid MAC string; update the
AirPodsService.kt code around the savedMac handling to either validate the MAC
format first or wrap getRemoteDevice(savedMac) in a try-catch that catches
IllegalArgumentException, logs the invalid value via Log.w/Log.e (including TAG
and savedMac), and skips attempting connectToSocket if invalid; ensure you still
launch CoroutineScope(Dispatchers.IO).launch { connectToSocket(bluetoothDevice)
} only when a non-null, valid bluetoothDevice is obtained.
- Around line 2429-2437: The access to aacpManager.connectedDevices from
AirPodsService (in the socketActuallyAlive check) is not thread-safe because
AACPManager mutates connectedDevices; update AACPManager to use a thread-safe
collection (e.g., declare connectedDevices as a
CopyOnWriteArrayList<ConnectedDevice>) or serialize access by adding a private
lock and guarding all reads/writes (including places that reassign or modify
connectedDevices such as the methods that add/remove or update entries). Ensure
all code that references connectedDevices (reads in AirPodsService and mutations
in AACPManager methods) uses the chosen thread-safe collection or the same
synchronization primitive so concurrent reads like
aacpManager.connectedDevices.isNotEmpty() are safe.
🧹 Nitpick comments (1)
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
2178-2190: Magic number10for A2DP playing state; potential reconnect churn.
- Replace
10withBluetoothA2dp.STATE_PLAYING(value is10) for readability.- Every time A2DP starts playing on the saved device, this re-broadcasts
AIRPODS_CONNECTION_DETECTED, which callsconnectToSocket. While theisConnectingguard andsocketActuallyAlivecheck prevent actual duplicate connections, this still triggers socket-creation + aliveness evaluation on every play-state transition. Consider gating this on!isConnectedLocally(or!socketActuallyAlive) to avoid unnecessary work.Proposed fix
} else if (action == "android.bluetooth.a2dp.profile.action.PLAYING_STATE_CHANGED") { val savedMac = context?.getSharedPreferences("settings", MODE_PRIVATE) ?.getString("mac_address", "") if (!savedMac.isNullOrEmpty() && bluetoothDevice?.address == savedMac) { - val state = intent.getIntExtra("android.bluetooth.profile.extra.STATE", -1) - if (state == 10) { // BluetoothA2dp.STATE_PLAYING + val state = intent.getIntExtra(BluetoothProfile.EXTRA_STATE, -1) + if (state == 10 /* BluetoothA2dp.STATE_PLAYING */) { + val service = ServiceManager.getService() + if (service != null && service.isConnectedLocally) { + Log.d(TAG, "A2DP playing but already connected locally, skipping re-trigger") + return + } Log.d(TAG, "A2DP playing on AirPods, re-triggering connection")
Comment on lines
+2210
to
+2222
| if (!isConnectedLocally && ::sharedPreferences.isInitialized) { | ||
| val savedMac = sharedPreferences.getString("mac_address", "") | ||
| if (!savedMac.isNullOrEmpty()) { | ||
| Log.d(TAG, "Service restarted, attempting L2CAP reconnect to $savedMac") | ||
| val bluetoothManager = getSystemService(BluetoothManager::class.java) | ||
| val bluetoothDevice = bluetoothManager?.adapter?.getRemoteDevice(savedMac) | ||
| if (bluetoothDevice != null) { | ||
| CoroutineScope(Dispatchers.IO).launch { | ||
| connectToSocket(bluetoothDevice) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
getRemoteDevice can throw on an invalid MAC string.
BluetoothAdapter.getRemoteDevice(savedMac) throws IllegalArgumentException if savedMac is not a valid Bluetooth hardware address (e.g., corrupted preference value). Wrap in a try-catch or validate the format first.
Proposed fix
if (!savedMac.isNullOrEmpty()) {
Log.d(TAG, "Service restarted, attempting L2CAP reconnect to $savedMac")
val bluetoothManager = getSystemService(BluetoothManager::class.java)
- val bluetoothDevice = bluetoothManager?.adapter?.getRemoteDevice(savedMac)
+ val bluetoothDevice = try {
+ bluetoothManager?.adapter?.getRemoteDevice(savedMac)
+ } catch (e: IllegalArgumentException) {
+ Log.e(TAG, "Invalid saved MAC address: $savedMac", e)
+ null
+ }
if (bluetoothDevice != null) {🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 2210 - 2222, The call to BluetoothAdapter.getRemoteDevice(savedMac)
can throw IllegalArgumentException for an invalid MAC string; update the
AirPodsService.kt code around the savedMac handling to either validate the MAC
format first or wrap getRemoteDevice(savedMac) in a try-catch that catches
IllegalArgumentException, logs the invalid value via Log.w/Log.e (including TAG
and savedMac), and skips attempting connectToSocket if invalid; ensure you still
launch CoroutineScope(Dispatchers.IO).launch { connectToSocket(bluetoothDevice)
} only when a non-null, valid bluetoothDevice is obtained.
Comment on lines
+2429
to
+2437
| val inHandshakeWindow = System.currentTimeMillis() - socketConnectedAt < 10_000 | ||
| val socketActuallyAlive = isConnectedLocally && this::socket.isInitialized && | ||
| socket.isConnected && (aacpManager.connectedDevices.isNotEmpty() || inHandshakeWindow) | ||
| if (!socketActuallyAlive) { | ||
| if (isConnectedLocally) { | ||
| Log.d(TAG, "isConnectedLocally was true but socket is dead, resetting") | ||
| isConnectedLocally = false | ||
| if (this::socket.isInitialized) try { socket.close() } catch (_: Exception) { } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "class AACPManager" | head -5Repository: kavishdevar/librepods
Length of output: 138
🏁 Script executed:
rg -n "connectedDevices" --type=kt -C3 | grep -E "(AACPManager|connectedDevices.*=|val connectedDevices|var connectedDevices)" | head -20Repository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
# Search for the AACPManager class definition and find connectedDevices
rg -n "class AACPManager" --type=kt -A 50Repository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
cat ./android/app/src/main/java/me/kavishdevar/librepods/utils/AACPManager.kt | head -100Repository: kavishdevar/librepods
Length of output: 3820
🏁 Script executed:
rg -n "connectedDevices" ./android/app/src/main/java/me/kavishdevar/librepods/utils/AACPManager.kt -A 2 -B 2Repository: kavishdevar/librepods
Length of output: 4014
Thread-safe access to connectedDevices is required.
The socketActuallyAlive check at lines 2430–2431 reads aacpManager.connectedDevices.isNotEmpty() without synchronization. However, connectedDevices in AACPManager is declared as a regular List<ConnectedDevice> (line 213) and is mutated in multiple places (e.g., line 535 where it's reassigned, and lines 551–561 where elements are modified). This creates a data race when AirPodsService reads it concurrently with mutations in AACPManager. Convert connectedDevices to a thread-safe collection such as CopyOnWriteArrayList or use synchronization to guard concurrent access.
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 2429 - 2437, The access to aacpManager.connectedDevices from
AirPodsService (in the socketActuallyAlive check) is not thread-safe because
AACPManager mutates connectedDevices; update AACPManager to use a thread-safe
collection (e.g., declare connectedDevices as a
CopyOnWriteArrayList<ConnectedDevice>) or serialize access by adding a private
lock and guarding all reads/writes (including places that reassign or modify
connectedDevices such as the methods that add/remove or update entries). Ensure
all code that references connectedDevices (reads in AirPodsService and mutations
in AACPManager methods) uses the chosen thread-safe collection or the same
synchronization primitive so concurrent reads like
aacpManager.connectedDevices.isNotEmpty() are safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR enables LibrePods to work on non-rooted OnePlus 12 phones with OxygenOS 16, bringing full ANC, transparency, audio control, and other core features without requiring root or Xposed.
Changes
Testing
Compatibility
Device Tested
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation