fix(android): support non-rooted OnePlus 12 / OxygenOS 16#450
fix(android): support non-rooted OnePlus 12 / OxygenOS 16#450teng-lin wants to merge 2 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>
|
Caution Review failedFailed to post review comments Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStarts the foreground service before binding in the UI, adds guarded unbind/unregister checks, implements coroutine-based L2CAP reconnect on service restart using saved MAC, short-circuits radare offset checks for OxygenOS/ColorOS 16+, and adds 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 |
![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/MainActivity.kt`:
- Around line 452-456: The service currently does significant setup before
posting its foreground notification, risking
ForegroundServiceDidNotStartInTimeException; modify AirPodsService.onCreate() to
call startForegroundNotification() as the very first operation (or immediately
after minimal safety initialization) so the notification/startForeground is
guaranteed before BLE setup, broadcast registration, or other heavy work in
AirPodsService; then move BLE initialization and registerReceiver calls into a
separate method or coroutine launched after startForegroundNotification() so
heavy work happens post-foreground, and ensure startForegroundNotification() and
any minimal initialization are safe to run early.
🧹 Nitpick comments (3)
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
2200-2231:attemptL2capReconnect()uses an unstructuredCoroutineScope— consider tying it to the service lifecycle.
CoroutineScope(Dispatchers.IO)at line 2228 creates an orphaned scope. While theJobreference is saved and cancelled inonDestroy, ifcancel()is called, only the job is cancelled — the scope is never cleaned up. This is a minor concern since there's no leak beyond the job itself.More importantly,
connectToSocket(line 2229) internally callsrunBlockingwithwithTimeout(5000L)(around line 2442–2493). SinceattemptL2capReconnectis launched onDispatchers.IO, therunBlockingwill block an IO thread — which is acceptable but worth noting. The reconnect will also trigger the full socket read loop insideconnectToSocket, which runs indefinitely until the socket closes.One edge case: if
onStartCommandis called rapidly (e.g., service restart + intent delivery), the previousreconnectJobis cancelled, butconnectToSocketmay have already setisConnectedLocally = trueand started the read loop in a nested coroutine that isn't tracked byreconnectJob. This nested coroutine (line 2512) is launched from a differentCoroutineScope(Dispatchers.IO)insideconnectToSocketand won't be cancelled.Consider using a service-scoped CoroutineScope
+ private val serviceScope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + `@SuppressLint`("MissingPermission") private fun attemptL2capReconnect() { // ... guards ... reconnectJob?.cancel() - reconnectJob = CoroutineScope(Dispatchers.IO).launch { + reconnectJob = serviceScope.launch { connectToSocket(bluetoothDevice) } } override fun onDestroy() { - reconnectJob?.cancel() + serviceScope.cancel() // ... }android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
142-143: Top-levellateinit varforserviceConnectionandconnectionStatusReceiveris fragile.These are module-level mutable properties shared between
MainActivityand theMain()composable. This coupling makes lifecycle management error-prone — any recomposition or configuration change could interact unexpectedly. TheisInitializedguards added in this PR are a good mitigation, but ideally these should be scoped to the Activity or a ViewModel.Not blocking since this is pre-existing design, but worth noting for future cleanup.
android/app/src/main/java/me/kavishdevar/librepods/utils/RadareOffsetFinder.kt (1)
64-69: OxygenOS/ColorOS detection uses manufacturer heuristic — edge cases are possible but low-impact.The detection logic correctly lowercases
Build.MANUFACTURERand checks for SDK 36+. However, the approach makes an assumption that may not hold in all cases:
- Custom ROMs: Devices running aftermarket ROMs on OnePlus/OPPO/Realme hardware would match the manufacturer check but may lack native L2CAP support, causing battery metadata setup to be skipped when it shouldn't be.
- Future device variants: Not all devices from these OEMs on SDK 36+ will necessarily have the QTI Bluetooth stack.
The codebase already has a
findBluetoothLibraryPath()function that checks for QTI library existence. If edge cases are a concern, consider adding a runtime capability check (e.g., probing for library availability) before assuming L2CAP is available. This would handle custom ROMs and variant devices more robustly.For mainstream use, the current heuristic is reasonable.
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt
Outdated
Show resolved
Hide resolved
teng-lin
force-pushed
the
fix/android-oneplus12-oxygenos16
branch
from
55e477a to
dec6b7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
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)
2193-2234:⚠️ Potential issue | 🟠 MajorPotential double-connect on
RECONNECT_AFTER_REVERSEintent.When the intent action is
RECONNECT_AFTER_REVERSE,takeOver()is called at line 2200 (which internally callsconnectToSocket), and thenattemptL2capReconnect()is called unconditionally at line 2203, which also launchesconnectToSocketin a coroutine. This can result in two concurrent connection attempts to the same device.Consider returning early or skipping the reconnect attempt when the reverse-reconnect path was taken:
Suggested 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 } attemptL2capReconnect()
🧹 Nitpick comments (1)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
142-143: Top-levellateinit vardeclarations are global mutable state.
serviceConnectionandconnectionStatusReceiverare top-level mutable properties, making them accessible and modifiable from anywhere. This is a pre-existing pattern, but consider encapsulating these withinMainActivityas private members in a future refactor to avoid accidental misuse.
teng-lin
force-pushed
the
fix/android-oneplus12-oxygenos16
branch
2 times, most recently
from
28990fc to
66c6a25
Compare
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)
2221-2234:⚠️ Potential issue | 🟠 MajorPotential duplicate connection attempt on
RECONNECT_AFTER_REVERSEintent.When the action is
RECONNECT_AFTER_REVERSE,takeOver()(Line 2228) may initiate its ownconnectToSocket()call, and thenattemptL2capReconnect()(Line 2231) runs immediately after. SincetakeOver()launches coroutines internally,isConnectedLocallymay not yet betruewhenattemptL2capReconnect()checks it, leading to a race where two connection attempts proceed concurrently.Consider guarding with an early return:
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 } attemptL2capReconnect()
🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`:
- Around line 380-423: The su fallback can hang because
Runtime.getRuntime().exec(...).waitFor() may block; update the su branch that
assigns localMac to use Process.waitFor(timeout, TimeUnit.MILLISECONDS/SECONDS)
(API 26+) on the Process returned by Runtime.getRuntime().exec in the
AirPodsService code (the block that constructs arrayOf("su", "-c", "settings get
secure bluetooth_address")), and if waitFor times out call process.destroy() (or
destroyForcibly()) and log a timeout error before returning an empty string;
ensure you still read inputStream only when exitCode == 0 and handle exceptions
as before.
In
`@android/app/src/main/java/me/kavishdevar/librepods/utils/RadareOffsetFinder.kt`:
- Around line 64-69: The current isOxygenOSOrColorOS16OrAbove() uses only
Build.MANUFACTURER (COLOROS_MANUFACTURERS) and SDK to detect stock
OxygenOS/ColorOS, which can false-positive on custom ROMs; update that function
to also verify the ROM identifier by checking Build.DISPLAY and/or the system
property ro.build.display.id (via android.os.SystemProperties or reflection) for
substrings like "OxygenOS", "ColorOS", "RealmeUI" before returning true, so both
manufacturer and ROM/display id checks must pass; keep the existing
COLOROS_MANUFACTURERS constant and the SDK >= 36 check and add the additional
display/property check inside isOxygenOSOrColorOS16OrAbove().
🧹 Nitpick comments (1)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
142-143: Top-levellateinit varforserviceConnectionandconnectionStatusReceiveris fragile.These module-level mutable variables are shared between the composable (
Main()) and the Activity lifecycle (onDestroy). If multipleMainActivityinstances exist (e.g., configuration change withsingleTopnot set), or if the composable is recomposed, the old reference could be lost. Consider moving these to Activity-level properties or using aremember-backed pattern that feeds into a callback for cleanup.This is a pre-existing pattern and not introduced by this PR, so flagging as optional.
| // Try Settings.Secure content provider first | ||
| localMac = try { | ||
| val process = Runtime.getRuntime().exec( | ||
| arrayOf("su", "-c", "settings get secure bluetooth_address") | ||
| ) | ||
|
|
||
| val exitCode = process.waitFor() | ||
|
|
||
| if (exitCode == 0) { | ||
| process.inputStream.bufferedReader().use { it.readLine()?.trim().orEmpty() } | ||
| } else { | ||
| "" | ||
| } | ||
| Settings.Secure.getString(contentResolver, "bluetooth_address")?.trim().orEmpty() | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "Error retrieving local MAC address: ${e.message}. We probably aren't rooted.") | ||
| Log.d(TAG, "Could not read bluetooth_address from Settings.Secure: ${e.message}") | ||
| "" | ||
| } | ||
| // Try shell command without root (works on OxygenOS 16+) | ||
| if (localMac.isEmpty()) { | ||
| localMac = try { | ||
| val process = Runtime.getRuntime().exec( | ||
| arrayOf("settings", "get", "secure", "bluetooth_address") | ||
| ) | ||
| val exitCode = process.waitFor() | ||
| if (exitCode == 0) { | ||
| process.inputStream.bufferedReader().use { it.readLine()?.trim().orEmpty() } | ||
| } else { | ||
| "" | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.d(TAG, "Could not read bluetooth_address via settings command: ${e.message}") | ||
| "" | ||
| } | ||
| } | ||
| // Last resort: try with su (rooted devices) | ||
| if (localMac.isEmpty()) { | ||
| localMac = try { | ||
| val process = Runtime.getRuntime().exec( | ||
| arrayOf("su", "-c", "settings get secure bluetooth_address") | ||
| ) | ||
| val exitCode = process.waitFor() | ||
| if (exitCode == 0) { | ||
| process.inputStream.bufferedReader().use { it.readLine()?.trim().orEmpty() } | ||
| } else { | ||
| "" | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "Error retrieving local MAC address: ${e.message}. We probably aren't rooted.") | ||
| "" | ||
| } | ||
| } | ||
| if (localMac.isNotEmpty()) { | ||
| Log.d(TAG, "Local Bluetooth MAC: ...${localMac.takeLast(5)}") | ||
| } |
There was a problem hiding this comment.
Multi-step MAC retrieval is a solid fallback chain, but the su step can hang.
Runtime.exec("su …").waitFor() (Line 410) blocks indefinitely if the su binary exists but prompts for user confirmation or stalls. Consider adding a timeout (e.g., via Process.waitFor(timeout, unit) available since API 26) to prevent onCreate from blocking.
Proposed fix: add timeout to su process
val process = Runtime.getRuntime().exec(
arrayOf("su", "-c", "settings get secure bluetooth_address")
)
- val exitCode = process.waitFor()
+ val finished = process.waitFor(3, java.util.concurrent.TimeUnit.SECONDS)
+ val exitCode = if (finished) process.exitValue() else -1
+ if (!finished) process.destroyForcibly()
if (exitCode == 0) {🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 380 - 423, The su fallback can hang because
Runtime.getRuntime().exec(...).waitFor() may block; update the su branch that
assigns localMac to use Process.waitFor(timeout, TimeUnit.MILLISECONDS/SECONDS)
(API 26+) on the Process returned by Runtime.getRuntime().exec in the
AirPodsService code (the block that constructs arrayOf("su", "-c", "settings get
secure bluetooth_address")), and if waitFor times out call process.destroy() (or
destroyForcibly()) and log a timeout error before returning an empty string;
ensure you still read inputStream only when exitCode == 0 and handle exceptions
as before.
| private val COLOROS_MANUFACTURERS = setOf("oneplus", "oppo", "realme") | ||
|
|
||
| fun isOxygenOSOrColorOS16OrAbove(): Boolean { | ||
| return Build.MANUFACTURER.lowercase() in COLOROS_MANUFACTURERS | ||
| && Build.VERSION.SDK_INT >= 36 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual implementation of isOxygenOSOrColorOS16OrAbove
cat -n android/app/src/main/java/me/kavishdevar/librepods/utils/RadareOffsetFinder.kt | head -80Repository: kavishdevar/librepods
Length of output: 4086
🏁 Script executed:
# Check where this function is used in the codebase
rg -n "isOxygenOSOrColorOS16OrAbove" --type=ktRepository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
# Check for other OS-fingerprinting patterns in the repo to see if there are better examples
rg -n "Build\.MANUFACTURER|Build\.DISPLAY|ro\.build" --type=kt -B2 -A2Repository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
# Check if there are system property checks elsewhere
rg -n "getprop|ro\." --type=kt | head -20Repository: kavishdevar/librepods
Length of output: 91
🏁 Script executed:
# Search for usage of the function without type filter
rg -n "isOxygenOSOrColorOS16OrAbove" --type-not=binaryRepository: kavishdevar/librepods
Length of output: 95
🏁 Script executed:
# Search for OS detection patterns without type filter
rg -n "Build\.MANUFACTURER|Build\.DISPLAY" --type-not=binary -B2 -A2Repository: kavishdevar/librepods
Length of output: 95
🏁 Script executed:
# Verify Android SDK INT mapping - check if SDK 36 is Android 16
rg -n "SDK_INT\|Build\.VERSION" --type-not=binary | grep -E "(36|35|16|15)" | head -10Repository: kavishdevar/librepods
Length of output: 95
🏁 Script executed:
# Look at how the function is used in context
rg -B5 -A5 "isOxygenOSOrColorOS16OrAbove" --type-not=binaryRepository: kavishdevar/librepods
Length of output: 95
🏁 Script executed:
# Search for function usage without type filter
rg -n "isOxygenOSOrColorOS16OrAbove"Repository: kavishdevar/librepods
Length of output: 615
🏁 Script executed:
# Search for Build.MANUFACTURER or Build.DISPLAY usage
rg -n "Build\.MANUFACTURER|Build\.DISPLAY"Repository: kavishdevar/librepods
Length of output: 221
🏁 Script executed:
# Check Android API level to version mapping (general knowledge verification)
# Look for any comments or documentation about SDK levels
rg -n "SDK_INT|Android [0-9]+" | head -20Repository: kavishdevar/librepods
Length of output: 3142
🏁 Script executed:
# Look for the uses of the isOxygenOSOrColorOS16OrAbove in context (lines around usage)
rg -B10 -A10 "isOxygenOSOrColorOS16OrAbove"Repository: kavishdevar/librepods
Length of output: 10510
🌐 Web query:
Android SDK_INT level 36 API version
💡 Result:
Build.VERSION.SDK_INT == 36 means the device is running Android 16, whose API level 36 version code/codename is BAKLAVA (“Baklava”). [1][2]
Sources: Android Developers Build.VERSION_CODES (shows BAKLAVA = 36) and Android SDK Platform release notes (lists “Android 16 (API level 36)”). [1][2]
Manufacturer-only check may false-positive on custom ROMs.
The detection relies solely on Build.MANUFACTURER + SDK level, but a OnePlus/OPPO/Realme device running a custom ROM (e.g., LineageOS) won't have the QTI Bluetooth stack. Consider additionally checking Build.DISPLAY or a system property (e.g., ro.build.display.id containing "OxygenOS" / "ColorOS") to confirm the stock ROM is present.
This won't affect stock ROM users but could cause silent misbehavior for custom-ROM users on matching hardware.
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/utils/RadareOffsetFinder.kt`
around lines 64 - 69, The current isOxygenOSOrColorOS16OrAbove() uses only
Build.MANUFACTURER (COLOROS_MANUFACTURERS) and SDK to detect stock
OxygenOS/ColorOS, which can false-positive on custom ROMs; update that function
to also verify the ROM identifier by checking Build.DISPLAY and/or the system
property ro.build.display.id (via android.os.SystemProperties or reflection) for
substrings like "OxygenOS", "ColorOS", "RealmeUI" before returning true, so both
manufacturer and ROM/display id checks must pass; keep the existing
COLOROS_MANUFACTURERS constant and the SDK >= 36 check and add the additional
display/property check inside isOxygenOSOrColorOS16OrAbove().
fbb268a to
8bdee2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt`:
- Around line 468-470: The startForegroundService and bindService calls are
being executed directly in the composable body and will run on every
recomposition; wrap these side-effects so they run once and are cleaned up: move
the Intent creation and context.startForegroundService(serviceIntent) and
context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE)
into a LaunchedEffect(Unit) or DisposableEffect(Unit) block (use
DisposableEffect if you need to unbind in onDispose), referencing the existing
serviceConnection and AirPodsService class, and in DisposableEffect's onDispose
call context.unbindService(serviceConnection) (and stop the service if needed)
to avoid multiple bindings and leaks.
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`:
- Around line 2198-2201: savedMac is only checked for null/empty but may be
malformed and cause BluetoothAdapter.getRemoteDevice(savedMac) to throw
IllegalArgumentException; update AirPodsService to validate the MAC string
before calling getRemoteDevice (e.g., check against a MAC regex for six hex
pairs separated by ':' or '-') or wrap the getRemoteDevice(...) call in a
try/catch for IllegalArgumentException, log the invalid savedMac and skip the
reconnect attempt; reference the savedMac variable and the call to
bluetoothManager?.adapter?.getRemoteDevice(savedMac) in your change.
- Around line 2196-2208: The reconnect coroutine launched in onStartCommand is
unstructured and can run multiple times concurrently; create a service-scoped
CoroutineScope and a cancellable reconnect Job to serialize and manage reconnect
attempts: add class-level fields like serviceScope =
CoroutineScope(SupervisorJob() + Dispatchers.IO) and var reconnectJob: Job?,
replace CoroutineScope(Dispatchers.IO).launch { connectToSocket(bluetoothDevice)
} with reconnectJob?.cancel() followed by reconnectJob = serviceScope.launch {
if (!isConnectedLocally) connectToSocket(bluetoothDevice) }, ensure
connectToSocket and the isConnectedLocally check are used inside that scope to
avoid race conditions, and cancel serviceScope (or call serviceScope.cancel())
in onDestroy to avoid leaked coroutines.
| val serviceIntent = Intent(context, AirPodsService::class.java) | ||
| context.startForegroundService(serviceIntent) | ||
| context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE) |
There was a problem hiding this comment.
startForegroundService() and bindService() are called on every recomposition — must be wrapped in a side-effect.
These calls sit directly in the composable body without any side-effect wrapper. Composables can recompose many times (on any state change in the parent tree), so startForegroundService and especially bindService will fire repeatedly, leading to multiple bindings without corresponding unbinds.
Wrap in LaunchedEffect(Unit) (or DisposableEffect) so they execute only once:
Proposed fix
- 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)
+ }📝 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.
| 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) | |
| } |
🤖 Prompt for AI Agents
In `@android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt` around
lines 468 - 470, The startForegroundService and bindService calls are being
executed directly in the composable body and will run on every recomposition;
wrap these side-effects so they run once and are cleaned up: move the Intent
creation and context.startForegroundService(serviceIntent) and
context.bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE)
into a LaunchedEffect(Unit) or DisposableEffect(Unit) block (use
DisposableEffect if you need to unbind in onDispose), referencing the existing
serviceConnection and AirPodsService class, and in DisposableEffect's onDispose
call context.unbindService(serviceConnection) (and stop the service if needed)
to avoid multiple bindings and leaks.
| 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.
Unstructured coroutine scope risks duplicate reconnect attempts and leaked coroutines.
onStartCommand can be called multiple times (START_STICKY restarts, duplicate intents). Each invocation launches a new CoroutineScope(Dispatchers.IO) coroutine with no tracking or cancellation. This creates two problems:
- Race condition: Multiple coroutines can pass the
!isConnectedLocallycheck concurrently before any setsisConnectedLocally = trueinsideconnectToSocket, leading to parallel socket connection attempts. - Leaked coroutine: The scope is unstructured and never cancelled in
onDestroy(), so the reconnect work can outlive the service.
Consider using a service-scoped CoroutineScope with SupervisorJob (as noted in the PR description) and cancelling previous reconnect jobs.
Proposed fix (sketch)
Add a service-level scope and reconnect job:
// Class-level fields
private val serviceScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
private var reconnectJob: Job? = nullIn onStartCommand:
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 {
+ reconnectJob?.cancel()
+ reconnectJob = serviceScope.launch {
connectToSocket(bluetoothDevice)
}
}
}
}In onDestroy:
+ serviceScope.cancel()
super.onDestroy()🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 2196 - 2208, The reconnect coroutine launched in onStartCommand is
unstructured and can run multiple times concurrently; create a service-scoped
CoroutineScope and a cancellable reconnect Job to serialize and manage reconnect
attempts: add class-level fields like serviceScope =
CoroutineScope(SupervisorJob() + Dispatchers.IO) and var reconnectJob: Job?,
replace CoroutineScope(Dispatchers.IO).launch { connectToSocket(bluetoothDevice)
} with reconnectJob?.cancel() followed by reconnectJob = serviceScope.launch {
if (!isConnectedLocally) connectToSocket(bluetoothDevice) }, ensure
connectToSocket and the isConnectedLocally check are used inside that scope to
avoid race conditions, and cancel serviceScope (or call serviceScope.cancel())
in onDestroy to avoid leaked coroutines.
| 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) |
There was a problem hiding this comment.
getRemoteDevice() throws IllegalArgumentException on malformed MAC addresses.
The savedMac value is only checked for null/empty, but if the stored preference contains a malformed MAC string, getRemoteDevice(savedMac) will throw. Consider validating the MAC format before calling it.
Proposed fix
val savedMac = sharedPreferences.getString("mac_address", "")
- if (!savedMac.isNullOrEmpty()) {
+ if (!savedMac.isNullOrEmpty() && BluetoothAdapter.checkBluetoothAddress(savedMac)) {📝 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.
| 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 (!savedMac.isNullOrEmpty() && BluetoothAdapter.checkBluetoothAddress(savedMac)) { | |
| Log.d(TAG, "Service restarted, attempting L2CAP reconnect to $savedMac") | |
| val bluetoothManager = getSystemService(BluetoothManager::class.java) | |
| val bluetoothDevice = bluetoothManager?.adapter?.getRemoteDevice(savedMac) |
🤖 Prompt for AI Agents
In
`@android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt`
around lines 2198 - 2201, savedMac is only checked for null/empty but may be
malformed and cause BluetoothAdapter.getRemoteDevice(savedMac) to throw
IllegalArgumentException; update AirPodsService to validate the MAC string
before calling getRemoteDevice (e.g., check against a MAC regex for six hex
pairs separated by ':' or '-') or wrap the getRemoteDevice(...) call in a
try/catch for IllegalArgumentException, log the invalid savedMac and skip the
reconnect attempt; reference the savedMac variable and the call to
bluetoothManager?.adapter?.getRemoteDevice(savedMac) in your change.
1 participant
Summary
Fixes LibrePods to launch and run on non-rooted OnePlus 12 (OxygenOS 16 / Android 16, SDK 36) where L2CAP works natively on the QTI Bluetooth stack without root hooks.
Device: OnePlus CPH2583, Android 16, OxygenOS 16.0.0.202, non-rooted
Changes
isOxygenOSOrColorOS16OrAbove()detects OnePlus/OPPO/Realme on SDK 36+ and returnstruefromisHookOffsetAvailable()/isSdpOffsetAvailable(), bypassing root/radare2 setupstartForegroundService()beforebindService()(wrapped inLaunchedEffect) so the service survives activity backgroundingstartForegroundNotification()to first call aftersuper.onCreate()to preventForegroundServiceDidNotStartInTimeExceptionattemptL2capReconnect()inonStartCommand()with BT adapter check, MAC validation, and service-scoped coroutine (SupervisorJob)localMacis empty (can't determine if audio source is local or remote)connectionStatusReceiverwith::isInitializedbefore unregisteringsetBatteryMetadata()on OxygenOS 16+ and whenskip_setupis setKnown limitation: self MAC address
On SDK 36 with
targetSdkVersion=36, the local Bluetooth MAC address cannot be retrieved programmatically without root:Settings.Secure.getString("bluetooth_address")→ blocked for targetSdk > 31settings get secure bluetooth_addressshell command → requiresINTERACT_ACROSS_USERSsu→ not available on non-rooted devicesWorkaround: Inject via ADB after install:
Without the MAC, the app adds a
localMac.isNotEmpty()guard to avoid surrendering AACP control, so ANC switching still works. Multi-device handoff (TiPi packets) will be degraded.Pre-existing issues found during testing (not addressed in this PR)
ConcurrentModificationExceptioninAACPManager.receivePacket()(AACPManager.kt:463) — ArrayList iterated while modified from another thread, causes crashIntentReceiverLeakedinNoiseControlSettings.kt:178— BroadcastReceiver registered but never unregisterednamepref shows generic "AirPods" instead of usingairpods_name("Teng's AirPods Pro 2")Test plan
startForegroundNotification()completes before timeoutUninitializedPropertyAccessExceptionBLUETOOTH_PRIVILEGEDspam🤖 Generated with Claude Code