diff --git a/core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepositoryBondTest.kt b/core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepositoryBondTest.kt index b3a6dd2d9..a0eb3fe4b 100644 --- a/core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepositoryBondTest.kt +++ b/core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepositoryBondTest.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runTest import org.junit.runner.RunWith import org.meshtastic.core.di.CoroutineDispatchers @@ -124,15 +125,18 @@ class AndroidBluetoothRepositoryBondTest { @Test @Config(sdk = [34], shadows = [ShadowBondingThenBonded::class]) - fun `createBond false but bond already established resumes immediately`() = runTest(UnconfinedTestDispatcher()) { - // Models the real-device race the fix targets: the bond completes between the early bondState check and - // the post-createBond re-check. The custom shadow returns BONDING first, then BONDED, with - // createBond()==false — driving the BOND_BONDED branch without any broadcast. + fun `post receiver registration bond recheck resumes without createBond`() = runTest(UnconfinedTestDispatcher()) { + // Models the real-device race where the bond completes between the early bondState check and receiver + // registration. The custom shadow returns BONDING first, then BONDED, so the registered receiver is cleaned + // up before createBond() is called. val mac = "AA:BB:CC:DD:EE:03" + RobolectricBleBonding.grantBluetoothConnectPermission() + ShadowBondingThenBonded.createBondCalls = 0 val repo = newRepository(UnconfinedTestDispatcher(testScheduler)) - // Resumes (no broadcast) via the post-createBond BOND_BONDED branch; assert no error surfaced. + // Resumes (no broadcast) via the post-registration BOND_BONDED re-check; assert no error surfaced. assertNull(launchBond(repo, mac).await(), "bond() should resume when the re-check finds BOND_BONDED") + assertEquals(0, ShadowBondingThenBonded.createBondCalls, "createBond() should not run after the re-check") } @Test @@ -213,6 +217,64 @@ class AndroidBluetoothRepositoryBondTest { assertNull(failure.await()) } + @Test + fun `bond times out when no terminal bond broadcast arrives`() = runTest(UnconfinedTestDispatcher()) { + val mac = "AA:BB:CC:DD:EE:0A" + RobolectricBleBonding.grantBluetoothConnectPermission() + RobolectricBleBonding.primeBond(mac, bondState = BluetoothDevice.BOND_BONDING, createBondReturns = false) + val repo = newRepository(UnconfinedTestDispatcher(testScheduler)) + + val failure = launchBond(repo, mac) + advanceTimeBy(29_999L) + assertFalse(failure.isCompleted, "bond() should still be waiting before the timeout") + advanceTimeBy(2L) + + assertEquals("Timed out waiting for bonding to complete", failure.await()?.message) + } + + @Test + fun `bond completes early when bond state becomes bonded without broadcast`() = + runTest(UnconfinedTestDispatcher()) { + val mac = "AA:BB:CC:DD:EE:0B" + RobolectricBleBonding.grantBluetoothConnectPermission() + val deviceShadow = + RobolectricBleBonding.primeBond( + mac, + bondState = BluetoothDevice.BOND_BONDING, + createBondReturns = false, + ) + val repo = newRepository(UnconfinedTestDispatcher(testScheduler)) + + val failure = launchBond(repo, mac) + advanceTimeBy(499L) + assertFalse(failure.isCompleted, "bond() should still be waiting before the poll interval") + deviceShadow.setBondState(BluetoothDevice.BOND_BONDED) + advanceTimeBy(2L) + + assertNull(failure.await(), "bond() should accept the polled BONDED state before timeout") + } + + @Test + fun `bond succeeds when bond state becomes bonded at timeout boundary`() = runTest(UnconfinedTestDispatcher()) { + val mac = "AA:BB:CC:DD:EE:0C" + RobolectricBleBonding.grantBluetoothConnectPermission() + val deviceShadow = + RobolectricBleBonding.primeBond( + mac, + bondState = BluetoothDevice.BOND_BONDING, + createBondReturns = false, + ) + val repo = newRepository(UnconfinedTestDispatcher(testScheduler)) + + val failure = launchBond(repo, mac) + advanceTimeBy(29_999L) + assertFalse(failure.isCompleted, "bond() should still be waiting before the timeout") + deviceShadow.setBondState(BluetoothDevice.BOND_BONDED) + advanceTimeBy(2L) + + assertNull(failure.await(), "bond() should accept the final BONDED state after timeout") + } + @Test fun `isBonded reflects the adapter bonded devices`() = runTest(UnconfinedTestDispatcher()) { val bondedMac = "AA:BB:CC:DD:EE:06" @@ -236,17 +298,25 @@ class AndroidBluetoothRepositoryBondTest { /** * Custom shadow that returns [BluetoothDevice.BOND_BONDING] on the first `getBondState()` read (the early guard) - * and [BluetoothDevice.BOND_BONDED] thereafter (the post-`createBond` re-check), with `createBond()` returning - * false — reproducing the bond-completed-mid-method race for the BOND_BONDED branch of the fix. + * and [BluetoothDevice.BOND_BONDED] thereafter (the post-receiver-registration re-check), reproducing the + * bond-completed-mid-method race without a broadcast. */ @Implements(BluetoothDevice::class) class ShadowBondingThenBonded : ShadowBluetoothDevice() { + companion object { + var createBondCalls: Int = 0 + } + private var bondStateReads = 0 @Implementation override fun getBondState(): Int = if (bondStateReads++ == 0) BluetoothDevice.BOND_BONDING else BluetoothDevice.BOND_BONDED - @Implementation override fun createBond(): Boolean = false + @Implementation + override fun createBond(): Boolean { + createBondCalls++ + return false + } } } diff --git a/core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepository.kt b/core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepository.kt index 57b327361..2f7e623fa 100644 --- a/core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepository.kt +++ b/core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBluetoothRepository.kt @@ -27,15 +27,20 @@ import androidx.core.content.ContextCompat import androidx.lifecycle.Lifecycle import androidx.lifecycle.coroutineScope import co.touchlab.kermit.Logger +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch -import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withTimeoutOrNull import org.koin.core.annotation.Named import org.koin.core.annotation.Single import org.meshtastic.core.di.CoroutineDispatchers -import kotlin.coroutines.resume +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds + +private val BOND_TIMEOUT = 30.seconds +private val BOND_STATE_POLL_INTERVAL = 500.milliseconds /** Android implementation of [BluetoothRepository]. */ @Single @@ -87,89 +92,130 @@ class AndroidBluetoothRepository( return } - suspendCancellableCoroutine { cont -> - val receiver = - object : android.content.BroadcastReceiver() { - @SuppressLint("MissingPermission") - override fun onReceive(c: Context, intent: android.content.Intent) { - if (intent.action == android.bluetooth.BluetoothDevice.ACTION_BOND_STATE_CHANGED) { - val d = - androidx.core.content.IntentCompat.getParcelableExtra( - intent, - android.bluetooth.BluetoothDevice.EXTRA_DEVICE, - android.bluetooth.BluetoothDevice::class.java, - ) - if (d?.address?.equals(macAddress, ignoreCase = true) == true) { - val state = - intent.getIntExtra( - android.bluetooth.BluetoothDevice.EXTRA_BOND_STATE, - android.bluetooth.BluetoothDevice.ERROR, - ) - val prevState = - intent.getIntExtra( - android.bluetooth.BluetoothDevice.EXTRA_PREVIOUS_BOND_STATE, - android.bluetooth.BluetoothDevice.ERROR, - ) + try { + val bonded = + withTimeoutOrNull(BOND_TIMEOUT) { + val result = CompletableDeferred() + val receiver = createBondReceiver(macAddress, result) - if (state == android.bluetooth.BluetoothDevice.BOND_BONDED) { - try { - context.unregisterReceiver(this) - } catch (ignored: Exception) {} - if (cont.isActive) cont.resume(Unit) - } else if ( - state == android.bluetooth.BluetoothDevice.BOND_NONE && - prevState == android.bluetooth.BluetoothDevice.BOND_BONDING - ) { - try { - context.unregisterReceiver(this) - } catch (ignored: Exception) {} - if (cont.isActive) { - cont.resumeWith(Result.failure(Exception("Bonding failed or rejected"))) - } - } - } - } + val filter = + android.content.IntentFilter(android.bluetooth.BluetoothDevice.ACTION_BOND_STATE_CHANGED) + ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED) + + try { + startOrObserveBond(remoteDevice, result) + awaitBondResult(remoteDevice, result) + } finally { + unregisterBondReceiver(receiver) } - } + // Reaching here means the suspended bond wait completed before BOND_TIMEOUT. + true + } ?: (remoteDevice.bondState == android.bluetooth.BluetoothDevice.BOND_BONDED) - val filter = android.content.IntentFilter(android.bluetooth.BluetoothDevice.ACTION_BOND_STATE_CHANGED) - ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED) - - cont.invokeOnCancellation { - try { - context.unregisterReceiver(receiver) - } catch (ignored: Exception) {} + if (!bonded) { + throw Exception("Timed out waiting for bonding to complete") } + } finally { + updateBluetoothState() + } + } - if (!remoteDevice.createBond()) { - // createBond() returns false when a bond is already in flight (initiated by the OS or - // triggered by a GATT operation hitting a secured characteristic) or already established. - // The ACTION_BOND_STATE_CHANGED broadcast is unreliable on some devices (see Kable #111), - // so re-check bondState directly rather than failing the whole flow. + @Suppress("TooGenericExceptionCaught") + @SuppressLint("MissingPermission") + private fun startOrObserveBond(remoteDevice: android.bluetooth.BluetoothDevice, result: CompletableDeferred) { + try { + if (result.isCompleted) return + + if (remoteDevice.bondState == android.bluetooth.BluetoothDevice.BOND_BONDED) { + result.complete(Unit) + } else if (!remoteDevice.createBond()) { + // createBond() returns false when a bond is already in flight, triggered by a GATT + // operation hitting a secured characteristic, or already established. + // ACTION_BOND_STATE_CHANGED is unreliable on some devices (see Kable #111), so + // re-check bondState directly rather than failing the whole flow. when (remoteDevice.bondState) { android.bluetooth.BluetoothDevice.BOND_BONDED -> { - try { - context.unregisterReceiver(receiver) - } catch (ignored: Exception) {} - if (cont.isActive) cont.resume(Unit) + result.complete(Unit) } android.bluetooth.BluetoothDevice.BOND_BONDING -> { - // Bond already in progress; leave the receiver registered to resolve it on the - // terminal BOND_BONDED / BOND_NONE transition instead of treating this as a failure. + // Bond already in progress; leave the receiver registered to resolve it on + // the terminal BOND_BONDED / BOND_NONE transition instead of treating this + // as a failure. Logger.d { "createBond() returned false but bonding is already in progress" } } else -> { - try { - context.unregisterReceiver(receiver) - } catch (ignored: Exception) {} - if (cont.isActive) cont.resumeWith(Result.failure(Exception("Failed to initiate bonding"))) + result.completeExceptionally(Exception("Failed to initiate bonding")) } } } + } catch (e: Exception) { + result.completeExceptionally(e) } - updateBluetoothState() + } + + @SuppressLint("MissingPermission") + private suspend fun awaitBondResult( + remoteDevice: android.bluetooth.BluetoothDevice, + result: CompletableDeferred, + ) { + while (!result.isCompleted) { + val completedFromReceiver = + withTimeoutOrNull(BOND_STATE_POLL_INTERVAL) { + result.await() + true + } == true + + if (!completedFromReceiver && remoteDevice.bondState == android.bluetooth.BluetoothDevice.BOND_BONDED) { + result.complete(Unit) + } + } + result.await() + } + + @Suppress("TooGenericExceptionThrown") + private fun createBondReceiver( + macAddress: String, + result: CompletableDeferred, + ): android.content.BroadcastReceiver = object : android.content.BroadcastReceiver() { + override fun onReceive(c: Context, intent: android.content.Intent) { + if (intent.action != android.bluetooth.BluetoothDevice.ACTION_BOND_STATE_CHANGED) return + val d = + androidx.core.content.IntentCompat.getParcelableExtra( + intent, + android.bluetooth.BluetoothDevice.EXTRA_DEVICE, + android.bluetooth.BluetoothDevice::class.java, + ) + if (d?.address?.equals(macAddress, ignoreCase = true) != true) return + + val state = + intent.getIntExtra( + android.bluetooth.BluetoothDevice.EXTRA_BOND_STATE, + android.bluetooth.BluetoothDevice.ERROR, + ) + val prevState = + intent.getIntExtra( + android.bluetooth.BluetoothDevice.EXTRA_PREVIOUS_BOND_STATE, + android.bluetooth.BluetoothDevice.ERROR, + ) + + if (state == android.bluetooth.BluetoothDevice.BOND_BONDED) { + result.complete(Unit) + } else if ( + state == android.bluetooth.BluetoothDevice.BOND_NONE && + prevState == android.bluetooth.BluetoothDevice.BOND_BONDING + ) { + result.completeExceptionally(Exception("Bonding failed or rejected")) + } + } + } + + @Suppress("TooGenericExceptionCaught", "SwallowedException") + private fun unregisterBondReceiver(receiver: android.content.BroadcastReceiver) { + try { + context.unregisterReceiver(receiver) + } catch (ignored: Exception) {} } internal suspend fun updateBluetoothState() {