fix(node): chart environment metrics in the user's display units (#6634)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
James RichandClaude Fable 5 authored and GitHub committed 2026-08-12 02:10:28 +00:00
1 parent bfb66eb988
commit 4e30fb5cd5
5 files changed
+186 -6

No files matched your search

@@ -24,6 +24,9 @@ object UnitConversions {
@Suppress("MagicNumber")
fun celsiusToFahrenheit(celsius: Float): Float = (celsius * 1.8F) + 32
@Suppress("MagicNumber")
fun metersPerSecondToMph(metersPerSecond: Float): Float = metersPerSecond * 2.23694f
/** Formats temperature as a string with the unit suffix. */
fun Float.toTempString(isFahrenheit: Boolean): String {
if (this.isNaN()) return "--"
@@ -0,0 +1,35 @@
/*
* 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.util
import kotlin.test.Test
import kotlin.test.assertEquals
class UnitConversionsTest {
@Test
fun celsiusToFahrenheit() {
assertEquals(32f, UnitConversions.celsiusToFahrenheit(0f))
assertEquals(212f, UnitConversions.celsiusToFahrenheit(100f))
}
@Test
fun metersPerSecondToMph() {
assertEquals(0f, UnitConversions.metersPerSecondToMph(0f))
assertEquals(22.3694f, UnitConversions.metersPerSecondToMph(10f), absoluteTolerance = 0.001f)
}
}
@@ -38,6 +38,7 @@ import com.patrykandpatrick.vico.compose.cartesian.layer.rememberLineCartesianLa
import com.patrykandpatrick.vico.compose.common.data.ExtraStore
import org.jetbrains.compose.resources.stringResource
import org.meshtastic.core.common.util.formatString
import org.meshtastic.core.model.util.UnitConversions
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.baro_pressure
import org.meshtastic.core.resources.humidity
@@ -154,12 +155,45 @@ internal fun pressureAxisRange(dataMin: Double, dataMax: Double): Pair<Double, D
else -> PRESSURE_DEFAULT_MIN to PRESSURE_DEFAULT_MAX
}
/**
* Wind speed arrives in m/s; imperial locales chart it in mph to match the cards (design §10.5). Temperatures are
* already converted upstream by the view model, so they pass through here unchanged.
*/
internal fun chartValue(metric: Environment, telemetry: Telemetry, isImperial: Boolean): Float? =
metric.getValue(telemetry)?.let {
if (metric == Environment.WIND_SPEED && isImperial) UnitConversions.metersPerSecondToMph(it) else it
}
/**
* Unit suffix for a plotted metric's axis and marker labels, in the user's display units, or "" for metrics whose unit
* would be noise on a shared axis. Includes any leading space, so it appends directly to a formatted value.
*/
internal fun unitSuffix(metric: Environment, isFahrenheit: Boolean, isImperial: Boolean): String = when (metric) {
Environment.TEMPERATURE,
Environment.SOIL_TEMPERATURE,
Environment.ONE_WIRE_TEMP_1,
Environment.ONE_WIRE_TEMP_2,
Environment.ONE_WIRE_TEMP_3,
Environment.ONE_WIRE_TEMP_4,
Environment.ONE_WIRE_TEMP_5,
Environment.ONE_WIRE_TEMP_6,
Environment.ONE_WIRE_TEMP_7,
Environment.ONE_WIRE_TEMP_8,
-> if (isFahrenheit) "°F" else "°C"
Environment.WIND_SPEED -> if (isImperial) " mph" else " m/s"
else -> ""
}
@Suppress("LongMethod", "CyclomaticComplexMethod")
@Composable
fun EnvironmentMetricsChart(
modifier: Modifier = Modifier,
telemetries: List<Telemetry>,
graphData: EnvironmentGraphingData,
isFahrenheit: Boolean,
isImperial: Boolean,
vicoScrollState: VicoScrollState,
selectedX: Double?,
onPointSelected: (Double) -> Unit,
@@ -186,6 +220,11 @@ fun EnvironmentMetricsChart(
}
val colorToLabel = allLegendData.associate { it.color to (it.labelOverride ?: stringResource(it.nameRes)) }
val colorToUnit =
allLegendData.associate { legend ->
val metric = legend.metricKey as? Environment
legend.color to (metric?.let { unitSuffix(it, isFahrenheit, isImperial) } ?: "")
}
val showPressure =
shouldPlot[Environment.BAROMETRIC_PRESSURE.ordinal] && Environment.BAROMETRIC_PRESSURE !in hiddenMetrics
@@ -221,7 +260,7 @@ fun EnvironmentMetricsChart(
}
}
LaunchedEffect(pressureData, otherMetricsData) {
LaunchedEffect(pressureData, otherMetricsData, isImperial) {
modelProducer.runTransaction {
/* Pressure on its own layer/axis */
if (showPressure && pressureData.isNotEmpty()) {
@@ -237,7 +276,10 @@ fun EnvironmentMetricsChart(
val metricData = otherMetricsData[metric] ?: emptyList()
if (metricData.isNotEmpty()) {
lineModel {
series(x = metricData.map { it.time }, y = metricData.map { metric.getValue(it)!! })
series(
x = metricData.map { it.time },
y = metricData.map { chartValue(metric, it, isImperial)!! },
)
}
}
}
@@ -249,7 +291,7 @@ fun EnvironmentMetricsChart(
valueFormatter =
ChartStyling.createColoredMarkerValueFormatter { value, color ->
val label = colorToLabel[color] ?: ""
formatString("%s: %.1f", label, value)
formatString("%s: %.1f", label, value) + (colorToUnit[color] ?: "")
},
)
@@ -322,9 +364,14 @@ fun EnvironmentMetricsChart(
},
endAxis =
if (otherMetrics.isNotEmpty()) {
// The end axis is shared, so it can only carry a unit when every metric on it uses the same
// one.
val endAxisUnit =
otherMetrics.map { unitSuffix(it, isFahrenheit, isImperial) }.distinct().singleOrNull()
?: ""
VerticalAxis.rememberEnd(
label = ChartStyling.rememberAxisLabel(color = endAxisColor),
valueFormatter = { _, value, _ -> formatString("%.0f", value) },
valueFormatter = { _, value, _ -> formatString("%.0f", value) + endAxisUnit },
)
} else {
null
@@ -85,6 +85,8 @@ fun EnvironmentMetricsScreen(viewModel: MetricsViewModel, onNavigateUp: () -> Un
viewModel.saveEnvironmentMetricsCSV(uri, filteredTelemetries)
}
val isImperial = state.displayUnits == org.meshtastic.proto.Config.DisplayConfig.DisplayUnits.IMPERIAL
BaseMetricScreen(
onNavigateUp = onNavigateUp,
telemetryType = TelemetryType.ENVIRONMENT,
@@ -108,6 +110,8 @@ fun EnvironmentMetricsScreen(viewModel: MetricsViewModel, onNavigateUp: () -> Un
modifier = modifier,
telemetries = filteredTelemetries.reversed(),
graphData = graphData,
isFahrenheit = state.isFahrenheit,
isImperial = isImperial,
vicoScrollState = vicoScrollState,
selectedX = selectedX,
onPointSelected = onPointSelected,
@@ -123,8 +127,7 @@ fun EnvironmentMetricsScreen(viewModel: MetricsViewModel, onNavigateUp: () -> Un
EnvironmentMetricsCard(
telemetry = telemetry,
environmentDisplayFahrenheit = state.isFahrenheit,
isImperial =
state.displayUnits == org.meshtastic.proto.Config.DisplayConfig.DisplayUnits.IMPERIAL,
isImperial = isImperial,
isSelected = telemetry.time.toDouble() == selectedX,
onClick = { onCardClick(telemetry.time.toDouble()) },
)
@@ -0,0 +1,92 @@
/*
* 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.core.common.util.nowSeconds
import org.meshtastic.proto.EnvironmentMetrics
import org.meshtastic.proto.Telemetry
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
@Suppress("MagicNumber")
class EnvironmentChartUnitsTest {
private fun telemetry(env: EnvironmentMetrics) = Telemetry(time = nowSeconds.toInt(), environment_metrics = env)
// ---- chartValue ----
@Test
fun windSpeedMetricStaysMetersPerSecond() {
val t = telemetry(EnvironmentMetrics(wind_speed = 10f))
assertEquals(10f, chartValue(Environment.WIND_SPEED, t, isImperial = false)!!, 0.001f)
}
@Test
fun windSpeedImperialIsConvertedToMph() {
val t = telemetry(EnvironmentMetrics(wind_speed = 10f))
assertEquals(22.3694f, chartValue(Environment.WIND_SPEED, t, isImperial = true)!!, 0.001f)
}
/** 0 m/s is a real reading (dead calm), so it must survive conversion rather than read as missing. */
@Test
fun windSpeedZeroIsPreserved() {
val t = telemetry(EnvironmentMetrics(wind_speed = 0f))
assertEquals(0f, chartValue(Environment.WIND_SPEED, t, isImperial = true)!!, 0.001f)
}
@Test
fun missingWindSpeedStaysNull() {
val t = telemetry(EnvironmentMetrics())
assertNull(chartValue(Environment.WIND_SPEED, t, isImperial = true))
}
/** Temperatures are converted upstream by the view model, so the chart must not convert them again. */
@Test
fun temperatureIsNotConvertedHere() {
val t = telemetry(EnvironmentMetrics(temperature = 20f))
assertEquals(20f, chartValue(Environment.TEMPERATURE, t, isImperial = true)!!, 0.001f)
}
// ---- unitSuffix ----
@Test
fun windSpeedSuffixFollowsDisplayUnits() {
assertEquals(" m/s", unitSuffix(Environment.WIND_SPEED, isFahrenheit = false, isImperial = false))
assertEquals(" mph", unitSuffix(Environment.WIND_SPEED, isFahrenheit = false, isImperial = true))
}
@Test
fun temperatureSuffixFollowsFahrenheitSetting() {
assertEquals("°C", unitSuffix(Environment.TEMPERATURE, isFahrenheit = false, isImperial = false))
assertEquals("°F", unitSuffix(Environment.TEMPERATURE, isFahrenheit = true, isImperial = false))
assertEquals("°F", unitSuffix(Environment.SOIL_TEMPERATURE, isFahrenheit = true, isImperial = false))
assertEquals("°F", unitSuffix(Environment.ONE_WIRE_TEMP_8, isFahrenheit = true, isImperial = false))
}
@Test
fun sharedAxisMetricsHaveNoSuffix() {
assertEquals("", unitSuffix(Environment.HUMIDITY, isFahrenheit = true, isImperial = true))
assertEquals("", unitSuffix(Environment.IAQ, isFahrenheit = true, isImperial = true))
assertEquals("", unitSuffix(Environment.LUX, isFahrenheit = true, isImperial = true))
}
}