Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .coderabbit/ast-grep-rules/no-float-metric-zero-sentinel.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Recurring defect class A — see "Presence vs sentinel zero" in .skills/code-review/SKILL.md.
#
# The protobuf models are Wire-generated, so presence IS nullability: EnvironmentMetrics.temperature and friends are
# declared `Float? = null` and there is no hasX() accessor. A `(x ?: 0f) != 0f` guard collapses "absent" and
# "measured zero" into one state and silently discards a real reading. Use `x?.let { ... }` instead — see
# `gatherSensors` in core/ui/.../NodeItem.kt for the reference pattern.
#
# Humidity is the deliberate exception: 0 %RH is not physically reachable, so relative_humidity / co2_humidity keep
# their zero-guards and are not listed here. Same for barometric_pressure (0 hPa is a vacuum).
id: no-float-metric-zero-sentinel
language: kotlin
severity: error
message: "0 is a real reading on this scale — use `?.let { }` instead of a zero-guard."
# Every spelling of the zero-float literal is listed: a reformat from `0f` to `0.0f` must not slip past the rule.
rule:
any:
- pattern: "($X ?: 0f) != 0f"
- pattern: "($X ?: 0f) == 0f"
- pattern: "($X ?: 0F) != 0F"
- pattern: "($X ?: 0F) == 0F"
- pattern: "($X ?: 0.0f) != 0.0f"
- pattern: "($X ?: 0.0f) == 0.0f"
- pattern: "($X ?: 0.0F) != 0.0F"
- pattern: "($X ?: 0.0F) == 0.0F"
constraints:
X:
regex: "(temperature|voltage|current|soil_moisture)$"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
20 changes: 20 additions & 0 deletions .coderabbit/ast-grep-rules/no-rssi-zero-default.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Recurring defect class A — see "Presence vs sentinel zero" in .skills/code-review/SKILL.md.
#
# 0 dBm is the STRONGEST value on the RSSI scale, so defaulting a missing reading to 0 renders an unknown signal as an
# excellent one. Keep the value nullable end-to-end and let `MetricFormatter.rssi(null)` render an em dash.
#
# Scope: this rule targets DEFAULTING a live reading (`?: 0`), not comparing a stored one. A `rssi == 0` test against
# persisted data can be legitimate migration handling — `Reaction.kt` reads pre-schema-51 rows that stored 0 where the
# column is now nullable, so there a 0 really is indistinguishable from "no reading". Broadening this rule to `== 0`
# would flag that documented exception and nothing else, so it deliberately stops at the `?: 0` form.
id: no-rssi-zero-default
language: kotlin
severity: error
message: "0 dBm is the strongest RSSI, not \"unknown\" — keep the value nullable."
rule:
any:
- pattern: "$X ?: 0"
- pattern: "($X ?: 0) != 0"
constraints:
X:
regex: "(?i)rssi$"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ interface BleDevice {
val rssi: Int?
get() = null

/** Reads the current RSSI value. */
suspend fun readRssi(): Int
/**
* Reads the current RSSI value in dBm, or `null` when no reading is available (no live connection and no scan
* advertisement). 0 dBm is the strongest value on this scale, so it must never stand in for "unknown".
*/
suspend fun readRssi(): Int?

/** Bond the device. */
suspend fun bond()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ class MeshtasticBleDevice(
override val rssi: Int? = advertisement?.rssi

@OptIn(ExperimentalKableApi::class)
override suspend fun readRssi(): Int {
override suspend fun readRssi(): Int? {
val active = ActiveBleConnection.active
return if (active != null && active.address == address) {
active.peripheral.rssi()
} else {
advertisement?.rssi ?: 0
advertisement?.rssi
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* 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.ble

import com.juul.kable.Advertisement
import dev.mokkery.MockMode
import dev.mokkery.answering.returns
import dev.mokkery.every
import dev.mokkery.mock
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull

/**
* 0 dBm is the strongest value on the RSSI scale, so a bonded-only device with no advertisement must report `null`
* rather than collapse into an excellent-signal reading. Both cases are pinned together.
*/
class MeshtasticBleDeviceRssiTest {

@Test
fun `advertised zero rssi is reported`() = runTest {
val advertisement: Advertisement = mock(MockMode.autofill) { every { rssi } returns 0 }
val device = MeshtasticBleDevice(address = ADDRESS, advertisement = advertisement)

assertEquals(0, device.rssi)
assertEquals(0, device.readRssi())
}

@Test
fun `bonded-only device without an advertisement reports no rssi`() = runTest {
val device = MeshtasticBleDevice(address = ADDRESS)

assertNull(device.rssi)
assertNull(device.readRssi())
}

private companion object {
const val ADDRESS = "AA:BB:CC:DD:EE:FF"
}
}
28 changes: 7 additions & 21 deletions core/model/src/commonMain/kotlin/org/meshtastic/core/model/Node.kt
Original file line number Diff line number Diff line change
Expand Up @@ -142,30 +142,16 @@ data class Node(

fun gpsString(): String = GPSFormat.toDec(latitude, longitude)

@Suppress("CyclomaticComplexMethod")
private fun EnvironmentMetrics.getDisplayStrings(isFahrenheit: Boolean): List<String> {
val temp =
if ((temperature ?: 0f) != 0f) {
MetricFormatter.temperature(temperature ?: 0f, isFahrenheit)
} else {
null
}
// These fields carry presence: `null` means "no sensor", so 0 °C / 0 V / 0 A / 0% are real readings and must
// still render. Humidity keeps its zero-guard because 0% RH is not physically reachable.
val temp = temperature?.let { MetricFormatter.temperature(it, isFahrenheit) }
val humidity = if ((relative_humidity ?: 0f) != 0f) MetricFormatter.humidity(relative_humidity ?: 0f) else null
val soilTemperatureStr =
if ((soil_temperature ?: 0f) != 0f) {
MetricFormatter.temperature(soil_temperature ?: 0f, isFahrenheit)
} else {
null
}
val soilTemperatureStr = soil_temperature?.let { MetricFormatter.temperature(it, isFahrenheit) }
val soilMoistureRange = 0..100
val soilMoisture =
if ((soil_moisture ?: Int.MIN_VALUE) in soilMoistureRange && (soil_temperature ?: 0f) != 0f) {
MetricFormatter.percent(soil_moisture ?: 0)
} else {
null
}
val voltage = if ((this.voltage ?: 0f) != 0f) MetricFormatter.voltage(this.voltage ?: 0f) else null
val current = if ((current ?: 0f) != 0f) MetricFormatter.current(current ?: 0f) else null
val soilMoisture = soil_moisture?.takeIf { it in soilMoistureRange }?.let { MetricFormatter.percent(it) }
val voltage = this.voltage?.let { MetricFormatter.voltage(it) }
val current = current?.let { MetricFormatter.current(it) }
val iaq = if ((iaq ?: 0) != 0) "IAQ: $iaq" else null

return listOfNotNull(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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.model

import org.meshtastic.proto.EnvironmentMetrics
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue

/**
* `EnvironmentMetrics` fields are Wire-generated and nullable, so `null` is the only "not reported" signal. Each metric
* is pinned twice — absent and measured-zero — because either assertion alone lets the two states collapse back into
* one.
*/
class NodeTelemetryStringsTest {

private fun telemetry(metrics: EnvironmentMetrics) =
Node(num = 1, environmentMetrics = metrics).getTelemetryStrings()

@Test
fun absent_environment_metrics_render_nothing() {
assertEquals(emptyList(), telemetry(EnvironmentMetrics()))
}

@Test
fun zero_temperature_is_reported() {
assertEquals(listOf("0.0°C"), telemetry(EnvironmentMetrics(temperature = 0f)))
}

@Test
fun zero_voltage_and_current_are_reported() {
val strings = telemetry(EnvironmentMetrics(voltage = 0f, current = 0f))
assertEquals(listOf("0.00 V", "0.0 mA"), strings)
}

@Test
fun zero_soil_readings_are_reported() {
val strings = telemetry(EnvironmentMetrics(soil_temperature = 0f, soil_moisture = 0))
assertEquals(listOf("0.0°C", "0%"), strings)
}

@Test
fun soil_moisture_no_longer_requires_a_soil_temperature() {
assertEquals(listOf("42%"), telemetry(EnvironmentMetrics(soil_moisture = 42)))
}

@Test
fun out_of_range_soil_moisture_is_still_rejected() {
assertEquals(emptyList(), telemetry(EnvironmentMetrics(soil_moisture = 101)))
}

@Test
fun zero_humidity_stays_filtered() {
// 0 %RH is not physically reachable, so unlike the other metrics its zero-guard is intentional.
assertTrue(telemetry(EnvironmentMetrics(relative_humidity = 0f)).isEmpty())
assertEquals(listOf("41%"), telemetry(EnvironmentMetrics(relative_humidity = 41f)))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,10 @@ class BleRadioTransport(
try {
bleConnection.deviceFlow.first()?.let { device ->
val rssi = retryBleOperation(tag = address) { device.readRssi() }
Logger.d { "[$address] Connection confirmed. Initial RSSI: $rssi dBm" }
Logger.d {
"[${address.anonymize()}] Connection confirmed. " +
"Initial RSSI: ${rssi?.let { "$it dBm" } ?: "unknown"}"
}
}
} catch (e: CancellationException) {
throw e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class FakeBleDevice(
override val isConnected: Boolean
get() = _state.value == BleConnectionState.Connected

override suspend fun readRssi(): Int = DEFAULT_RSSI
override suspend fun readRssi(): Int? = rssi

override suspend fun bond() {
_isBonded.value = true
Expand All @@ -65,10 +65,6 @@ class FakeBleDevice(
fun setState(newState: BleConnectionState) {
_state.value = newState
}

companion object {
private const val DEFAULT_RSSI = -60
}
}

class FakeBleScanner :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,26 +389,33 @@ private fun gatherSensors(node: Node, tempInFahrenheit: Boolean, contentColor: C
)
}
}
if ((env.soil_temperature ?: 0f) != 0f) {
val temp = MetricFormatter.temperature(env.soil_temperature ?: 0f, tempInFahrenheit)
// Soil temperature, moisture, voltage and current all carry presence: `null` is "no sensor", 0 is a real reading.
env.soil_temperature?.let { soilTemperature ->
val temp = MetricFormatter.temperature(soilTemperature, tempInFahrenheit)
items.add { SoilTemperatureInfo(temp = temp, contentColor = contentColor) }
}
if ((env.soil_moisture ?: 0) != 0 && (env.soil_temperature ?: 0f) != 0f) {
items.add { SoilMoistureInfo(moisture = "${env.soil_moisture}%", contentColor = contentColor) }
}
if ((env.voltage ?: 0f) != 0f) {
// Range-checked to match Node.getTelemetryStrings — a sensor fault reporting 101% is not a reading.
val soilMoistureRange = 0..100
env.soil_moisture
?.takeIf { it in soilMoistureRange }
?.let { soilMoisture ->
items.add {
SoilMoistureInfo(moisture = MetricFormatter.percent(soilMoisture), contentColor = contentColor)
}
}
env.voltage?.let { voltage ->
items.add {
PowerInfo(
value = MetricFormatter.voltage(env.voltage ?: 0f),
value = MetricFormatter.voltage(voltage),
label = stringResource(Res.string.voltage),
contentColor = contentColor,
)
}
}
if ((env.current ?: 0f) != 0f) {
env.current?.let { current ->
items.add {
PowerInfo(
value = MetricFormatter.current(env.current ?: 0f),
value = MetricFormatter.current(current),
label = stringResource(Res.string.current),
contentColor = contentColor,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,10 @@ private fun CompactMetricsRow(thatNode: Node, tempInFahrenheit: Boolean, content
val env = thatNode.environmentMetrics
val segments =
buildList<@Composable () -> Unit> {
if ((env.temperature ?: 0f) != 0f) {
val temp = MetricFormatter.temperature(env.temperature ?: 0f, tempInFahrenheit)
// Temperature carries presence, so `null` already means "no sensor" — testing against 0 hid an ordinary
// 0 °C reading. Mirrors the fix already made in NodeItem's gatherSensors.
env.temperature?.let { temperature ->
val temp = MetricFormatter.temperature(temperature, tempInFahrenheit)
add {
IconInfo(
icon = MeshtasticIcons.Temperature,
Expand Down
Loading
Loading