From dcc944795497cbd9e701bf71d10a84f53a45da8d Mon Sep 17 00:00:00 2001 From: James Rich Date: Tue, 16 Jun 2026 14:58:25 -0500 Subject: [PATCH 1/2] fix: resolve release/2.8.0 branch-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes four defects found reviewing the full release/2.8.0 branch; three would have shipped a feature broken/unsafe in production, invisible to CI. - car: HostValidator allowlist now uses Google's official "digest,packageName" entries (copied from androidx.car.app:1.9.0-alpha01's hosts_allowlist_sample). Bare package names made addAllowedHosts() throw IllegalArgumentException on the first entry, so the car app never connected to Android Auto in any release build (masked locally by the debug ALLOW_ALL_HOSTS_VALIDATOR). - ai(appfunctions): format/parse node IDs via NodeAddress.numToDefaultId/idToNum. The old Int.toString(16) emitted "!-1" instead of "!ffffffff" and the reverse toInt(16) failed for canonical 8-hex IDs > Int.MAX, so AI agents got malformed IDs and broken lookups for ~half the node-num space. Drops the dead HEX_RADIX const; adds a high-bit round-trip regression test. - discovery: pauseAndAbort() now finalizes the session, reaches the terminal state, and restores the home preset on applicationScope before cancelScanInternal() runs last. Previously it cancelled its own scanScope first, so the next suspend threw CancellationException and skipped restore/finalize — stranding the radio on the scan modem preset with the UI stuck on Failed. Mirrors stopScan()'s ordering. - node(air-quality): show present-zero readings in the summary cards (drop the != 0 guards, keep the null/presence check), consistent with the #5793 chart fix. Verified: spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile -> BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/data/ai/AiFunctionProviderImpl.kt | 12 +++++---- .../data/ai/AiFunctionProviderImplTest.kt | 16 ++++++++++++ .../src/main/res/values/hosts_allowlist.xml | 24 ++++++++++++------ .../feature/discovery/DiscoveryScanEngine.kt | 10 ++++++-- .../node/component/AirQualityMetrics.kt | 25 +++++++------------ 5 files changed, 57 insertions(+), 30 deletions(-) diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt index 26f999a5fb..f1b5a37fc9 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt @@ -130,7 +130,7 @@ class AiFunctionProviderImpl( val nodes = nodeMap.values.map { node -> NodeSummary( - id = "!${node.num.toString(HEX_RADIX)}", + id = NodeAddress.numToDefaultId(node.num), name = node.user.long_name.takeIf { it.isNotBlank() } ?: "Node ${node.num}", batteryLevel = node.deviceMetrics.battery_level?.coerceIn(0, MAX_BATTERY_LEVEL), lastHeard = node.lastHeard.toLong() * MS_PER_SEC, @@ -201,8 +201,11 @@ class AiFunctionProviderImpl( try { val node = if (nodeId.startsWith("!")) { - // Hex format: extract number and search - val nodeNum = nodeId.drop(1).toInt(HEX_RADIX) + // Canonical hex node ID (e.g. "!ffffffff"). idToNum parses the full unsigned 32-bit + // range and returns null for malformed input, which we surface as NotFound. + val nodeNum = + NodeAddress.idToNum(nodeId) + ?: return@withTimeout GetNodeDetailsResult.NotFound("Node not found: $nodeId") nodeRepository.nodeDBbyNum.first()[nodeNum] } else { // User ID format @@ -218,7 +221,7 @@ class AiFunctionProviderImpl( val details = NodeDetails( - id = "!${node.num.toString(HEX_RADIX)}", + id = NodeAddress.numToDefaultId(node.num), userId = node.user.id, name = node.user.long_name.takeIf { it.isNotBlank() } ?: "Node ${node.num}", batteryLevel = node.deviceMetrics.battery_level?.coerceIn(0, MAX_BATTERY_LEVEL), @@ -502,7 +505,6 @@ class AiFunctionProviderImpl( companion object { private val OPERATION_TIMEOUT = 5.seconds private const val MAX_BATTERY_LEVEL = 100 - private const val HEX_RADIX = 16 private const val MS_PER_SEC = 1000L private const val HEALTH_SCORE_BASE = 50 private const val HEALTH_SCORE_ONLINE_RATIO = 50 diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt index 5b87e65133..4da351b0cc 100644 --- a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt @@ -137,6 +137,22 @@ class AiFunctionProviderImplTest { assertEquals(true, isHandled) } + @Test + fun getNodeDetails_round_trips_high_bit_node_num() = runTest { + // A node num with the high bit set (-1 == 0xFFFFFFFF) must format and parse as the canonical + // "!ffffffff", not the signed "!-1" — regression guard for the node-ID hex fix. + val testNode = Node(num = -1, user = User(id = "!ffffffff", long_name = "HighBit", short_name = "HB")) + val nodeMap = MutableStateFlow(mapOf(-1 to testNode)) + every { nodeRepository.nodeDBbyNum } returns nodeMap + + val provider = createProvider() + val result = provider.getNodeDetails("!ffffffff") + + assertIs(result) + assertEquals("!ffffffff", result.node.id) + assertEquals("HighBit", result.node.name) + } + // --- getMeshMetrics tests --- @Test diff --git a/feature/car/src/main/res/values/hosts_allowlist.xml b/feature/car/src/main/res/values/hosts_allowlist.xml index b623ae4422..b1d74f95dc 100644 --- a/feature/car/src/main/res/values/hosts_allowlist.xml +++ b/feature/car/src/main/res/values/hosts_allowlist.xml @@ -1,11 +1,21 @@ - - - com.google.android.projection.gearhead - - com.android.car.carlauncher - - com.google.android.apps.auto + + + + fdb00c43dbde8b51cb312aa81d3b5fa17713adb94b28f598d77f8eb89daceedf,com.google.android.projection.gearhead + 70811a3eacfd2e83e18da9bfede52df16ce91f2e69a44d21f18ab66991130771,com.google.android.projection.gearhead + 1975b2f17177bc89a5dff31f9e64a6cae281a53dc1d1d59b1d147fe1c82afa00,com.google.android.projection.gearhead + + c241ffbc8e287c4e9a4ad19632ba1b1351ad361d5177b7d7b29859bd2b7fc631,com.google.android.apps.automotive.templates.host + dd66deaf312d8daec7adbe85a218ecc8c64f3b152f9b5998d5b29300c2623f61,com.google.android.apps.automotive.templates.host + 50e603d333c6049a37bd751375d08f3bd0abebd33facd30bd17b64b89658b421,com.google.android.apps.automotive.templates.host diff --git a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt index 9308a38411..269d6ef3b2 100644 --- a/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt +++ b/feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt @@ -321,10 +321,16 @@ class DiscoveryScanEngine( /** Common cleanup path when a scan step fails mid-loop. */ private suspend fun pauseAndAbort() { _scanState.value = DiscoveryScanState.Failed("Connection lost during scan") - cancelScanInternal() - restoreHomePreset() + // pauseAndAbort runs inside the runScanLoop coroutine, which is a child of scanScope. + // cancelScanInternal() cancels scanScope (and therefore this coroutine), so it must run LAST: + // any suspend after it — finalizeSession or restoreHomePreset — would throw CancellationException + // and silently skip cleanup, stranding the radio on the scan modem preset. So finalize and reach + // the terminal state first, then restore the home preset on applicationScope (which outlives + // scanScope), mirroring stopScan(). finalizeSession("failed") _scanState.value = DiscoveryScanState.Complete(DiscoveryScanState.CompletionOutcome.Failed) + applicationScope.launch { restoreHomePreset() } + cancelScanInternal() } private suspend fun shiftPreset(preset: ChannelOption) { diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/AirQualityMetrics.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/AirQualityMetrics.kt index fce8dea97b..18a640156b 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/AirQualityMetrics.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/AirQualityMetrics.kt @@ -37,8 +37,9 @@ import org.meshtastic.core.ui.icon.MeshtasticIcons import org.meshtastic.feature.node.model.VectorMetricInfo /** - * Displays air quality info cards for a node showing PM1.0, PM2.5, PM10 and CO₂ values. Cards with zero values are - * hidden. CO₂ value text is color-coded by severity. + * Displays air quality info cards for a node showing PM1.0, PM2.5, PM10 and CO₂ values. A card is shown for each metric + * the node actually reports; a present reading of 0 (e.g. clean air at 0 µg/m³) is a valid value and is shown — only + * absent metrics are hidden. CO₂ value text is color-coded by severity. */ @Composable internal fun AirQualityInfoCards(node: Node) { @@ -47,26 +48,18 @@ internal fun AirQualityInfoCards(node: Node) { val ppmUnit = stringResource(Res.string.ppm) val cards = buildList { + // A present reading of 0 is a valid value (e.g. clean air at 0 µg/m³), so only the `?.` null-check (an + // absent metric) hides a card — matching the #5793 chart/CSV zero-suppression fix. metrics.pm10_standard?.let { pm -> - if (pm != 0) { - add(VectorMetricInfo(Res.string.pm1_0, "$pm $ugm3", MeshtasticIcons.AirQuality)) - } + add(VectorMetricInfo(Res.string.pm1_0, "$pm $ugm3", MeshtasticIcons.AirQuality)) } metrics.pm25_standard?.let { pm -> - if (pm != 0) { - add(VectorMetricInfo(Res.string.pm2_5, "$pm $ugm3", MeshtasticIcons.AirQuality)) - } + add(VectorMetricInfo(Res.string.pm2_5, "$pm $ugm3", MeshtasticIcons.AirQuality)) } metrics.pm100_standard?.let { pm -> - if (pm != 0) { - add(VectorMetricInfo(Res.string.pm10, "$pm $ugm3", MeshtasticIcons.AirQuality)) - } - } - metrics.co2?.let { co2 -> - if (co2 != 0) { - add(VectorMetricInfo(Res.string.co2, "$co2 $ppmUnit", MeshtasticIcons.AirQuality)) - } + add(VectorMetricInfo(Res.string.pm10, "$pm $ugm3", MeshtasticIcons.AirQuality)) } + metrics.co2?.let { co2 -> add(VectorMetricInfo(Res.string.co2, "$co2 $ppmUnit", MeshtasticIcons.AirQuality)) } } if (cards.isEmpty()) return From 8b9fe0dcd80aee5ff558739662017c37f5df965f Mon Sep 17 00:00:00 2001 From: James Rich Date: Tue, 16 Jun 2026 15:20:56 -0500 Subject: [PATCH 2/2] fix: repair FTS5 historical-message backfill and add review test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the two MEDIUM test-coverage findings from the release/2.8.0 review. - database(fts5): the historical-message backfill was dead code. It keyed on json_extract(data, '$.text'), but DataPacket.text is a computed property that is never serialized into the stored JSON (the body is persisted as `bytes`), so the predicate was always NULL — every message received before the v39 upgrade stayed permanently unsearchable. Replace it with a Kotlin-side decode that reads each packet's payload and writes message_text via updateMessageText (previously unused). countPacketsNeedingBackfill no longer depends on json_extract. Adds PacketFtsSearchTest covering search match/non-match, conversation scoping, and the backfill regression (the new test fails against the old json_extract code). - discovery: add tests for the home-preset restoration path — a reconnect-timeout abort now reaches Complete(Failed), finalizes the session as "failed", and restores the home LoRa preset (the behavior the pauseAndAbort ordering fix enables), plus stopScan and normal-completion restore. FakeRadioController now records setLocalConfig calls so the restore can be asserted. Verified: spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile -> BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/database/dao/PacketFtsSearchTest.kt | 141 ++++++++++++++++++ .../core/database/DatabaseManager.kt | 7 +- .../meshtastic/core/database/dao/PacketDao.kt | 34 +++-- .../core/testing/FakeRadioController.kt | 11 +- .../discovery/DiscoveryScanEngineTest.kt | 54 +++++++ 5 files changed, 232 insertions(+), 15 deletions(-) create mode 100644 core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/PacketFtsSearchTest.kt diff --git a/core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/PacketFtsSearchTest.kt b/core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/PacketFtsSearchTest.kt new file mode 100644 index 0000000000..0d83b89332 --- /dev/null +++ b/core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/PacketFtsSearchTest.kt @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.database.dao + +import androidx.room3.Room +import androidx.sqlite.driver.bundled.BundledSQLiteDriver +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.meshtastic.core.common.util.nowMillis +import org.meshtastic.core.database.MeshtasticDatabase +import org.meshtastic.core.database.MeshtasticDatabaseConstructor +import org.meshtastic.core.database.entity.MyNodeEntity +import org.meshtastic.core.database.entity.Packet +import org.meshtastic.core.model.DataPacket +import org.meshtastic.core.model.NodeAddress +import org.meshtastic.proto.PortNum +import org.robolectric.annotation.Config +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Verifies FTS5 full-text message search (#5373) and the historical-message backfill. + * + * [backfillMessageTexts_makesHistoricalMessagesSearchable] is the regression guard for the original `json_extract(data, + * '$.text')` backfill, which silently matched nothing: `DataPacket.text` is a computed property and is never serialized + * into the stored JSON, so historical messages stayed permanently unsearchable. + */ +@RunWith(AndroidJUnit4::class) +@Config(sdk = [34]) +class PacketFtsSearchTest { + private lateinit var database: MeshtasticDatabase + private lateinit var packetDao: PacketDao + private lateinit var nodeInfoDao: NodeInfoDao + + private val myNodeNum = 42424242 + + private val myNodeInfo = + MyNodeEntity( + myNodeNum = myNodeNum, + model = null, + firmwareVersion = null, + couldUpdate = false, + shouldUpdate = false, + currentPacketId = 1L, + messageTimeoutMsec = 5 * 60 * 1000, + minAppVersion = 1, + maxChannels = 8, + hasWifi = false, + ) + + @Before + fun createDb(): Unit = runTest { + val context = ApplicationProvider.getApplicationContext() + database = + Room.inMemoryDatabaseBuilder( + context = context, + factory = { MeshtasticDatabaseConstructor.initialize() }, + ) + .setDriver(BundledSQLiteDriver()) + .build() + nodeInfoDao = database.nodeInfoDao().apply { setMyNodeInfo(myNodeInfo) } + packetDao = database.packetDao() + } + + @After + fun closeDb() { + database.close() + } + + @Test + fun searchMessages_matchesIndexedText_ignoresNonMatches() = runTest { + insertTextPacket(contactKey = CONTACT, text = "the quick brown fox", messageText = "the quick brown fox") + + assertEquals(1, packetDao.searchMessages("brown").size, "a term in the indexed message should match") + assertTrue(packetDao.searchMessages("zebra").isEmpty(), "an absent term should not match") + } + + @Test + fun searchMessagesInConversation_scopesToContact() = runTest { + insertTextPacket(contactKey = CONTACT, text = "shared keyword here", messageText = "shared keyword here") + insertTextPacket(contactKey = OTHER_CONTACT, text = "shared keyword here", messageText = "shared keyword here") + + assertEquals(2, packetDao.searchMessages("keyword").size, "both conversations match the global search") + assertEquals(1, packetDao.searchMessagesInConversation("keyword", CONTACT).size, "scoped to one contact") + } + + @Test + fun backfillMessageTexts_makesHistoricalMessagesSearchable() = runTest { + // A pre-v39 packet: the payload carries the text, but message_text was never populated, so it is unindexed. + insertTextPacket(contactKey = CONTACT, text = "historical needle", messageText = "") + + assertTrue(packetDao.searchMessages("needle").isEmpty(), "the historical message is unindexed before backfill") + assertEquals(1, packetDao.countPacketsNeedingBackfill(), "the empty-message_text packet needs backfill") + + val updated = packetDao.backfillMessageTexts() + packetDao.rebuildFtsIndex() + + assertEquals(1, updated, "the historical text packet should be backfilled") + assertEquals(1, packetDao.searchMessages("needle").size, "the backfilled message should now be searchable") + assertEquals(0, packetDao.countPacketsNeedingBackfill(), "nothing left to backfill") + } + + private suspend fun insertTextPacket(contactKey: String, text: String, messageText: String) { + packetDao.insert( + Packet( + uuid = 0L, + myNodeNum = myNodeNum, + port_num = PortNum.TEXT_MESSAGE_APP.value, + contact_key = contactKey, + received_time = nowMillis, + read = false, + data = DataPacket(to = NodeAddress.ID_BROADCAST, channel = 0, text = text), + messageText = messageText, + ), + ) + } + + companion object { + private const val CONTACT = "0!aaaa1111" + private const val OTHER_CONTACT = "0!bbbb2222" + } +} diff --git a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt index 92db4c4e7c..ceb2391230 100644 --- a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt +++ b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt @@ -310,9 +310,10 @@ open class DatabaseManager( } /** - * Backfills [Packet.messageText] for existing text-message packets that predate the FTS5 schema. Uses a single SQL - * UPDATE with json_extract to avoid loading all packets into memory, then rebuilds the FTS index so search covers - * historical messages. + * Backfills [Packet.messageText] for existing text-message packets that predate the FTS5 schema, then rebuilds the + * FTS index so search covers historical messages. The text is decoded in Kotlin from each packet's payload (see + * [PacketDao.backfillMessageTexts]); it cannot be read in SQL because the message body is stored as serialized + * `bytes`, not a `text` JSON field. */ private suspend fun backfillSearchIndexIfNeeded(db: MeshtasticDatabase) { val needsBackfill = db.packetDao().countPacketsNeedingBackfill() > 0 diff --git a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt index 29068a7c16..06c14dcbb7 100644 --- a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt +++ b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt @@ -568,19 +568,31 @@ interface PacketDao { @Query("UPDATE packet SET message_text = :text WHERE uuid = :uuid") suspend fun updateMessageText(uuid: Long, text: String) - @Query( - "SELECT COUNT(*) FROM packet " + - "WHERE port_num = 1 AND (message_text IS NULL OR message_text = '') " + - "AND json_extract(data, '\$.text') IS NOT NULL", - ) + @Query("SELECT COUNT(*) FROM packet WHERE port_num = 1 AND (message_text IS NULL OR message_text = '')") suspend fun countPacketsNeedingBackfill(): Int - @Query( - "UPDATE packet SET message_text = json_extract(data, '\$.text') " + - "WHERE port_num = 1 AND (message_text IS NULL OR message_text = '') " + - "AND json_extract(data, '\$.text') IS NOT NULL", - ) - suspend fun backfillMessageTexts(): Int + @Query("SELECT * FROM packet WHERE port_num = 1 AND (message_text IS NULL OR message_text = '')") + suspend fun getPacketsNeedingBackfill(): List + + /** + * Populates [Packet.messageText] for historical text packets that predate the FTS5 schema (v39) so they become + * searchable. The text is decoded in Kotlin from each packet's [DataPacket.text]; it cannot be read with a SQL + * `json_extract(data, '$.text')` because [DataPacket.text] is a computed property that is never serialized into the + * stored JSON (the payload is persisted as `bytes`). Returns the number of rows updated; the caller rebuilds the + * FTS index via [rebuildFtsIndex] when this is greater than zero. + */ + @Transaction + suspend fun backfillMessageTexts(): Int { + var updated = 0 + getPacketsNeedingBackfill().forEach { packet -> + val text = packet.data.text + if (!text.isNullOrEmpty()) { + updateMessageText(packet.uuid, text) + updated++ + } + } + return updated + } @Query("INSERT INTO packet_fts(packet_fts) VALUES('rebuild')") suspend fun rebuildFtsIndex() diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt index 5ce42d269e..72297c5982 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt @@ -46,6 +46,12 @@ class FakeRadioController : val sentPackets = mutableListOf() val favoritedNodes = mutableListOf() val sentSharedContacts = mutableListOf() + + /** Every [setLocalConfig] call, in order — lets tests assert e.g. that a scan restored the home LoRa preset. */ + val localConfigs = mutableListOf() + val lastLocalConfig: Config? + get() = localConfigs.lastOrNull() + var throwOnSend: Boolean = false var lastSetDeviceAddress: String? = null var editSettingsCalled = false @@ -57,6 +63,7 @@ class FakeRadioController : sentPackets.clear() favoritedNodes.clear() sentSharedContacts.clear() + localConfigs.clear() throwOnSend = false lastSetDeviceAddress = null editSettingsCalled = false @@ -93,7 +100,9 @@ class FakeRadioController : override suspend fun refreshMetadata(destNum: Int) {} - override suspend fun setLocalConfig(config: Config) {} + override suspend fun setLocalConfig(config: Config) { + localConfigs.add(config) + } override suspend fun setLocalChannel(channel: Channel) {} diff --git a/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt b/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt index 21183a1afb..ae5dc1a806 100644 --- a/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt +++ b/feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import okio.ByteString import okio.ByteString.Companion.toByteString @@ -535,4 +536,57 @@ class DiscoveryScanEngineTest { "Distance should be between 10km and 25km, was ${node.distanceFromUser}m", ) } + + // region Home-preset restoration (the one config-mutating, safety-critical behavior of a scan) + + @Test + fun reconnectTimeoutAbortsRestoresHomePresetAndFinalizesSession() = runTest { + val engine = createEngine(this) + // Scan a preset different from the seeded home (LONG_FAST) so a restore-to-home is observable. + engine.startScan(listOf(ChannelOption.SHORT_FAST), dwellDurationSeconds = 60) + assertScanActive(engine) + + // The radio fails to come back after the preset shift (firmware reboots on a LoRa config change). + serviceRepository.setConnectionState(ConnectionState.Disconnected) + advanceUntilIdle() + + // The abort path must reach a terminal Failed state, finalize the session, and restore the home preset — + // none of which happened before the fix, because cancelScanInternal() cancelled this coroutine first. + val state = engine.scanState.value + assertTrue(state is DiscoveryScanState.Complete, "expected Complete, was $state") + assertEquals(DiscoveryScanState.CompletionOutcome.Failed, (state as DiscoveryScanState.Complete).outcome) + assertFalse(engine.isActive) + assertNull(collectorRegistry.collector, "collector should be unregistered") + + assertEquals("failed", discoveryDao.sessions.values.first().completionStatus) + // The last LoRa config applied is the home preset (LONG_FAST), not the scan preset (SHORT_FAST). + assertEquals(ChannelOption.LONG_FAST.modemPreset, radioController.lastLocalConfig?.lora?.modem_preset) + } + + @Test + fun stopScanRestoresHomePreset() = runTest { + val engine = createEngine(this) + engine.startScan(listOf(ChannelOption.SHORT_FAST), dwellDurationSeconds = 60) + assertScanActive(engine) + + engine.stopScan() + advanceUntilIdle() // let the applicationScope restore complete + + assertEquals(ChannelOption.LONG_FAST.modemPreset, radioController.lastLocalConfig?.lora?.modem_preset) + } + + @Test + fun normalCompletionRestoresHomePreset() = runTest { + val engine = createEngine(this) + engine.startScan(listOf(ChannelOption.SHORT_FAST), dwellDurationSeconds = 1) + advanceUntilIdle() // connection stays Connected, so the scan runs to completion + + val state = engine.scanState.value + assertTrue(state is DiscoveryScanState.Complete, "expected Complete, was $state") + assertEquals(DiscoveryScanState.CompletionOutcome.Success, (state as DiscoveryScanState.Complete).outcome) + assertEquals("complete", discoveryDao.sessions.values.first().completionStatus) + assertEquals(ChannelOption.LONG_FAST.modemPreset, radioController.lastLocalConfig?.lora?.modem_preset) + } + + // endregion }