fix(ui): give rx_snr real presence semantics end to end (#6523)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
James RichandClaude Opus 5 authored and GitHub committed 2026-07-30 16:40:41 +00:00
1 parent 4846425ff1
commit 2d20cd8a47
46 files changed
+2194 -148

No files matched your search

@@ -37,7 +37,6 @@ import org.meshtastic.proto.Config.LoRaConfig.ModemPreset
private const val MILLIS_PER_SECOND = 1000L
private const val MAX_BATTERY_PERCENT = 100
private const val SNR_UNSET_THRESHOLD = 100f
/** Pre-resolved localized strings for TalkBack node descriptions. */
@Immutable
@@ -82,8 +81,7 @@ internal fun buildNodeDescription(
hopsAway: Int,
batteryLevel: Int?,
distance: String?,
snr: Float,
rssi: Int,
snr: Float?,
viaMqtt: Boolean,
strings: NodeDescriptionStrings,
lastHeardIsRelative: Boolean = true,
@@ -122,7 +120,9 @@ internal fun buildNodeDescription(
append(", ")
append(strings.distanceAway.replace("%s", it))
}
if (hopsAway == 0 && !viaMqtt && snr < SNR_UNSET_THRESHOLD && rssi < 0) {
// Rated from SNR alone: RSSI cannot indicate demodulability without the noise floor, and the old `rssi < 0` gate
// suppressed the announcement for a genuine 0 dBm reading.
if (hopsAway == 0 && !viaMqtt && snr != null) {
val quality = determineSignalQuality(snr, modemPreset)
append(", ")
append(strings.signal.replace("%s", quality.name.lowercase()))
@@ -20,11 +20,7 @@ package org.meshtastic.core.ui.component
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.FlowRow
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.material3.Icon
@@ -56,6 +52,7 @@ import org.meshtastic.core.resources.rssi
import org.meshtastic.core.resources.signal
import org.meshtastic.core.resources.signal_quality
import org.meshtastic.core.resources.snr
import org.meshtastic.core.resources.unknown
import org.meshtastic.core.ui.theme.StatusColors.StatusGreen
import org.meshtastic.core.ui.theme.StatusColors.StatusOrange
import org.meshtastic.core.ui.theme.StatusColors.StatusRed
@@ -88,60 +85,22 @@ enum class Quality(
GOOD(Res.string.good, Res.drawable.ic_signal_cellular_4_bar, { colorScheme.StatusGreen }),
}
/**
* Displays the `snr` and `rssi` color coded based on the signal quality, along with a human readable description and
* related icon.
*/
@OptIn(ExperimentalLayoutApi::class)
@Composable
fun NodeSignalQuality(
snr: Float,
rssi: Int?,
modifier: Modifier = Modifier,
modemPreset: ModemPreset? = LocalModemPreset.current,
) {
val quality = determineSignalQuality(snr, modemPreset)
FlowRow(
modifier = modifier,
itemVerticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.SpaceBetween,
) {
Snr(snr, modemPreset = modemPreset)
Rssi(rssi)
Text(
text = "${stringResource(Res.string.signal)} ${stringResource(quality.nameRes)}",
style = MaterialTheme.typography.labelSmall,
maxLines = 1,
)
Icon(
modifier = Modifier.size(SIZE_ICON_DP.dp),
imageVector = vectorResource(quality.icon),
contentDescription = stringResource(Res.string.signal_quality),
tint = quality.color(),
)
}
}
private const val SIZE_ICON_DP = 16
/** Displays the `snr` and `rssi` with color depending on the values respectively. */
@Composable
fun SnrAndRssi(snr: Float, rssi: Int?, modemPreset: ModemPreset? = LocalModemPreset.current) {
Row(modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.SpaceBetween) {
Snr(snr, modemPreset = modemPreset)
Rssi(rssi)
}
}
/** Displays a human readable description and icon representing the signal quality. */
/**
* Displays a human readable description and icon representing the signal quality.
*
* A null [snr] means the packet carried no measurement, which is rendered as "Unknown" in a neutral tint. It must not
* fall through to [Quality.NONE] — that band means "measured, and too weak to demodulate", a different claim.
*/
@Composable
fun LoraSignalIndicator(
snr: Float,
snr: Float?,
modifier: Modifier = Modifier,
modemPreset: ModemPreset? = LocalModemPreset.current,
contentColor: Color = MaterialTheme.colorScheme.onSurface,
) {
val quality = determineSignalQuality(snr, modemPreset)
val quality = snr?.let { determineSignalQuality(it, modemPreset) }
Column(
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally,
@@ -149,20 +108,22 @@ fun LoraSignalIndicator(
) {
Icon(
modifier = Modifier.size(SIZE_ICON_DP.dp),
imageVector = vectorResource(quality.icon),
imageVector = vectorResource(quality?.icon ?: Res.drawable.ic_signal_cellular_alt),
contentDescription = stringResource(Res.string.signal_quality),
tint = quality.color(),
tint = quality?.color?.invoke() ?: MaterialTheme.colorScheme.onSurfaceVariant,
)
Text(
text = "${stringResource(Res.string.signal)} ${stringResource(quality.nameRes)}",
text = "${stringResource(Res.string.signal)} " + stringResource(quality?.nameRes ?: Res.string.unknown),
style = MaterialTheme.typography.labelSmall,
color = contentColor,
)
}
}
/** Renders nothing when [snr] is absent — 0 dB is a real reading, so it must not stand in for "no reading". */
@Composable
fun Snr(snr: Float, modifier: Modifier = Modifier, modemPreset: ModemPreset? = LocalModemPreset.current) {
fun Snr(snr: Float?, modifier: Modifier = Modifier, modemPreset: ModemPreset? = LocalModemPreset.current) {
if (snr == null) return
val color: Color = determineSignalQuality(snr, modemPreset).color.invoke()
Text(
@@ -145,8 +145,7 @@ fun NodeItem(
hopsAway = thatNode.hopsAway,
batteryLevel = thatNode.batteryLevel,
distance = distance,
snr = thatNode.snr,
rssi = thatNode.rssi,
snr = thatNode.snrOrNull,
viaMqtt = thatNode.viaMqtt,
strings = a11yStrings,
modemPreset = modemPreset,
@@ -312,9 +311,9 @@ private fun NodeSignalRow(thatNode: Node, isThisNode: Boolean, contentColor: Col
if (thatNode.hopsAway > 0) {
add { HopsInfo(hops = thatNode.hopsAway, contentColor = contentColor) }
} else if (thatNode.hopsAway == 0 && !thatNode.viaMqtt) {
val showSnr = thatNode.snr < 100f
val showRssi = thatNode.rssi < 0
if (showSnr || showRssi) {
val snr = thatNode.snrOrNull
val rssi = thatNode.rssiOrNull
if (snr != null || rssi != null) {
signalChip = {
// Full-width row: SNR left, RSSI center, quality right.
Row(
@@ -322,10 +321,10 @@ private fun NodeSignalRow(thatNode: Node, isThisNode: Boolean, contentColor: Col
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.SpaceBetween,
) {
if (showSnr) Snr(thatNode.snr)
if (showRssi) Rssi(thatNode.rssi)
if (showSnr && showRssi) {
val quality = determineSignalQuality(thatNode.snr, LocalModemPreset.current)
Snr(snr)
Rssi(rssi)
if (snr != null) {
val quality = determineSignalQuality(snr, LocalModemPreset.current)
IconInfo(
icon = vectorResource(quality.icon),
contentDescription = stringResource(Res.string.signal_quality),
@@ -155,8 +155,7 @@ fun NodeItemCompact(
hopsAway = thatNode.hopsAway,
batteryLevel = thatNode.batteryLevel,
distance = distance,
snr = thatNode.snr,
rssi = thatNode.rssi,
snr = thatNode.snrOrNull,
viaMqtt = thatNode.viaMqtt,
strings = a11yStrings,
lastHeardIsRelative = lastHeardIsRelative,
@@ -350,10 +349,10 @@ private fun CompactHealthRow(
)
}
// Signal quality
val hasDirectSignal = thatNode.hopsAway == 0 && thatNode.snr < 100f && !thatNode.viaMqtt && thatNode.rssi < 0
if (showSignal && hasDirectSignal) {
val quality = determineSignalQuality(thatNode.snr, LocalModemPreset.current)
// Signal quality, rated from SNR alone — RSSI is not part of the rating (#5446), so it must not gate it.
val directSnr = thatNode.snrOrNull?.takeIf { thatNode.hopsAway == 0 && !thatNode.viaMqtt }
if (showSignal && directSnr != null) {
val quality = determineSignalQuality(directSnr, LocalModemPreset.current)
add(
@Composable {
IconInfo(
@@ -42,17 +42,21 @@ import org.meshtastic.core.ui.component.preview.NodePreviewParameterProvider
import org.meshtastic.core.ui.theme.AppTheme
import org.meshtastic.core.ui.util.LocalModemPreset
const val MAX_VALID_SNR = 100F
const val MAX_VALID_RSSI = 0
/**
* Renders the node's signal quality, or nothing when it has no SNR reading to rate.
*
* Presence comes from [Node.snrOrNull]/[Node.rssiOrNull], not from threshold comparisons: the previous `rssi < 0` gate
* hid the whole row for a genuine 0 dBm reading, and `snr < 100f` would have hidden any reading at or above 100 dB.
*/
@Composable
fun SignalInfo(
modifier: Modifier = Modifier,
node: Node,
@Suppress("UNUSED_PARAMETER") contentColor: Color = MaterialTheme.colorScheme.onSurface,
) {
if (node.snr < MAX_VALID_SNR && node.rssi < MAX_VALID_RSSI) {
val quality = determineSignalQuality(node.snr, LocalModemPreset.current)
val snr = node.snrOrNull
if (snr != null) {
val quality = determineSignalQuality(snr, LocalModemPreset.current)
val signalColor = quality.color.invoke()
Row(
modifier = modifier,
@@ -67,9 +71,8 @@ fun SignalInfo(
)
Text(
text =
"${MetricFormatter.snr(
node.snr,
)} · ${MetricFormatter.rssi(node.rssi)} · ${stringResource(quality.nameRes)}",
"${MetricFormatter.snr(snr)} · ${MetricFormatter.rssi(node.rssiOrNull)} · " +
stringResource(quality.nameRes),
style =
MaterialTheme.typography.labelSmall.copy(
fontWeight = FontWeight.Bold,
@@ -48,8 +48,7 @@ class BuildNodeDescriptionTest {
hopsAway: Int = 0,
batteryLevel: Int? = null,
distance: String? = null,
snr: Float = Float.MAX_VALUE,
rssi: Int = 0,
snr: Float? = null,
viaMqtt: Boolean = false,
lastHeardIsRelative: Boolean = true,
): String = buildNodeDescription(
@@ -62,7 +61,6 @@ class BuildNodeDescriptionTest {
batteryLevel = batteryLevel,
distance = distance,
snr = snr,
rssi = rssi,
viaMqtt = viaMqtt,
strings = testStrings,
lastHeardIsRelative = lastHeardIsRelative,
@@ -157,32 +155,33 @@ class BuildNodeDescriptionTest {
// ---- Signal ----
@Test
fun signal_hidden_when_snr_is_max_float() {
val result = describe(snr = Float.MAX_VALUE, rssi = -100, hopsAway = 0, viaMqtt = false)
fun signal_hidden_when_snr_is_absent() {
val result = describe(snr = null, hopsAway = 0, viaMqtt = false)
assertFalse(result.contains("signal"))
}
@Test
fun signal_hidden_when_via_mqtt() {
val result = describe(snr = -5f, rssi = -100, hopsAway = 0, viaMqtt = true)
val result = describe(snr = -5f, hopsAway = 0, viaMqtt = true)
assertFalse(result.contains("signal"))
}
@Test
fun signal_hidden_when_hops_greater_than_zero() {
val result = describe(snr = -5f, rssi = -100, hopsAway = 1, viaMqtt = false)
val result = describe(snr = -5f, hopsAway = 1, viaMqtt = false)
assertFalse(result.contains("signal"))
}
@Test
fun signal_hidden_when_rssi_not_negative() {
val result = describe(snr = -5f, rssi = 0, hopsAway = 0, viaMqtt = false)
assertFalse(result.contains("signal"))
fun signal_shown_for_a_zero_snr_reading() {
// 0 dB is a real, strong reading. It was previously announced only when RSSI happened to be negative.
val result = describe(snr = 0f, hopsAway = 0, viaMqtt = false)
assertContains(result, "signal")
}
@Test
fun signal_shown_when_direct_and_valid_values() {
val result = describe(snr = -5f, rssi = -100, hopsAway = 0, viaMqtt = false)
val result = describe(snr = -5f, hopsAway = 0, viaMqtt = false)
assertContains(result, "signal")
}
}
@@ -77,6 +77,22 @@ class LoraSignalIndicatorTest {
assertEquals(Quality.NONE, determineSignalQuality(snr = -30f, modemPreset = preset)) // < limit-7.5
}
@Test
fun `a zero SNR reading is rated rather than treated as missing`() {
// 0 dB sits well above every preset's demod floor, so it is an excellent signal — not an absent one. If a
// presence check ever folds zero into "unknown", this is the reading that disappears.
assertEquals(Quality.GOOD, determineSignalQuality(snr = 0f, modemPreset = ModemPreset.LONG_FAST))
assertEquals(Quality.GOOD, determineSignalQuality(snr = 0f, modemPreset = ModemPreset.SHORT_FAST))
}
@Test
fun `absent SNR is not a quality band`() {
// Quality has no member for "no measurement": callers must pass a non-null SNR, and the composables render
// absence as Unknown rather than mapping it onto NONE (which asserts a measured, undemodulable signal).
assertEquals(4, Quality.entries.size)
assertEquals(listOf(Quality.NONE, Quality.BAD, Quality.FAIR, Quality.GOOD), Quality.entries.toList())
}
@Test
fun `RSSI does not influence the rating`() {
// Identical SNR + preset always yields the same verdict regardless of any RSSI (RSSI is display-only now).
@@ -34,6 +34,29 @@ class LoraSignalIndicatorUiTest {
onNodeWithText("Signal strength -70 dBm").assertIsDisplayed()
}
@Test
fun snrRendersAZeroReading() = runComposeUiTest {
// 0 dB is a measurement and must be shown, not suppressed as "no reading".
setContent { AppTheme { Snr(snr = 0f) } }
onNodeWithText("SNR 0.00 dB").assertIsDisplayed()
}
@Test
fun snrRendersNothingWhenAbsent() = runComposeUiTest {
setContent { AppTheme { Snr(snr = null) } }
onNodeWithText("SNR 0.00 dB").assertDoesNotExist()
}
@Test
fun loraSignalIndicatorShowsUnknownWhenSnrIsAbsent() = runComposeUiTest {
// Absence must not render as "Signal None" — that band means a measured, undemodulable signal.
setContent { AppTheme { LoraSignalIndicator(snr = null) } }
onNodeWithText("Signal Unknown").assertIsDisplayed()
}
@Test
fun batteryUsesCallerProvidedUnknownLabel() = runComposeUiTest {
setContent { AppTheme { MaterialBatteryInfo(level = null, unknownLabel = "Unavailable") } }