fix(node): disambiguate signal-log packet keys (#6492)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
James RichandClaude Opus 5 authored and GitHub committed 2026-07-28 15:11:09 +00:00
1 parent 92e33006da
commit 72ca3313d8
2 files changed
+90 -11

No files matched your search

@@ -111,7 +111,14 @@ private val LEGEND_DATA =
LegendData(nameRes = Res.string.snr, color = SignalMetric.SNR.color),
)
private sealed interface SignalLogEntry {
/** Builds the combined signal log, newest first. Kept out of the composable so the LazyColumn keys are testable. */
internal fun buildSignalLog(signalData: List<MeshPacket>, localStatsData: List<Telemetry>): List<SignalLogEntry> {
val localStats = localStatsData.mapIndexed { index, telemetry -> SignalLogEntry.LocalStatsEntry(telemetry, index) }
val packets = signalData.mapIndexed { index, packet -> SignalLogEntry.PacketEntry(packet, index) }
return (localStats + packets).sortedByDescending { it.timeSeconds }
}
internal sealed interface SignalLogEntry {
val timeSeconds: Int
/** Stable, collision-free identity for use as a LazyColumn item key across both entry types. */
@@ -128,9 +135,12 @@ private sealed interface SignalLogEntry {
override val contentType: Any = "local_stats"
}
data class PacketEntry(val meshPacket: MeshPacket) : SignalLogEntry {
data class PacketEntry(val meshPacket: MeshPacket, val index: Int) : SignalLogEntry {
override val timeSeconds: Int = meshPacket.rx_time
override val key: Any = "packet_${meshPacket.id}"
// MeshPacket.id repeats: it is unique only per originating node, and retransmissions are stored per reception.
// The source-list index disambiguates, as it does for local stats.
override val key: Any = "packet_${meshPacket.id}_$index"
override val contentType: Any = "signal_packet"
}
}
@@ -144,14 +154,7 @@ fun SignalMetricsScreen(viewModel: MetricsViewModel, onNavigateUp: () -> Unit, m
val threshold = timeFrame.timeThreshold()
val signalData = state.signalMetrics.filter { it.rx_time.toLong() >= threshold }
val localStatsData = state.localStats.filter { it.time.toLong() >= threshold && it.local_stats != null }
val data =
remember(signalData, localStatsData) {
(
localStatsData.mapIndexed { index, telemetry -> SignalLogEntry.LocalStatsEntry(telemetry, index) } +
signalData.map { SignalLogEntry.PacketEntry(it) }
)
.sortedByDescending { it.timeSeconds }
}
val data = remember(signalData, localStatsData) { buildSignalLog(signalData, localStatsData) }
val hasNoiseFloor = remember(localStatsData) { localStatsData.any { it.local_stats?.noise_floor != 0 } }
val hasRssi = remember(signalData) { signalData.any { it.rx_rssi != 0 } }
val hasSnr = remember(signalData) { signalData.any { !it.rx_snr.isNaN() } }
@@ -0,0 +1,76 @@
/*
* 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.feature.node.metrics
import org.meshtastic.proto.MeshPacket
import org.meshtastic.proto.Telemetry
import kotlin.test.Test
import kotlin.test.assertEquals
/**
* Duplicate LazyColumn keys are a fatal `IllegalArgumentException`, and `MeshPacket.id` is unique only per originating
* node, with retransmissions stored per reception.
*/
class SignalLogKeyTest {
@Test
fun `repeated packet ids still produce distinct keys`() {
val packets = listOf(packet(id = 163275746), packet(id = 163275746), packet(id = 163275746))
val keys = buildSignalLog(signalData = packets, localStatsData = emptyList()).map { it.key }
assertEquals(3, keys.toSet().size, "duplicate LazyColumn keys crash the screen: $keys")
}
@Test
fun `a negative packet id is not conflated with its positive twin`() {
// Packet ids are signed in the proto, so the sign has to survive into the key. Both packets sit at index 0 so
// the index cannot be what separates them.
val negative = buildSignalLog(signalData = listOf(packet(id = -1344023121)), localStatsData = emptyList())
val positive = buildSignalLog(signalData = listOf(packet(id = 1344023121)), localStatsData = emptyList())
assertEquals("packet_-1344023121_0", negative.single().key)
assertEquals("packet_1344023121_0", positive.single().key)
}
@Test
fun `local stats and packets sharing a timestamp do not collide`() {
val keys =
buildSignalLog(
signalData = listOf(packet(id = 7, rxTime = 100), packet(id = 7, rxTime = 100)),
localStatsData = listOf(stats(time = 100), stats(time = 100)),
)
.map { it.key }
assertEquals(4, keys.toSet().size, "same-second entries across both types must stay distinct: $keys")
}
@Test
fun `entries are ordered newest first across both types`() {
val log =
buildSignalLog(
signalData = listOf(packet(id = 1, rxTime = 10), packet(id = 2, rxTime = 30)),
localStatsData = listOf(stats(time = 20)),
)
assertEquals(listOf(30, 20, 10), log.map { it.timeSeconds })
}
private fun packet(id: Int, rxTime: Int = 0) = MeshPacket(id = id, rx_time = rxTime)
private fun stats(time: Int) = Telemetry(time = time)
}