diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt index 8f65bf6d8..a425e272d 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt @@ -35,6 +35,7 @@ import androidx.compose.material3.IconToggleButton import androidx.compose.material3.Scaffold import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope @@ -79,6 +80,9 @@ import org.meshtastic.core.ui.icon.MeshtasticIcons import org.meshtastic.core.ui.icon.Refresh import org.meshtastic.core.ui.icon.Save +/** Minimum x-step (in seconds) to prevent the default GCD from producing a value of 1 with irregular timestamps. */ +private const val MIN_X_STEP_SECONDS = 60.0 + /** * A generic chart host for Meshtastic metric charts. Handles common boilerplate for markers, scrolling, and point * selection synchronization. @@ -100,43 +104,50 @@ fun GenericMetricChart( onPointSelected: ((Double) -> Unit)? = null, vicoScrollState: VicoScrollState = rememberVicoScrollState(), ) { - // Hoist zoom state above rememberCartesianChart so that the variable slot count - // from the vararg layers spread does not shift this remember call during recomposition - // (toggling legend chips changes the layer count, which corrupts the slot table). - val zoomState = rememberVicoZoomState(zoomEnabled = true, initialZoom = Zoom.Content) + // Key on layer count so Compose rebuilds the entire subtree when legend chip toggles + // add/remove layers. rememberCartesianChart uses vararg internally, so changing the + // argument count without a key corrupts the slot table. + key(layers.size) { + val zoomState = rememberVicoZoomState(zoomEnabled = true, initialZoom = Zoom.Content) - val markerVisibilityListener = - remember(onPointSelected) { - object : CartesianMarkerVisibilityListener { - override fun onShown(marker: CartesianMarker, targets: List) { - targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) } - } + val markerVisibilityListener = + remember(onPointSelected) { + object : CartesianMarkerVisibilityListener { + override fun onShown(marker: CartesianMarker, targets: List) { + targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) } + } - override fun onUpdated(marker: CartesianMarker, targets: List) { - targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) } + override fun onUpdated(marker: CartesianMarker, targets: List) { + targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) } + } } } - } - CartesianChartHost( - chart = - @Suppress("SpreadOperator") - rememberCartesianChart( - *layers.toTypedArray(), - startAxis = startAxis, - endAxis = endAxis, - bottomAxis = bottomAxis, - marker = marker, - markerVisibilityListener = markerVisibilityListener, - persistentMarkers = { _ -> if (selectedX != null && marker != null) marker at selectedX else null }, - fadingEdges = rememberFadingEdges(), - decorations = decorations, - ), - modelProducer = modelProducer, - modifier = modifier, - scrollState = vicoScrollState, - zoomState = zoomState, - ) + CartesianChartHost( + chart = + @Suppress("SpreadOperator") + rememberCartesianChart( + *layers.toTypedArray(), + startAxis = startAxis, + endAxis = endAxis, + bottomAxis = bottomAxis, + marker = marker, + markerVisibilityListener = markerVisibilityListener, + persistentMarkers = { _ -> if (selectedX != null && marker != null) marker at selectedX else null }, + fadingEdges = rememberFadingEdges(), + decorations = decorations, + // Telemetry timestamps arrive at irregular intervals. Without an explicit + // x-step, Vico computes the GCD of consecutive x-value differences which can + // be as small as 1 second, making the chart logically enormous. A 60-second + // floor keeps the internal slot count reasonable for any practical interval. + getXStep = { model -> maxOf(model.getXDeltaGcd(), MIN_X_STEP_SECONDS) }, + ), + modelProducer = modelProducer, + modifier = modifier, + scrollState = vicoScrollState, + zoomState = zoomState, + ) + } } /** diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt index c1cf0e04e..da8b16e47 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt @@ -57,7 +57,7 @@ import com.patrykandpatrick.vico.compose.common.component.rememberTextComponent * **Design principles** (per [design#53](https://github.com/meshtastic/design/issues/53)): * - Default to thin lines **without** point markers to avoid clutter on dense timeseries. * - Show a single dot only at the marker/cursor position (handled by [rememberMarker]). - * - Use `Interpolator.catmullRom()` for smooth curves that pass through every data point. + * - Use `Interpolator.cubic()` for smooth monotone curves that won't overshoot between sparse points. * - Reserve bold lines for the single most-important series; use subtle/gradient fills for secondary data. */ @Suppress("TooManyFunctions") @@ -73,15 +73,21 @@ object ChartStyling { * * @param lineColor The color of the line * @param lineWidth Width of the line in dp + * @param interpolator The line interpolation strategy. Defaults to monotone + * [cubic][LineCartesianLayer.Interpolator.cubic] which won't overshoot between sparse data points (unlike + * catmull-rom). Use [Sharp][LineCartesianLayer.Interpolator.Sharp] for discrete/integer metrics like hop counts. * @return Configured [LineCartesianLayer.Line] */ @Composable - fun createStyledLine(lineColor: Color, lineWidth: Float = MEDIUM_LINE_WIDTH_DP): LineCartesianLayer.Line = - LineCartesianLayer.rememberLine( - fill = LineCartesianLayer.LineFill.single(Fill(lineColor)), - stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp), - interpolator = LineCartesianLayer.Interpolator.catmullRom(), - ) + fun createStyledLine( + lineColor: Color, + lineWidth: Float = MEDIUM_LINE_WIDTH_DP, + interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(), + ): LineCartesianLayer.Line = LineCartesianLayer.rememberLine( + fill = LineCartesianLayer.LineFill.single(Fill(lineColor)), + stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp), + interpolator = interpolator, + ) /** * Creates a line with a gradient area fill effect. Ideal for emphasising a single series or showing magnitude. The @@ -92,14 +98,18 @@ object ChartStyling { * @return Configured [LineCartesianLayer.Line] */ @Composable - fun createGradientLine(lineColor: Color, lineWidth: Float = MEDIUM_LINE_WIDTH_DP): LineCartesianLayer.Line { + fun createGradientLine( + lineColor: Color, + lineWidth: Float = MEDIUM_LINE_WIDTH_DP, + interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(), + ): LineCartesianLayer.Line { val gradientBrush = Brush.verticalGradient(colors = listOf(lineColor.copy(alpha = 0.3f), lineColor.copy(alpha = 0.05f))) return LineCartesianLayer.rememberLine( fill = LineCartesianLayer.LineFill.single(Fill(lineColor)), areaFill = LineCartesianLayer.AreaFill.single(Fill(gradientBrush)), stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp), - interpolator = LineCartesianLayer.Interpolator.catmullRom(), + interpolator = interpolator, ) } @@ -110,8 +120,11 @@ object ChartStyling { * @return Configured [LineCartesianLayer.Line] */ @Composable - fun createBoldLine(lineColor: Color): LineCartesianLayer.Line = - createStyledLine(lineColor = lineColor, lineWidth = THICK_LINE_WIDTH_DP) + fun createBoldLine( + lineColor: Color, + interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(), + ): LineCartesianLayer.Line = + createStyledLine(lineColor = lineColor, lineWidth = THICK_LINE_WIDTH_DP, interpolator = interpolator) /** * Creates a subtle line suitable for secondary metrics that should not dominate the chart. @@ -131,7 +144,10 @@ object ChartStyling { * @return Configured [LineCartesianLayer.Line] */ @Composable - fun createDashedLine(lineColor: Color): LineCartesianLayer.Line = LineCartesianLayer.rememberLine( + fun createDashedLine( + lineColor: Color, + interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(), + ): LineCartesianLayer.Line = LineCartesianLayer.rememberLine( fill = LineCartesianLayer.LineFill.single(Fill(lineColor)), stroke = LineCartesianLayer.LineStroke.Dashed( @@ -139,7 +155,7 @@ object ChartStyling { dashLength = 6.dp, gapLength = 3.dp, ), - interpolator = LineCartesianLayer.Interpolator.catmullRom(), + interpolator = interpolator, ) /** diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt index 1e749d22e..609048a92 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt @@ -307,12 +307,13 @@ private fun DeviceMetricsChart( } } + val percentRangeProvider = remember { CartesianLayerRangeProvider.fixed(minY = 0.0, maxY = 100.0) } val leftLayer = rememberConditionalLayer( hasData = leftLayerSeriesStyles.isNotEmpty(), lineProvider = LineCartesianLayer.LineProvider.series(leftLayerSeriesStyles), verticalAxisPosition = Axis.Position.Vertical.Start, - rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0, maxY = 100.0), + rangeProvider = percentRangeProvider, ) val rightLayer = diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt index 0f809ef81..5029729ca 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt @@ -158,11 +158,11 @@ fun EnvironmentMetricsChart( graphData.shouldPlot[(it.metricKey as? Environment)?.ordinal ?: 0] } - // Legend toggle state: tracks indices into allLegendData that are hidden - var hiddenIndices by remember { mutableStateOf(emptySet()) } - val hiddenMetrics = - remember(hiddenIndices, allLegendData) { - hiddenIndices.mapNotNull { allLegendData.getOrNull(it)?.metricKey as? Environment }.toSet() + // Track hidden metrics by key (not index) so toggling survives changes in allLegendData ordering. + var hiddenMetrics by remember { mutableStateOf(emptySet()) } + val hiddenIndices = + remember(hiddenMetrics, allLegendData) { + allLegendData.indices.filter { (allLegendData[it].metricKey as? Environment) in hiddenMetrics }.toSet() } val colorToLabel = allLegendData.associate { it.color to (it.labelOverride ?: stringResource(it.nameRes)) } @@ -233,6 +233,7 @@ fun EnvironmentMetricsChart( }, ) + val pressureRangeProvider = remember { CartesianLayerRangeProvider.fixed(minY = 700.0, maxY = 1200.0) } val layers = mutableListOf() if (showPressure && pressureData.isNotEmpty()) { layers.add( @@ -244,7 +245,7 @@ fun EnvironmentMetricsChart( verticalAxisPosition = Axis.Position.Vertical.Start, // Fixed range per Oscar's UX guidance: barometric pressure should NOT autoscale, // otherwise trends (storms) are invisible. 700-1200 hPa covers sea-level to altitude. - rangeProvider = CartesianLayerRangeProvider.fixed(minY = 700.0, maxY = 1200.0), + rangeProvider = pressureRangeProvider, ), ) } @@ -254,7 +255,7 @@ fun EnvironmentMetricsChart( when (metric) { Environment.RADIATION, Environment.WIND_SPEED, - -> CartesianLayerRangeProvider.fixed(minY = 0.0) + -> CartesianLayerRangeProvider.auto() else -> null } val lineStyle = @@ -310,7 +311,8 @@ fun EnvironmentMetricsChart( modifier = Modifier.padding(top = 0.dp), hiddenSet = hiddenIndices, onToggle = { index -> - hiddenIndices = if (index in hiddenIndices) hiddenIndices - index else hiddenIndices + index + val metric = allLegendData.getOrNull(index)?.metricKey as? Environment ?: return@Legend + hiddenMetrics = if (metric in hiddenMetrics) hiddenMetrics - metric else hiddenMetrics + metric }, ) } diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt index 598cd5ca9..b3b0b36e0 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt @@ -141,11 +141,20 @@ private fun PaxMetricsChart( rememberLineCartesianLayer( lineProvider = LineCartesianLayer.LineProvider.series( - ChartStyling.createGradientLine(lineColor = bleColor), - ChartStyling.createGradientLine(lineColor = wifiColor), - ChartStyling.createBoldLine(lineColor = paxColor), + ChartStyling.createGradientLine( + lineColor = bleColor, + interpolator = LineCartesianLayer.Interpolator.Sharp, + ), + ChartStyling.createGradientLine( + lineColor = wifiColor, + interpolator = LineCartesianLayer.Interpolator.Sharp, + ), + ChartStyling.createBoldLine( + lineColor = paxColor, + interpolator = LineCartesianLayer.Interpolator.Sharp, + ), ), - rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0), + rangeProvider = CartesianLayerRangeProvider.auto(), ), ), startAxis = VerticalAxis.rememberStart(label = axisLabel), diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt index c1e5e69fe..c27f111d1 100644 --- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt +++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt @@ -189,17 +189,26 @@ internal fun TracerouteMetricsChart( val forwardLayer = rememberConditionalLayer( hasData = forwardData.isNotEmpty(), - lineProvider = LineCartesianLayer.LineProvider.series(ChartStyling.createStyledLine(forwardColor)), + lineProvider = + LineCartesianLayer.LineProvider.series( + ChartStyling.createStyledLine( + forwardColor, + interpolator = LineCartesianLayer.Interpolator.Sharp, + ), + ), verticalAxisPosition = Axis.Position.Vertical.Start, - rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0), + rangeProvider = CartesianLayerRangeProvider.auto(), ) val returnLayer = rememberConditionalLayer( hasData = returnData.isNotEmpty(), - lineProvider = LineCartesianLayer.LineProvider.series(ChartStyling.createDashedLine(returnColor)), + lineProvider = + LineCartesianLayer.LineProvider.series( + ChartStyling.createDashedLine(returnColor, interpolator = LineCartesianLayer.Interpolator.Sharp), + ), verticalAxisPosition = Axis.Position.Vertical.Start, - rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0), + rangeProvider = CartesianLayerRangeProvider.auto(), ) val rttLayer =