fix: Address Codex review on auto-monitor-mode

- Seed connectedDevices flow with onStart so ALWAYS autolaunch fires before HEADSET profile binds

- Gate UnavailableMissingPermission classification on Android 12+ (BLUETOOTH_CONNECT only exists from API 31)

- Treat blank/empty profile.address as unpaired (matches AutoConnect)

- Subscribe DeviceSettingsViewModel to nudgeCapabilityStore.availability so UI updates immediately on verdict change

- Seed availability StateFlow with the persisted value via valueBlocking to close the cold-start race
This commit is contained in:
Matthias Urhahn
2026-05-07 14:10:16 +02:00
committed by Matthias Urhahn
parent 1eeeb63a76
commit 0e6848d886
6 changed files with 47 additions and 8 deletions
@@ -10,6 +10,7 @@ import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.os.Build
import android.os.Handler
import android.os.HandlerThread
import dagger.hilt.android.qualifiers.ApplicationContext
@@ -443,7 +444,13 @@ class BluetoothManager2 @Inject constructor(
Bugs.report(tag = TAG, "BluetoothHeadset.connect(device) is unavailable", exception = e)
return@map NudgeAttemptResult.Rejected
}
if (!Permission.BLUETOOTH_CONNECT.isGranted(context)) {
// BLUETOOTH_CONNECT is only a runtime permission on Android 12+. On older
// versions a SecurityException can't be a missing-permission failure for that
// permission — it's hidden-API enforcement (e.g. MODIFY_PHONE_STATE) and we
// should persist BROKEN.
val missingBtConnect = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S &&
!Permission.BLUETOOTH_CONNECT.isGranted(context)
if (missingBtConnect) {
log(TAG, WARN) { "nudgeConnection failed because BLUETOOTH_CONNECT is not granted" }
NudgeAttemptResult.UnavailableMissingPermission
} else {
@@ -4,6 +4,7 @@ import eu.darken.capod.common.coroutine.AppScope
import eu.darken.capod.common.coroutine.DispatcherProvider
import eu.darken.capod.common.datastore.DataStoreValue
import eu.darken.capod.common.datastore.value
import eu.darken.capod.common.datastore.valueBlocking
import eu.darken.capod.common.debug.logging.Logging.Priority.INFO
import eu.darken.capod.common.debug.logging.log
import eu.darken.capod.common.debug.logging.logTag
@@ -23,11 +24,15 @@ class NudgeCapabilityStore @Inject constructor(
private val dispatcherProvider: DispatcherProvider,
) {
// Resolve the persisted value synchronously at construction so consumers that read
// `availability.value` (resolver, AutoConnect precondition, UI state) never see the
// synthetic UNKNOWN seed before the first DataStore emission arrives. On a known-broken
// device this prevents one bonus ALWAYS-mode + nudge-attempt cycle per cold start.
val availability: StateFlow<NudgeAvailability> = persistedValue.flow
.stateIn(
scope = appScope + dispatcherProvider.IO,
started = SharingStarted.Eagerly,
initialValue = NudgeAvailability.UNKNOWN,
initialValue = persistedValue.valueBlocking,
)
fun record(result: NudgeAttemptResult) {
@@ -119,6 +119,7 @@ class DeviceSettingsViewModel @Inject constructor(
bluetoothManager.connectedDevices.onStart { emit(emptyList()) },
monitorModeResolver.effectiveMode,
profilesRepo.profiles,
nudgeCapabilityStore.availability,
) { args ->
val device = args[1] as PodDevice?
val upgrade = args[2] as UpgradeRepo.Info
@@ -130,6 +131,7 @@ class DeviceSettingsViewModel @Inject constructor(
@Suppress("UNCHECKED_CAST")
val profiles = args[6] as List<eu.darken.capod.profiles.core.DeviceProfile>
val nudgeAvailability = args[7] as NudgeAvailability
val stemActions = profiles.filterIsInstance<AppleDeviceProfile>()
.firstOrNull { it.id == profileId }
?.stemActions
@@ -145,7 +147,7 @@ class DeviceSettingsViewModel @Inject constructor(
device = device,
now = timeSource.now(),
isPro = upgrade.isPro,
isNudgeAvailable = nudgeCapabilityStore.availability.value != NudgeAvailability.BROKEN,
isNudgeAvailable = nudgeAvailability != NudgeAvailability.BROKEN,
isForceConnecting = forcing,
isClassicallyConnected = device?.address?.let { it in connectedAddresses } == true,
monitorMode = monitorMode,
@@ -34,9 +34,12 @@ import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.channelFlow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.isActive
import java.time.Instant
import javax.inject.Inject
@@ -79,16 +82,23 @@ class OverviewViewModel @Inject constructor(
val workerAutolaunch = combine(
permissionTool.missingScanPermissions,
monitorModeResolver.effectiveMode,
bluetoothManager.connectedDevices,
) { missing, mode, connected -> Triple(missing, mode, connected) }
.onEach { (missing, mode, connected) ->
// BluetoothManager2.connectedDevices is gated by a slow HEADSET-profile lookup. Without
// an onStart seed the combine wouldn't fire until that profile is bound, so ALWAYS mode
// would silently delay autolaunch until the first connected-device emission.
bluetoothManager.connectedDevices
.onStart { emit(emptyList()) }
.map { it.isNotEmpty() }
.distinctUntilChanged(),
) { missing, mode, anyConnected -> Triple(missing, mode, anyConnected) }
.distinctUntilChanged()
.onEach { (missing, mode, anyConnected) ->
if (missing.isNotEmpty()) {
log(TAG) { "Missing scan permissions: $missing" }
return@onEach
}
val shouldStart = when (mode) {
MonitorMode.MANUAL -> false
MonitorMode.AUTOMATIC -> connected.isNotEmpty()
MonitorMode.AUTOMATIC -> anyConnected
MonitorMode.ALWAYS -> true
}
if (shouldStart) {
@@ -33,7 +33,8 @@ class MonitorModeResolver @Inject constructor(
.onEach { log(TAG, VERBOSE) { "effectiveMode = $it" } }
private fun DeviceProfile.requiredMode(nudge: NudgeAvailability): MonitorMode = when {
address == null -> MonitorMode.MANUAL
// Match AutoConnect.kt's isNullOrEmpty check — a blank/legacy "" address is no address.
address.isNullOrBlank() -> MonitorMode.MANUAL
toReactionConfig().autoConnect && nudge != NudgeAvailability.BROKEN -> MonitorMode.ALWAYS
else -> MonitorMode.AUTOMATIC
}
@@ -85,6 +85,20 @@ class MonitorModeResolverTest : BaseTest() {
resolver.effectiveMode.first() shouldBe MonitorMode.MANUAL
}
@Test
fun `case 4 variant - profile with blank address is treated as unpaired`() = runTest {
profilesFlow.value = listOf(profile(address = "", autoConnect = true))
resolver.effectiveMode.first() shouldBe MonitorMode.MANUAL
}
@Test
fun `case 4 variant - profile with whitespace address is treated as unpaired`() = runTest {
profilesFlow.value = listOf(profile(address = " ", autoConnect = true))
resolver.effectiveMode.first() shouldBe MonitorMode.MANUAL
}
@Test
fun `multi-profile primary-only - primary case 2 + secondary case 3 - AUTOMATIC`() = runTest {
profilesFlow.value = listOf(