feat(troubleshooter): Persist only the minimal working compat combo

Probe compatibility options through a transient in-memory override on BlePodMonitor instead of writing the user's persisted settings on every attempt. Only the winning combo is persisted, and only on success; failure or cancellation clears the override, restoring the user's original settings. Combos are tried fewest-disables-first so a phone that only needs batching disabled isn't left with filtering disabled too. Per-attempt cache clearing plus a freshness cutoff stop a previous combo's cached devices from satisfying the next one, and the 'found' checks now require a fresh live BLE observation rather than cached/AAP state.
2026-09-14 18:26:11 -04:00 · 2026-06-08 07:45:29 +02:00
parent 20a6953b54
commit df2783d8a1
3 changed files with 293 additions and 169 deletions
@@ -59,6 +59,17 @@ class BlePodMonitor @Inject constructor(
    private val deviceCache = mutableMapOf<BlePodSnapshot.Id, BlePodSnapshot>()
    private val cacheLock = Mutex()

+    /**
+     * Drops all cached observations. The troubleshooter calls this between probe attempts so a
+     * previous compat combo's cached devices (kept up to [STALE_DEVICE_TIMEOUT]) can't leak into
+     * the next attempt and falsely satisfy it — including via [preferCaseContextPod], which would
+     * otherwise hand back a stale snapshot whose timestamp predates the fresh scan.
+     */
+    suspend fun clearDeviceCache() = cacheLock.withLock {
+        log(TAG) { "clearDeviceCache()" }
+        deviceCache.clear()
+    }
+
    /**
     * Ephemeral override that disables the proximity-pairing scan filter so
     * the troubleshooter can collect raw BLE broadcasts. Resets to false on
@@ -70,6 +81,40 @@ class BlePodMonitor @Inject constructor(
        unfilteredOverride.value = enabled
    }

+    /**
+     * Ephemeral override for the three BLE compatibility options. The troubleshooter uses this to
+     * probe combinations without writing the user's persisted settings: while set, it fully replaces
+     * the persisted compat values for the active scan. Resets to null on every process start; the
+     * troubleshooter is the only writer and always clears it when finished, so clearing it restores
+     * the user's original settings for free.
+     */
+    private val compatOverride = MutableStateFlow<CompatOverride?>(null)
+
+    fun setCompatOverride(override: CompatOverride?) {
+        log(TAG) { "setCompatOverride($override)" }
+        compatOverride.value = override
+    }
+
+    data class CompatOverride(
+        val offloadedFilteringDisabled: Boolean,
+        val offloadedBatchingDisabled: Boolean,
+        val indirectCallback: Boolean,
+    )
+
+    /** Persisted compat settings, transparently replaced by [compatOverride] while it is set. */
+    private val effectiveCompat: Flow<CompatOverride> = combine(
+        compatOverride,
+        generalSettings.isOffloadedFilteringDisabled.flow,
+        generalSettings.isOffloadedBatchingDisabled.flow,
+        generalSettings.useIndirectScanResultCallback.flow,
+    ) { override, filteringDisabled, batchingDisabled, indirectCallback ->
+        override ?: CompatOverride(
+            offloadedFilteringDisabled = filteringDisabled,
+            offloadedBatchingDisabled = batchingDisabled,
+            indirectCallback = indirectCallback,
+        )
+    }
+
    val devices: Flow<List<BlePodSnapshot>> = combine(
        permissionTool.missingScanPermissions,
        bluetoothManager.isBluetoothEnabled
@@ -145,22 +190,14 @@ class BlePodMonitor @Inject constructor(
    private fun createBleScanner() = combine(
        bleScanModeController.scannerMode,
        unfilteredOverride,
-        generalSettings.isOffloadedBatchingDisabled.flow,
-        generalSettings.isOffloadedFilteringDisabled.flow,
-        generalSettings.useIndirectScanResultCallback.flow,
-    ) {
-            scannermode,
-            showUnfiltered,
-            isOffloadedBatchingDisabled,
-            isOffloadedFilteringDisabled,
-            useIndirectScanResultCallback,
-        ->
+        effectiveCompat,
+    ) { scannermode, showUnfiltered, compat ->
        ScannerOptions(
            scannerMode = scannermode,
            showUnfiltered = showUnfiltered,
-            offloadedFilteringDisabled = isOffloadedFilteringDisabled,
-            offloadedBatchingDisabled = isOffloadedBatchingDisabled,
-            disableDirectCallback = useIndirectScanResultCallback,
+            offloadedFilteringDisabled = compat.offloadedFilteringDisabled,
+            offloadedBatchingDisabled = compat.offloadedBatchingDisabled,
+            disableDirectCallback = compat.indirectCallback,
        )
    }
        .throttleLatest(1000)
@@ -9,6 +9,7 @@ import eu.darken.capod.common.bluetooth.ScannerMode
 import eu.darken.capod.common.coroutine.DispatcherProvider
 import eu.darken.capod.common.datastore.valueBlocking
 import eu.darken.capod.common.debug.logging.Logging.Priority.INFO
+import eu.darken.capod.common.debug.logging.Logging.Priority.WARN
 import eu.darken.capod.common.debug.logging.log
 import eu.darken.capod.common.debug.logging.logTag
 import eu.darken.capod.common.uix.ViewModel4
@@ -22,14 +23,13 @@ import eu.darken.capod.pods.core.unknown.UnknownSnapshotBle
 import eu.darken.capod.profiles.core.AppleDeviceProfile
 import eu.darken.capod.profiles.core.DeviceProfilesRepo
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.filterNotNull
+import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
-import kotlinx.coroutines.flow.take
 import kotlinx.coroutines.flow.takeWhile
-import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.withTimeoutOrNull
 import javax.inject.Inject

@@ -47,6 +47,9 @@ class TroubleShooterViewModel @Inject constructor(

    private val _bleState = MutableStateFlow<BleState>(BleState.Intro())

+    /** Guards against re-entrant runs (e.g. a double tap on "Try again"). */
+    private val runLock = Mutex()
+
    data class State(val bleState: BleState)

    val state = _bleState.map { State(it) }.asLiveState()
@@ -82,169 +85,184 @@ class TroubleShooterViewModel @Inject constructor(
    }

    fun troubleShootBle() = launch(context = dispatcherProvider.IO) {
+        if (!runLock.tryLock()) {
+            log(TAG, WARN) { "troubleShootBle() ignored, a run is already in progress" }
+            return@launch
+        }
        log(TAG, INFO) { "troubleShootBle()" }

-        bleScanModeController.withTemporaryOverride(ScannerMode.LOW_LATENCY) override@{
-            try {
-            run {
-                progress("Checking for headphones...")
-                val mainDevice = withTimeoutOrNull(STEP_TIME) {
-                    deviceMonitor.primaryDevice().filterNotNull().firstOrNull()
-                }
-                if (mainDevice != null) {
-                    success("Headphones found, nothing to troubleshoot.")
-                    return@override
-                } else {
-                    progress("Headphones not detected.\n")
-                }
-            }
+        try {
+            bleScanModeController.withTemporaryOverride(ScannerMode.LOW_LATENCY) override@{
+                // The combo under which supported headphones were detected (set in sweep 2).
+                var supportedCombo: BlePodMonitor.CompatOverride? = null
+                // Set only when the run reaches a terminal success. Persisted on exit; staying null
+                // (any failure, or cancellation) means we just clear the override, which restores the
+                // user's original — never-touched — settings.
+                var comboToPersist: BlePodMonitor.CompatOverride? = null
+                try {
+                    run {
+                        progress("Checking for headphones...")
+                        if (findLiveBleHeadphones()) {
+                            success("Headphones found, nothing to troubleshoot.")
+                            return@override
+                        } else {
+                            progress("Headphones not detected.\n")
+                        }
+                    }

-            val doScan: suspend (Boolean, Boolean, Boolean, Boolean) -> Collection<BlePodSnapshot> =
-                { hardwareFilteringDisabled,
-                  hardwareBatchingDisabled,
-                  indirectCallback,
-                  unfiltered ->
-                    val sb = StringBuilder("SCAN - Settings: ")
-                    sb.append("hardwareFilteringDisabled=$hardwareFilteringDisabled, ")
-                    sb.append("hardwareBatchingDisabled=$hardwareBatchingDisabled, ")
-                    sb.append("indirectCallback=$indirectCallback, ")
-                    sb.append("unfiltered=$unfiltered")
-                    progress(sb.toString())
-                    generalSettings.isOffloadedFilteringDisabled.valueBlocking = hardwareFilteringDisabled
-                    generalSettings.isOffloadedBatchingDisabled.valueBlocking = hardwareBatchingDisabled
-                    generalSettings.useIndirectScanResultCallback.valueBlocking = indirectCallback
-                    blePodMonitor.setUnfilteredOverride(unfiltered)
+                    val doScan: suspend (BlePodMonitor.CompatOverride, Boolean) -> Collection<BlePodSnapshot> =
+                        { combo, unfiltered ->
+                            progress(
+                                "SCAN - Settings: filteringDisabled=${combo.offloadedFilteringDisabled}, " +
+                                    "batchingDisabled=${combo.offloadedBatchingDisabled}, " +
+                                    "indirectCallback=${combo.indirectCallback}, unfiltered=$unfiltered"
+                            )
+                            blePodMonitor.setCompatOverride(combo)
+                            blePodMonitor.setUnfilteredOverride(unfiltered)
+                            val devices = collectFreshDevices()
+                            log(TAG) { "SCAN: Fresh BLE devices: $devices" }
+                            if (devices.isNotEmpty()) {
+                                progress("SCAN: Received data from ${devices.size} BLE devices")
+                            } else {
+                                progress("SCAN: No data received")
+                            }
+                            devices
+                        }

-                    val start = timeSource.elapsedRealtime()
-                    val devices = withTimeoutOrNull(STEP_TIME) {
-                        blePodMonitor.devices
-                            .take(10)
-                            .takeWhile { timeSource.elapsedRealtime() - start < STEP_TIME - 1000 }
-                            .toList()
-                            .flatten()
-                            .distinctBy { it.address }
-                    } ?: emptyList()
-                    log(TAG) { "SCAN: BLE Devices: $devices" }
-                    if (devices.isNotEmpty()) {
-                        progress("SCAN: Received data from ${devices.size} BLE devices")
-                        devices
-                    } else {
-                        progress("SCAN: No data received")
-                        devices
+                    run {
+                        progress("Checking if we can receive BLE data at all.")
+                        val gotData = COMPAT_COMBOS.any { combo -> doScan(combo, true).isNotEmpty() }
+                        if (!gotData) {
+                            failure("Phone is not receiving BLE data.", BleState.Result.Failure.Type.PHONE)
+                            return@override
+                        }
+                    }
+
+                    progress("We received at least some BLE data.\n")
+
+                    run {
+                        progress("Checking for supported headphones.")
+                        supportedCombo = COMPAT_COMBOS.firstOrNull { combo ->
+                            doScan(combo, false).any { it !is UnknownSnapshotBle }
+                        }
+                        if (supportedCombo == null) {
+                            failure("No compatible headphones found", BleState.Result.Failure.Type.HEADPHONES)
+                            return@override
+                        }
+                    }
+
+                    progress("Found some headphones that are supported by CAPod.\n")
+
+                    run {
+                        progress("Checking for your headphones with new BLE settings...")
+                        if (findLiveBleHeadphones()) {
+                            comboToPersist = supportedCombo
+                            success("Found your headphones, new BLE settings worked :)!")
+                            return@override
+                        }
+                    }
+
+                    progress("Still no headphones detected that count as yours.\n")
+
+                    run {
+                        progress("Checking all closeby headphones.")
+
+                        val otherDevices = collectFreshDevices()
+                        otherDevices.forEachIndexed { index, dev -> log(TAG) { "Device #$index: $dev" } }
+
+                        val candidate = otherDevices
+                            .filter { it !is UnknownSnapshotBle }
+                            .maxByOrNull { it.signalQuality }
+
+                        if (candidate == null) {
+                            failure(
+                                "No supported headphones found near your device.",
+                                BleState.Result.Failure.Type.HEADPHONES,
+                            )
+                            return@override
+                        }
+
+                        progress("Headphones found nearby, but not detected as yours.\n")
+                        progress("Creating profile for closest headphones.")
+                        log(TAG, INFO) { "Candidate is $candidate" }
+
+                        profilesRepo.addProfile(
+                            profile = AppleDeviceProfile(
+                                label = context.getString(R.string.troubleshooter_title),
+                                model = candidate.model,
+                            ),
+                            addFirst = true,
+                        )
+
+                        if (findLiveBleHeadphones()) {
+                            comboToPersist = supportedCombo
+                            success("Success! Detected your headphones.")
+                        } else {
+                            failure("No headphones detected near your device.", BleState.Result.Failure.Type.HEADPHONES)
+                        }
+                    }
+                } finally {
+                    blePodMonitor.setUnfilteredOverride(false)
+                    try {
+                        // Persist the winning combo before dropping the override so the effective scan
+                        // settings stay equal with no restart flicker. Done in its own try so the
+                        // override is still cleared (restoring originals) even if a write throws.
+                        comboToPersist?.let { persistCompat(it) }
+                    } finally {
+                        blePodMonitor.setCompatOverride(null)
                    }
                }
-
-            run {
-                progress("Checking if we can receive BLE data at all.")
-                if (doScan(false, false, false, true).isNotEmpty()) return@run
-                if (doScan(false, false, true, true).isNotEmpty()) return@run
-                if (doScan(true, true, true, true).isNotEmpty()) return@run
-                if (doScan(true, true, false, true).isNotEmpty()) return@run
-                if (doScan(true, false, true, true).isNotEmpty()) return@run
-                if (doScan(true, false, false, true).isNotEmpty()) return@run
-                if (doScan(false, true, true, true).isNotEmpty()) return@run
-                if (doScan(false, true, false, true).isNotEmpty()) return@run
-
-                failure("Phone is not receiving BLE data.", BleState.Result.Failure.Type.PHONE)
-
-                generalSettings.isOffloadedFilteringDisabled.valueBlocking = false
-                generalSettings.isOffloadedBatchingDisabled.valueBlocking = false
-                generalSettings.useIndirectScanResultCallback.valueBlocking = false
-
-                return@override
-            }
-
-            progress("We received at least some BLE data.\n")
-
-            run {
-                progress("Checking for supported headphones.")
-
-                if (doScan(false, false, false, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(false, false, true, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(true, true, true, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(true, true, false, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(true, false, true, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(true, false, false, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(false, true, true, false).any { it !is UnknownSnapshotBle }) return@run
-                if (doScan(false, true, false, false).any { it !is UnknownSnapshotBle }) return@run
-
-                failure("No compatible headphones found", BleState.Result.Failure.Type.HEADPHONES)
-
-                generalSettings.isOffloadedFilteringDisabled.valueBlocking = false
-                generalSettings.isOffloadedBatchingDisabled.valueBlocking = false
-                generalSettings.useIndirectScanResultCallback.valueBlocking = false
-
-                return@override
-            }
-
-            progress("Found some headphones that are supported by CAPod.\n")
-
-            run {
-                progress("Checking for your headphones with new BLE settings...")
-                val mainDevice = withTimeoutOrNull(STEP_TIME) {
-                    deviceMonitor.primaryDevice().filterNotNull().firstOrNull()
-                }
-                if (mainDevice != null) {
-                    success("Found your headphones, new BLE settings worked :)!")
-                    return@override
-                }
-            }
-
-            progress("Still no headphones detected that count as yours.\n")
-
-            run {
-                progress("Checking all closeby headphones.")
-
-                val otherDevices = withTimeoutOrNull(STEP_TIME) {
-                    val start = timeSource.elapsedRealtime()
-                    blePodMonitor.devices
-                        .take(10)
-                        .takeWhile { timeSource.elapsedRealtime() - start < STEP_TIME - 1000 }
-                        .toList()
-                        .flatten()
-                        .distinctBy { it.address }
-                } ?: emptyList()
-
-                otherDevices.forEachIndexed { index, dev -> log(TAG) { "Device #$index: $dev" } }
-
-                if (otherDevices.isEmpty()) {
-                    failure("No supported headphones found near your device.", BleState.Result.Failure.Type.HEADPHONES)
-                    return@override
-                }
-
-                progress("Headphones found nearby, but not detected as yours.\n")
-                progress("Creating profile for closest headphones.")
-
-                val candidate = otherDevices
-                    .filter { it !is UnknownSnapshotBle }
-                    .maxBy { it.signalQuality }
-
-                log(TAG, INFO) { "Candidate is $candidate" }
-
-                profilesRepo.addProfile(
-                    profile = AppleDeviceProfile(
-                        label = context.getString(R.string.troubleshooter_title),
-                        model = candidate.model,
-                    ),
-                    addFirst = true,
-                )
-
-                val mainDevice = withTimeoutOrNull(STEP_TIME) {
-                    deviceMonitor.primaryDevice().filterNotNull().firstOrNull()
-                }
-
-                if (mainDevice != null) {
-                    success("Success! Detected your headphones.")
-                } else {
-                    failure("No headphones detected near your device.", BleState.Result.Failure.Type.HEADPHONES)
-                }
-            }
-            } finally {
-                blePodMonitor.setUnfilteredOverride(false)
            }
+        } finally {
+            runLock.unlock()
        }
    }

+    /**
+     * Waits up to [STEP_TIME] for the primary profile to be backed by a *fresh, live BLE*
+     * observation. A cached-only / AAP-only primary does not count — the troubleshooter is about
+     * whether BLE advertisements are actually reaching us. The freshness cutoff (snapshot seen at or
+     * after this call) means it reflects the currently-active scan settings and doesn't rely on the
+     * device cache having been cleared beforehand.
+     */
+    private suspend fun findLiveBleHeadphones(): Boolean {
+        val threshold = timeSource.now()
+        return withTimeoutOrNull(STEP_TIME) {
+            deviceMonitor.primaryDevice().firstOrNull { device ->
+                device?.ble != null && (device.seenLastAt?.let { it >= threshold } == true)
+            } != null
+        } ?: false
+    }
+
+    /**
+     * Collects BLE devices observed *after the current scan settings take effect*. Option changes
+     * restart the scan after a throttle, and [BlePodMonitor] keeps a 20s device cache, so without a
+     * freshness cutoff a stale observation from a previous combo could be mistaken for a "win".
+     */
+    private suspend fun collectFreshDevices(): List<BlePodSnapshot> {
+        // Drop anything cached under a previous combo so it can't satisfy this attempt.
+        blePodMonitor.clearDeviceCache()
+        val freshThreshold = timeSource.now().plusMillis(SCAN_SETTLE_MS)
+        val start = timeSource.elapsedRealtime()
+        val collected = mutableListOf<BlePodSnapshot>()
+        // Accumulate as we go: a timeout must not discard what we already observed. toList() only
+        // returns once the flow completes, which a quiet channel may never do within the window.
+        withTimeoutOrNull(STEP_TIME) {
+            blePodMonitor.devices
+                .takeWhile { timeSource.elapsedRealtime() - start < STEP_TIME - 1000 }
+                .collect { snapshots -> collected.addAll(snapshots) }
+        }
+        return collected
+            .filter { it.seenLastAt >= freshThreshold }
+            .distinctBy { it.address }
+    }
+
+    private fun persistCompat(combo: BlePodMonitor.CompatOverride) {
+        generalSettings.isOffloadedFilteringDisabled.valueBlocking = combo.offloadedFilteringDisabled
+        generalSettings.isOffloadedBatchingDisabled.valueBlocking = combo.offloadedBatchingDisabled
+        generalSettings.useIndirectScanResultCallback.valueBlocking = combo.indirectCallback
+    }
+
    sealed class BleState {
        class Intro : BleState()

@@ -295,6 +313,29 @@ class TroubleShooterViewModel @Inject constructor(
    companion object {
        const val STEP_TIME = 10 * 1000L

+        /**
+         * Grace period after switching compat settings before an observation counts as "fresh".
+         * Covers [BlePodMonitor]'s ~1s scan-option throttle plus the scanner restart.
+         */
+        const val SCAN_SETTLE_MS = 1500L
+
+        /**
+         * Compatibility combinations to probe, ordered fewest-disables-first so the first one that
+         * works (and gets persisted) is the *minimal* set of overrides — e.g. a phone that only
+         * needs batching disabled won't also get filtering disabled. Triple semantics:
+         * (offloadedFilteringDisabled, offloadedBatchingDisabled, indirectCallback).
+         */
+        val COMPAT_COMBOS: List<BlePodMonitor.CompatOverride> = listOf(
+            BlePodMonitor.CompatOverride(false, false, false), // baseline (no overrides)
+            BlePodMonitor.CompatOverride(false, true, false),  // batching only
+            BlePodMonitor.CompatOverride(true, false, false),  // filtering only
+            BlePodMonitor.CompatOverride(false, false, true),  // indirect callback only
+            BlePodMonitor.CompatOverride(false, true, true),   // batching + indirect
+            BlePodMonitor.CompatOverride(true, false, true),   // filtering + indirect
+            BlePodMonitor.CompatOverride(true, true, false),   // filtering + batching
+            BlePodMonitor.CompatOverride(true, true, true),    // everything
+        )
+
        val TAG = logTag("TroubleShooter", "VM")
    }
 }
@@ -0,0 +1,46 @@
+package eu.darken.capod.troubleshooter.ui
+
+import eu.darken.capod.monitor.core.ble.BlePodMonitor
+import io.kotest.matchers.collections.shouldHaveSize
+import io.kotest.matchers.comparables.shouldBeLessThan
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+class TroubleShooterViewModelTest : BaseTest() {
+
+    private fun BlePodMonitor.CompatOverride.disabledCount() =
+        listOf(offloadedFilteringDisabled, offloadedBatchingDisabled, indirectCallback).count { it }
+
+    @Test
+    fun `compat combos cover every combination exactly once`() {
+        val combos = TroubleShooterViewModel.COMPAT_COMBOS
+        combos shouldHaveSize 8
+        combos.toSet() shouldHaveSize 8
+    }
+
+    @Test
+    fun `compat combos start with the no-override baseline`() {
+        TroubleShooterViewModel.COMPAT_COMBOS.first() shouldBe
+            BlePodMonitor.CompatOverride(false, false, false)
+    }
+
+    @Test
+    fun `compat combos are ordered fewest-disables-first`() {
+        val counts = TroubleShooterViewModel.COMPAT_COMBOS.map { it.disabledCount() }
+        counts shouldBe counts.sorted()
+    }
+
+    @Test
+    fun `batching-only is probed before any combo that also disables filtering`() {
+        // The #603 fix: a phone that only needs batching disabled must land on the minimal combo,
+        // not on "all off", so we don't needlessly disable hardware filtering too.
+        val combos = TroubleShooterViewModel.COMPAT_COMBOS
+        val batchingOnly = combos.indexOf(BlePodMonitor.CompatOverride(false, true, false))
+        val filteringAndBatching = combos.indexOf(BlePodMonitor.CompatOverride(true, true, false))
+        val everything = combos.indexOf(BlePodMonitor.CompatOverride(true, true, true))
+
+        batchingOnly shouldBeLessThan filteringAndBatching
+        batchingOnly shouldBeLessThan everything
+    }
+}