fix: resolve release/2.8.0 branch-review findings (car hosts, AI node IDs, discovery abort, AQ zeros) (#5813)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
James Rich
2026-06-16 15:31:58 -05:00
parent bfe3440a11
commit 8874352ba4
10 changed files with 289 additions and 45 deletions

View File

@@ -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

View File

@@ -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<GetNodeDetailsResult.Success>(result)
assertEquals("!ffffffff", result.node.id)
assertEquals("HighBit", result.node.name)
}
// --- getMeshMetrics tests ---
@Test

View File

@@ -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 <https://www.gnu.org/licenses/>.
*/
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<android.content.Context>()
database =
Room.inMemoryDatabaseBuilder<MeshtasticDatabase>(
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"
}
}

View File

@@ -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

View File

@@ -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<Packet>
/**
* 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()

View File

@@ -46,6 +46,12 @@ class FakeRadioController :
val sentPackets = mutableListOf<DataPacket>()
val favoritedNodes = mutableListOf<Int>()
val sentSharedContacts = mutableListOf<Int>()
/** Every [setLocalConfig] call, in order — lets tests assert e.g. that a scan restored the home LoRa preset. */
val localConfigs = mutableListOf<Config>()
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) {}

View File

@@ -1,11 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string-array name="car_hosts_allowlist">
<!-- Android Auto host (Google) -->
<item>com.google.android.projection.gearhead</item>
<!-- Android Automotive OS system host -->
<item>com.android.car.carlauncher</item>
<!-- Android Auto for Phone Screens (testing) -->
<item>com.google.android.apps.auto</item>
<!--
Hosts allowed to bind to the Meshtastic CarAppService.
Each entry MUST be "sha256SigningCertDigest,packageName" — androidx.car.app's HostValidator
rejects bare package names (it splits on ',' and throws IllegalArgumentException unless the
entry yields exactly two parts). Values are copied verbatim from androidx.car.app:1.9.0-alpha01's
bundled hosts_allowlist_sample (Google's official Android Auto / AAOS signing-cert digests);
re-sync them whenever the car-app library version is bumped.
-->
<string-array name="car_hosts_allowlist" translatable="false">
<!-- Android Auto (com.google.android.projection.gearhead) -->
<item>fdb00c43dbde8b51cb312aa81d3b5fa17713adb94b28f598d77f8eb89daceedf,com.google.android.projection.gearhead</item>
<item>70811a3eacfd2e83e18da9bfede52df16ce91f2e69a44d21f18ab66991130771,com.google.android.projection.gearhead</item>
<item>1975b2f17177bc89a5dff31f9e64a6cae281a53dc1d1d59b1d147fe1c82afa00,com.google.android.projection.gearhead</item>
<!-- Android Automotive OS template host (com.google.android.apps.automotive.templates.host) -->
<item>c241ffbc8e287c4e9a4ad19632ba1b1351ad361d5177b7d7b29859bd2b7fc631,com.google.android.apps.automotive.templates.host</item>
<item>dd66deaf312d8daec7adbe85a218ecc8c64f3b152f9b5998d5b29300c2623f61,com.google.android.apps.automotive.templates.host</item>
<item>50e603d333c6049a37bd751375d08f3bd0abebd33facd30bd17b64b89658b421,com.google.android.apps.automotive.templates.host</item>
</string-array>
</resources>

View File

@@ -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) {

View File

@@ -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
}

View File

@@ -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