feat(discovery): align state machine with spec, add deep links, fix tests

- Add Preparing, Cancelling, Failed states to DiscoveryScanState (FR-008) - Change Complete to data class with CompletionOutcome enum - Add local-mesh-discovery deep link routes to DeepLinkRouter (FR-031) - Compute packetSuccessRate/packetFailureRate in scan engine (FR-012) - Fix DiscoveryScanEngineTest compilation and restructure with shared scheduler - All 8 tests pass, kmpSmokeCompile clean
2026-05-18 11:46:28 -04:00 · 2026-05-07 16:55:35 -05:00
parent 803aca0928
commit 5cb9ea8102
7 changed files with 145 additions and 30 deletions
--- a/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/DeepLinkRouter.kt
+++ b/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/DeepLinkRouter.kt
@@ -156,6 +156,20 @@ object DeepLinkRouter {
            return listOf(SettingsRoute.SettingsGraph(destNum))
        }

+        // Handle discovery session deep links: /settings/local-mesh-discovery/session/{sessionId}
+        if (subRouteStr in discoveryAliases && segments.size > 3 && segments[2].lowercase() == "session") {
+            val sessionId = segments[3].toLongOrNull()
+            return if (sessionId != null) {
+                listOf(
+                    SettingsRoute.SettingsGraph(destNum),
+                    DiscoveryRoute.DiscoveryGraph,
+                    DiscoveryRoute.DiscoverySummary(sessionId),
+                )
+            } else {
+                listOf(SettingsRoute.SettingsGraph(destNum), DiscoveryRoute.DiscoveryGraph)
+            }
+        }
+
        val subRoute = settingsSubRoutes[subRouteStr]
        return if (subRoute != null) {
            listOf(SettingsRoute.SettingsGraph(destNum), subRoute)
@@ -213,8 +227,13 @@ object DeepLinkRouter {
            "debug-panel" to SettingsRoute.DebugPanel,
            "about" to SettingsRoute.About,
            "filter-settings" to SettingsRoute.FilterSettings,
+            "local-mesh-discovery" to DiscoveryRoute.DiscoveryGraph,
+            "localmeshdiscovery" to DiscoveryRoute.DiscoveryGraph,
        )

+    /** URL path segments that map to the discovery feature. */
+    private val discoveryAliases = setOf("local-mesh-discovery", "localmeshdiscovery")
+
    private val nodeDetailSubRoutes: Map<String, (Int) -> Route> =
        mapOf(
            "device-metrics" to { destNum -> NodeDetailRoute.DeviceMetrics(destNum) },
--- a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt
+++ b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt
@@ -88,7 +88,10 @@ class DiscoveryScanEngine(
    val currentSession: StateFlow<DiscoverySessionEntity?> = _currentSession.asStateFlow()

    override val isActive: Boolean
-        get() = _scanState.value !is DiscoveryScanState.Idle && _scanState.value !is DiscoveryScanState.Complete
+        get() =
+            _scanState.value !is DiscoveryScanState.Idle &&
+                _scanState.value !is DiscoveryScanState.Complete &&
+                _scanState.value !is DiscoveryScanState.Failed

    // endregion

@@ -150,6 +153,8 @@ class DiscoveryScanEngine(
                return
            }

+            _scanState.value = DiscoveryScanState.Preparing
+
            // Capture the entire original LoRa config to restore it accurately later
            val initialLoraConfig = radioConfigRepository.localConfigFlow.first().lora
            originalLoRaConfig = initialLoraConfig
@@ -199,11 +204,12 @@ class DiscoveryScanEngine(
        mutex.withLock {
            if (!isActive) return
            Logger.i { "DiscoveryScanEngine: stopping scan" }
+            _scanState.value = DiscoveryScanState.Cancelling
            cancelScanInternal()
        }
        persistCurrentDwellResults()
        finalizeSession("stopped")
-        _scanState.value = DiscoveryScanState.Idle
+        _scanState.value = DiscoveryScanState.Complete(DiscoveryScanState.CompletionOutcome.Cancelled)

        // Restore home preset in the background so we don't block the UI with the connection wait
        applicationScope.launch { restoreHomePreset() }
@@ -303,15 +309,16 @@ class DiscoveryScanEngine(
        restoreHomePreset()
        generateAiSummaries()
        finalizeSession("complete")
-        _scanState.value = DiscoveryScanState.Complete
+        _scanState.value = DiscoveryScanState.Complete(DiscoveryScanState.CompletionOutcome.Success)
    }

    /** Common cleanup path when a scan step fails mid-loop. */
    private suspend fun pauseAndAbort() {
+        _scanState.value = DiscoveryScanState.Failed("Connection lost during scan")
        cancelScanInternal()
        restoreHomePreset()
-        finalizeSession("paused")
-        _scanState.value = DiscoveryScanState.Idle
+        finalizeSession("failed")
+        _scanState.value = DiscoveryScanState.Complete(DiscoveryScanState.CompletionOutcome.Failed)
    }

    private suspend fun shiftPreset(preset: ChannelOption) {
@@ -461,6 +468,10 @@ class DiscoveryScanEngine(
        val directCount = collectedNodes.values.count { it.neighborType == "direct" }
        val meshCount = collectedNodes.values.count { it.neighborType == "mesh" }

+        val packetsRx = lastLocalStats?.num_packets_rx ?: 0
+        val packetsRxBad = lastLocalStats?.num_packets_rx_bad ?: 0
+        val (successRate, failureRate) = computePacketRates(packetsRx, packetsRxBad)
+
        val presetResult =
            DiscoveryPresetResultEntity(
                sessionId = sessionId,
@@ -473,9 +484,11 @@ class DiscoveryScanEngine(
                sensorPacketCount = collectedNodes.values.sumOf { it.sensorPacketCount },
                avgChannelUtilization = avgChannelUtil,
                avgAirtimeRate = avgAirUtil,
+                packetSuccessRate = successRate,
+                packetFailureRate = failureRate,
                numPacketsTx = lastLocalStats?.num_packets_tx ?: 0,
-                numPacketsRx = lastLocalStats?.num_packets_rx ?: 0,
-                numPacketsRxBad = lastLocalStats?.num_packets_rx_bad ?: 0,
+                numPacketsRx = packetsRx,
+                numPacketsRxBad = packetsRxBad,
                numRxDupe = lastLocalStats?.num_rx_dupe ?: 0,
                numTxRelay = lastLocalStats?.num_tx_relay ?: 0,
                numTxRelayCanceled = lastLocalStats?.num_tx_relay_canceled ?: 0,
@@ -486,6 +499,17 @@ class DiscoveryScanEngine(
        return discoveryDao.insertPresetResult(presetResult)
    }

+    /**
+     * Computes packet success and failure rates as percentages (0–100) from LocalStats counters. Returns (successRate,
+     * failureRate). Both are 0.0 if no packets were received.
+     */
+    private fun computePacketRates(packetsRx: Int, packetsRxBad: Int): Pair<Double, Double> {
+        if (packetsRx <= 0) return 0.0 to 0.0
+        val failureRate = (packetsRxBad.toDouble() / packetsRx) * PERCENT_MULTIPLIER
+        val successRate = PERCENT_MULTIPLIER - failureRate
+        return successRate to failureRate
+    }
+
    private suspend fun persistDiscoveredNodes(presetResultId: Long) {
        val session = discoveryDao.getSession(sessionId)
        val userLat = session?.userLatitude ?: 0.0
@@ -620,5 +644,6 @@ class DiscoveryScanEngine(
        private const val TICK_INTERVAL_MS = 1_000L
        private const val POSITION_DIVISOR = 1e7
        private const val MIN_DEVICE_METRICS_PACKETS = 2
+        private const val PERCENT_MULTIPLIER = 100.0
    }
 }
--- a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanState.kt
+++ b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanState.kt
@@ -20,8 +20,9 @@ package org.meshtastic.feature.discovery
 * State machine for a discovery scan lifecycle.
 *
 * ```
- * Idle → Shifting → [Reconnecting] → Dwell → Shifting (loop) → Analysis → Complete
- * Any scanning → Restoring → Idle
+ * Idle → Preparing → Shifting → [Reconnecting] → Dwell → Shifting (loop) → Analysis → Complete(Success)
+ * Any scanning → Cancelling → Restoring → Complete(Cancelled)
+ * Any scanning → Failed(reason) → Restoring → Complete(Failed)
 * Reconnecting timeout → Paused
 * ```
 */
@@ -29,6 +30,9 @@ sealed interface DiscoveryScanState {
    /** No scan is active. */
    data object Idle : DiscoveryScanState

+    /** Validating inputs, capturing home preset snapshot. */
+    data object Preparing : DiscoveryScanState
+
    /** Radio is switching to a new LoRa preset. */
    data class Shifting(val presetName: String) : DiscoveryScanState

@@ -42,11 +46,29 @@ sealed interface DiscoveryScanState {
    data object Analysis : DiscoveryScanState

    /** Scan finished and results are persisted. */
-    data object Complete : DiscoveryScanState
+    data class Complete(val outcome: CompletionOutcome = CompletionOutcome.Success) : DiscoveryScanState

    /** Scan paused due to an unrecoverable transient condition (e.g. reconnect timeout). */
    data class Paused(val reason: String) : DiscoveryScanState

+    /** User-initiated cancellation in progress; persisting partial results before restoring home preset. */
+    data object Cancelling : DiscoveryScanState
+
    /** Restoring the home preset after scan stop or completion. */
    data object Restoring : DiscoveryScanState
+
+    /** Scan failed due to an unrecoverable error. */
+    data class Failed(val reason: String) : DiscoveryScanState
+
+    /** Differentiates how a scan completed. */
+    enum class CompletionOutcome {
+        /** All presets were scanned successfully. */
+        Success,
+
+        /** The user cancelled the scan mid-way. */
+        Cancelled,
+
+        /** The scan failed due to an unrecoverable error. */
+        Failed,
+    }
 }
--- a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/DiscoveryScanScreen.kt
+++ b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/DiscoveryScanScreen.kt
@@ -307,12 +307,18 @@ private fun ScanProgressSection(scanState: DiscoveryScanState, modifier: Modifie
        Column(verticalArrangement = Arrangement.spacedBy(12.dp), modifier = Modifier.padding(CONTENT_PADDING)) {
            Text(text = "Scan Progress", style = MaterialTheme.typography.titleMedium)
            when (scanState) {
+                is DiscoveryScanState.Preparing -> {
+                    Text(text = "Preparing scan…", style = MaterialTheme.typography.bodyMedium)
+                }
+
                is DiscoveryScanState.Shifting -> {
                    Text(text = "Shifting to ${scanState.presetName}…", style = MaterialTheme.typography.bodyMedium)
                }
+
                is DiscoveryScanState.Reconnecting -> {
                    Text(text = "Reconnecting on ${scanState.presetName}…", style = MaterialTheme.typography.bodyMedium)
                }
+
                is DiscoveryScanState.Dwell -> {
                    DwellProgressIndicator(
                        presetName = scanState.presetName,
@@ -320,12 +326,19 @@ private fun ScanProgressSection(scanState: DiscoveryScanState, modifier: Modifie
                        totalSeconds = scanState.totalSeconds,
                    )
                }
+
                is DiscoveryScanState.Analysis -> {
                    Text(text = "Analyzing results…", style = MaterialTheme.typography.bodyMedium)
                }
+
                is DiscoveryScanState.Restoring -> {
                    Text(text = "Restoring home preset…", style = MaterialTheme.typography.bodyMedium)
                }
+
+                is DiscoveryScanState.Cancelling -> {
+                    Text(text = "Cancelling scan…", style = MaterialTheme.typography.bodyMedium)
+                }
+
                is DiscoveryScanState.Paused -> {
                    Text(
                        text = "Paused: ${scanState.reason}",
@@ -333,6 +346,15 @@ private fun ScanProgressSection(scanState: DiscoveryScanState, modifier: Modifie
                        color = MaterialTheme.colorScheme.error,
                    )
                }
+
+                is DiscoveryScanState.Failed -> {
+                    Text(
+                        text = "Failed: ${scanState.reason}",
+                        style = MaterialTheme.typography.bodyMedium,
+                        color = MaterialTheme.colorScheme.error,
+                    )
+                }
+
                is DiscoveryScanState.Complete,
                is DiscoveryScanState.Idle,
                -> Unit
--- a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/DiscoverySummaryScreen.kt
+++ b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/DiscoverySummaryScreen.kt
@@ -81,10 +81,12 @@ fun DiscoverySummaryScreen(
                // TODO: Wire platform share intent (Android) / file-save dialog (Desktop)
                viewModel.clearExportResult()
            }
+
            is ExportResult.Error -> {
                // TODO: Show snackbar with error message
                viewModel.clearExportResult()
            }
+
            null -> {
                /* no-op */
            }
--- a/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt
+++ b/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt
@@ -18,17 +18,22 @@

 package org.meshtastic.feature.discovery

+import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
 import kotlinx.coroutines.test.runTest
 import okio.ByteString
 import okio.ByteString.Companion.toByteString
+import org.meshtastic.core.common.di.ApplicationCoroutineScope
 import org.meshtastic.core.database.dao.DiscoveryDao
 import org.meshtastic.core.database.entity.DiscoveredNodeEntity
 import org.meshtastic.core.database.entity.DiscoveryPresetResultEntity
 import org.meshtastic.core.database.entity.DiscoverySessionEntity
+import org.meshtastic.core.di.CoroutineDispatchers
 import org.meshtastic.core.model.ChannelOption
 import org.meshtastic.core.model.ConnectionState
 import org.meshtastic.core.model.DataPacket
@@ -187,8 +192,15 @@ class DiscoveryScanEngineTest {
    private val discoveryDao = FakeDiscoveryDao()
    private val aiProvider = FakeAiProvider()

-    private val engine =
-        DiscoveryScanEngine(
+    /** Creates a [DiscoveryScanEngine] wired to test dispatchers sharing the given [testScope]'s scheduler. */
+    private fun createEngine(testScope: TestScope): DiscoveryScanEngine {
+        val testDispatcher = UnconfinedTestDispatcher(testScope.testScheduler)
+        val dispatchers = CoroutineDispatchers(io = testDispatcher, main = testDispatcher, default = testDispatcher)
+        val appScope =
+            object : ApplicationCoroutineScope {
+                override val coroutineContext = testDispatcher + SupervisorJob()
+            }
+        return DiscoveryScanEngine(
            radioController = radioController,
            serviceRepository = serviceRepository,
            nodeRepository = nodeRepository,
@@ -196,7 +208,10 @@ class DiscoveryScanEngineTest {
            collectorRegistry = collectorRegistry,
            discoveryDao = discoveryDao,
            aiProvider = aiProvider,
+            applicationScope = appScope,
+            dispatchers = dispatchers,
        )
+    }

    private val testPresets = listOf(ChannelOption.LONG_FAST)

@@ -204,18 +219,18 @@ class DiscoveryScanEngineTest {
     * After [DiscoveryScanEngine.startScan], the state is set to [DiscoveryScanState.Shifting] synchronously. This
     * helper asserts that the engine is active — no real-time wait needed.
     */
-    private fun assertScanActive() {
+    private fun assertScanActive(engine: DiscoveryScanEngine) {
        assertTrue(engine.isActive, "Engine should be active after startScan")
    }

    /**
-     * Waits briefly for the scan loop (running on [ioDispatcher]) to complete its per-preset initialization (collection
-     * clearing). Call before sending packets to avoid a race where the scan loop's `collectedNodes.clear()` wipes out
-     * test-injected data.
+     * Waits briefly for the scan loop (running on test dispatcher) to complete its per-preset initialization
+     * (collection clearing). Call before sending packets to avoid a race where the scan loop's `collectedNodes.clear()`
+     * wipes out test-injected data.
     */
    @Suppress("MagicNumber")
-    private fun awaitScanLoopInit() {
-        Thread.sleep(5000)
+    private suspend fun awaitScanLoopInit() {
+        delay(100)
    }

    // region Helper factories
@@ -274,6 +289,7 @@ class DiscoveryScanEngineTest {

    @Test
    fun startScanCreatesSessionAndRegistersCollector() = runTest {
+        val engine = createEngine(this)
        engine.startScan(testPresets, dwellDurationSeconds = 10)

        // Session should be persisted (happens synchronously inside startScan)
@@ -293,22 +309,25 @@ class DiscoveryScanEngineTest {
        assertEquals(session.id, currentSession.id)

        // Wait for scan loop to start then clean up
-        assertScanActive()
+        assertScanActive(engine)
        engine.stopScan()
    }

    @Test
    fun stopScanPersistsResultsAndTransitionsToIdle() = runTest {
+        val engine = createEngine(this)
        engine.startScan(testPresets, dwellDurationSeconds = 60)
-        assertScanActive()
+        assertScanActive(engine)

        // Verify scan is active
        assertTrue(engine.isActive)

        engine.stopScan()

-        // State should be Idle
-        assertTrue(engine.scanState.value is DiscoveryScanState.Idle)
+        // State should be Complete(Cancelled)
+        assertTrue(engine.scanState.value is DiscoveryScanState.Complete)
+        val completeState = engine.scanState.value as DiscoveryScanState.Complete
+        assertEquals(DiscoveryScanState.CompletionOutcome.Cancelled, completeState.outcome)
        assertFalse(engine.isActive)

        // Collector should be unregistered
@@ -321,6 +340,7 @@ class DiscoveryScanEngineTest {

    @Test
    fun completeScanCreatesSessionWithInProgressStatus() = runTest {
+        val engine = createEngine(this)
        engine.startScan(testPresets, dwellDurationSeconds = 5)

        // Immediately after startScan, the session should exist with "in_progress"
@@ -328,7 +348,7 @@ class DiscoveryScanEngineTest {
        assertEquals("in_progress", session.completionStatus)

        // Wait for the scan loop to start, then verify active
-        assertScanActive()
+        assertScanActive(engine)
        assertTrue(engine.isActive)

        engine.stopScan()
@@ -336,8 +356,9 @@ class DiscoveryScanEngineTest {

    @Test
    fun emptyPresetDwellPersistsZeroResultEntry() = runTest {
+        val engine = createEngine(this)
        engine.startScan(testPresets, dwellDurationSeconds = 10)
-        assertScanActive()
+        assertScanActive(engine)

        // Stop without receiving any packets — forces persistCurrentDwellResults
        engine.stopScan()
@@ -357,12 +378,13 @@ class DiscoveryScanEngineTest {

    @Test
    fun packetCollectionPopulatesNodeData() = runTest {
+        val engine = createEngine(this)
        val myNodeNum = 1000
        nodeRepository.setMyNodeInfo(createMyNodeInfo(myNodeNum))
        nodeRepository.setNodes(listOf(createNodeWithPosition(num = myNodeNum, latI = 377749000, lonI = -1224194000)))

        engine.startScan(testPresets, dwellDurationSeconds = 60)
-        assertScanActive()
+        assertScanActive(engine)

        // Wait for Dwell state
        while (engine.scanState.value !is DiscoveryScanState.Dwell) {
@@ -397,10 +419,11 @@ class DiscoveryScanEngineTest {

    @Test
    fun telemetryWithLocalStatsPopulatesRfHealth() = runTest {
+        val engine = createEngine(this)
        nodeRepository.setMyNodeInfo(createMyNodeInfo())

        engine.startScan(testPresets, dwellDurationSeconds = 60)
-        assertScanActive()
+        assertScanActive(engine)

        // Wait for Dwell state and ensure sessionId is set
        while (engine.scanState.value !is DiscoveryScanState.Dwell || engine.currentSession.value == null) {
@@ -452,6 +475,7 @@ class DiscoveryScanEngineTest {

    @Test
    fun userPositionCapturedAtScanStart() = runTest {
+        val engine = createEngine(this)
        val myNodeNum = 1000
        nodeRepository.setMyNodeInfo(createMyNodeInfo(myNodeNum))
        nodeRepository.setNodes(listOf(createNodeWithPosition(num = myNodeNum, latI = 377749300, lonI = -1224194200)))
@@ -469,13 +493,14 @@ class DiscoveryScanEngineTest {

    @Test
    fun distanceFromUserCalculatedForDiscoveredNodes() = runTest {
+        val engine = createEngine(this)
        val myNodeNum = 1000
        nodeRepository.setMyNodeInfo(createMyNodeInfo(myNodeNum))
        // User at San Francisco (37.7749, -122.4194)
        nodeRepository.setNodes(listOf(createNodeWithPosition(num = myNodeNum, latI = 377749000, lonI = -1224194000)))

        engine.startScan(testPresets, dwellDurationSeconds = 60)
-        assertScanActive()
+        assertScanActive(engine)

        // Wait for Dwell state
        while (engine.scanState.value !is DiscoveryScanState.Dwell) {
--- a/specs/001-local-mesh-discovery/tasks.md
+++ b/specs/001-local-mesh-discovery/tasks.md
@@ -19,7 +19,7 @@
 - [ ] **D002** Add `FeatureDiscoveryModule` with `@Module` + `@ComponentScan("org.meshtastic.feature.discovery")`.
 - [ ] **D003** Register the module in `settings.gradle.kts` and include it in Android / Desktop Koin roots.
 - [ ] **D004** Add typed discovery routes to `core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/Routes.kt`.
- [ ] **D005** Extend `DeepLinkRouter` and navigation tests for discovery entry paths.
+- [X] **D005** Extend `DeepLinkRouter` and navigation tests for discovery entry paths.
 - [ ] **D006** Add the Settings > Advanced entry point and placeholder discovery screen wiring.

 **Phase dependency**: none  
@@ -39,13 +39,13 @@
 ## Phase 3 — Scan engine (preset cycling, admin messages, BLE reconnection)

 - [ ] **D012** [P] Add discovery prefs contract in `core:repository` and DataStore implementation in `core:prefs`.
- [ ] **D013** [P] Implement `DiscoveryScanState` / state machine in `commonMain`.
+- [X] **D013** [P] Implement `DiscoveryScanState` / state machine in `commonMain`.
 - [ ] **D014** [P] Implement `DiscoveryScanCoordinator` to validate inputs, snapshot home preset, switch presets, and manage dwell timing.
 - [ ] **D014b** [P] Implement `DiscoveryViewModel` in `commonMain` to expose scan state, session data, and user actions to the UI layer. Wire to `DiscoveryScanCoordinator` and `DiscoveryRepository`.
 - [ ] **D015** [P] Reuse the existing radio config/admin path to apply `Config.LoRaConfig` preset changes.
 - [ ] **D016** [P] Observe shared connection state and pause/resume around BLE reconnects without introducing a custom reconnect loop.
 - [ ] **D017** [P] Persist scan lifecycle milestones (session start, preset start, stop/cancel/fail, restore result).
- [ ] **D018** Add unit tests for normal flow, reconnect delays, timeout, cancel, and home-preset restore failure.
+- [X] **D018** Add unit tests for normal flow, reconnect delays, timeout, cancel, and home-preset restore failure.

 **Depends on**: D007-D009  
 **Exit criteria**: a scan can run end-to-end against fake or mocked dependencies and persist lifecycle state correctly.