From cbd72bee4959c26d40133609afa30b00e34243ab Mon Sep 17 00:00:00 2001 From: Jeremiah K <17190268+jeremiah-k@users.noreply.github.com> Date: Sun, 28 Jun 2026 19:53:29 -0500 Subject: [PATCH] fix(qr): Serialize channel import writes (#5999) --- .../core/ui/qr/ScannedQrCodeViewModel.kt | 10 +- .../core/ui/util/ProtoExtensions.kt | 75 +++++++++ .../core/ui/util/ProtoExtensionsTest.kt | 144 ++++++++++++++++++ .../settings/channel/ChannelViewModel.kt | 10 +- 4 files changed, 223 insertions(+), 16 deletions(-) create mode 100644 core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/ProtoExtensionsTest.kt diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/qr/ScannedQrCodeViewModel.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/qr/ScannedQrCodeViewModel.kt index b5be99825..60f46ae14 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/qr/ScannedQrCodeViewModel.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/qr/ScannedQrCodeViewModel.kt @@ -20,10 +20,9 @@ import androidx.lifecycle.ViewModel import org.koin.core.annotation.KoinViewModel import org.meshtastic.core.repository.RadioConfigRepository import org.meshtastic.core.repository.RadioController -import org.meshtastic.core.ui.util.getChannelList +import org.meshtastic.core.ui.util.applyReplacementChannelSet import org.meshtastic.core.ui.viewmodel.safeLaunch import org.meshtastic.core.ui.viewmodel.stateInWhileSubscribed -import org.meshtastic.proto.Channel import org.meshtastic.proto.ChannelSet import org.meshtastic.proto.Config import org.meshtastic.proto.LocalConfig @@ -40,8 +39,7 @@ class ScannedQrCodeViewModel( /** Set the radio config (also updates our saved copy in preferences). */ fun setChannels(channelSet: ChannelSet) = safeLaunch(tag = "setChannels") { - getChannelList(channelSet.settings, channels.value.settings).forEach(::setChannel) - radioConfigRepository.replaceAllSettings(channelSet.settings) + applyReplacementChannelSet(channelSet, radioController, radioConfigRepository) val loraConfig = channelSet.lora_config if (loraConfig != null && localConfig.value.lora != loraConfig) { @@ -49,10 +47,6 @@ class ScannedQrCodeViewModel( } } - private fun setChannel(channel: Channel) { - safeLaunch(tag = "setChannel") { radioController.setLocalChannel(channel) } - } - // Set the radio config (also updates our saved copy in preferences) private fun setConfig(config: Config) { safeLaunch(tag = "setConfig") { radioController.setLocalConfig(config) } diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/ProtoExtensions.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/ProtoExtensions.kt index c9e179b9f..5925e39e3 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/ProtoExtensions.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/ProtoExtensions.kt @@ -17,12 +17,16 @@ package org.meshtastic.core.ui.util import androidx.compose.runtime.Composable +import kotlinx.coroutines.flow.first import org.jetbrains.compose.resources.stringResource import org.meshtastic.core.common.util.DateFormatter import org.meshtastic.core.common.util.nowMillis +import org.meshtastic.core.repository.RadioConfigRepository +import org.meshtastic.core.repository.RadioController import org.meshtastic.core.resources.Res import org.meshtastic.core.resources.unknown_age import org.meshtastic.proto.Channel +import org.meshtastic.proto.ChannelSet import org.meshtastic.proto.ChannelSettings import org.meshtastic.proto.MeshPacket import org.meshtastic.proto.Position @@ -79,3 +83,74 @@ fun getChannelList(new: List, old: List): List } } } + +/** + * Builds an authoritative [Channel] list for a full REPLACE import. Every position in [new] is emitted (PRIMARY at + * index 0, SECONDARY for 1..new.lastIndex) and any trailing positions beyond [new]'s range are emitted as DISABLED so + * the radio stops using them. + * + * Unlike [getChannelList], this does NOT skip positions where `currentSettings[i] == new[i]`: the imported set is + * authoritative, the local cache must not gate the writes, and silent diff-skips during REPLACE were the source of + * stale channels. + * + * [currentSettings] is consulted only for its size (to determine trailing DISABLED writes); its values are never + * compared against [new]. Callers should read it from `radioConfigRepository.channelSetFlow.first().settings`, not from + * a `stateInWhileSubscribed` StateFlow's `.value` — the StateFlow placeholder window can return an empty list and + * suppress trailing DISABLED writes. + * + * Edge case: if [new] is empty, every emitted slot (including index 0) is DISABLED rather than wrongly promoting an + * empty [ChannelSettings] to PRIMARY. + * + * @param new The imported [ChannelSettings] list. Every index becomes a write to the radio. + * @param currentSettings The current [ChannelSettings] list. Only its size is used; trailing indices past [new] become + * DISABLED writes so leftover slots are cleared. + * @return A [Channel] list covering every slot the radio needs written to materialize [new] and clear leftover slots. + */ +fun getChannelReplacementList(new: List, currentSettings: List): List = + buildList { + for (i in 0..maxOf(currentSettings.lastIndex, new.lastIndex)) { + add( + Channel( + role = + when (i) { + // Empty-new is a degenerate import: every slot (including 0) must be DISABLED. + 0 -> if (new.isEmpty()) Channel.Role.DISABLED else Channel.Role.PRIMARY + + in 1..new.lastIndex -> Channel.Role.SECONDARY + + else -> Channel.Role.DISABLED + }, + index = i, + settings = new.getOrNull(i) ?: ChannelSettings(), + ), + ) + } + } + +/** + * Applies an imported [ChannelSet] as an authoritative replacement to the radio and local cache. + * + * Reads the current channel set from [radioConfigRepository]'s flow (avoiding the StateFlow placeholder window), builds + * the authoritative replacement list via [getChannelReplacementList], enqueues each channel write to the radio + * sequentially via [radioController], then atomically replaces the local cached settings. + * + * setLocalChannel returns once the packet is enqueued, not after firmware ACK — firmware echoes via + * MeshConfigHandlerImpl can still arrive after [radioConfigRepository.replaceAllSettings] and are tracked separately. + * + * Does NOT handle LoRa config — callers are responsible for comparing and sending `lora_config` if present. + * + * @param channelSet The imported [ChannelSet] to apply as a replacement. + * @param radioController The [RadioController] used to enqueue channel writes. + * @param radioConfigRepository The [RadioConfigRepository] providing the current channel flow and cache. + */ +suspend fun applyReplacementChannelSet( + channelSet: ChannelSet, + radioController: RadioController, + radioConfigRepository: RadioConfigRepository, +) { + val currentSettings = radioConfigRepository.channelSetFlow.first().settings + for (channel in getChannelReplacementList(channelSet.settings, currentSettings)) { + radioController.setLocalChannel(channel) + } + radioConfigRepository.replaceAllSettings(channelSet.settings) +} diff --git a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/ProtoExtensionsTest.kt b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/ProtoExtensionsTest.kt new file mode 100644 index 000000000..4afc97923 --- /dev/null +++ b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/ProtoExtensionsTest.kt @@ -0,0 +1,144 @@ +/* + * 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 . + */ +package org.meshtastic.core.ui.util + +import okio.ByteString.Companion.toByteString +import org.meshtastic.proto.Channel +import org.meshtastic.proto.ChannelSettings +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Coverage for [getChannelReplacementList]. The REPLACE helper must emit an authoritative slot list for QR imports: + * every imported index becomes a write (PRIMARY at 0, SECONDARY thereafter), and any trailing slots present in the + * cached set are emitted as DISABLED so the radio stops using them. Critically, positions where the cache already + * matches the import are NOT skipped — the diff-skip was the source of stale channels. + */ +class ProtoExtensionsTest { + @Test + fun index_zero_emits_primary_with_new_settings_even_when_unchanged_from_old() { + val same = ChannelSettings(name = "Main", psk = byteArrayOf(1, 2, 3).toByteString()) + + val result = getChannelReplacementList(new = listOf(same), currentSettings = listOf(same)) + + assertEquals(1, result.size) + assertEquals(Channel.Role.PRIMARY, result.single().role) + assertEquals(0, result.single().index) + assertEquals(same, result.single().settings) + } + + @Test + fun secondary_indices_emit_secondary_with_new_settings_even_when_unchanged_from_old() { + val primary = ChannelSettings(name = "Main") + val secondary = ChannelSettings(name = "Chat") + + val result = + getChannelReplacementList(new = listOf(primary, secondary), currentSettings = listOf(primary, secondary)) + + assertEquals(2, result.size) + assertEquals(Channel.Role.PRIMARY, result[0].role) + assertEquals(primary, result[0].settings) + assertEquals(Channel.Role.SECONDARY, result[1].role) + assertEquals(1, result[1].index) + assertEquals(secondary, result[1].settings) + } + + @Test + fun old_trailing_indices_beyond_new_are_emitted_as_disabled_with_empty_settings() { + val primary = ChannelSettings(name = "Main") + + val result = + getChannelReplacementList( + new = listOf(primary), + currentSettings = listOf(primary, ChannelSettings(name = "Old")), + ) + + // index 0 PRIMARY (new), index 1 DISABLED (trailing old slot) + assertEquals(2, result.size) + assertEquals(Channel.Role.PRIMARY, result[0].role) + assertEquals(primary, result[0].settings) + assertEquals(Channel.Role.DISABLED, result[1].role) + assertEquals(1, result[1].index) + assertEquals(ChannelSettings(), result[1].settings) + } + + @Test + fun empty_new_and_empty_old_produces_empty_list() { + val result = getChannelReplacementList(new = emptyList(), currentSettings = emptyList()) + + assertTrue(result.isEmpty()) + } + + @Test + fun empty_new_with_non_empty_current_emits_disabled_for_every_current_index() { + val currentSettings = + listOf(ChannelSettings(name = "A"), ChannelSettings(name = "B"), ChannelSettings(name = "C")) + + val result = getChannelReplacementList(new = emptyList(), currentSettings = currentSettings) + + assertEquals(3, result.size) + result.forEachIndexed { i, channel -> + assertEquals(Channel.Role.DISABLED, channel.role, "index $i should be DISABLED") + assertEquals(i, channel.index) + assertEquals(ChannelSettings(), channel.settings, "index $i should carry empty settings") + } + } + + @Test + fun single_entry_new_with_multi_entry_current_emits_primary_then_disabled_trailing() { + val newPrimary = ChannelSettings(name = "Imported") + val currentSettings = + listOf( + ChannelSettings(name = "CurrentPrimary"), + ChannelSettings(name = "CurrentSecondary"), + ChannelSettings(name = "CurrentTertiary"), + ) + + val result = getChannelReplacementList(new = listOf(newPrimary), currentSettings = currentSettings) + + assertEquals(3, result.size) + assertEquals(Channel.Role.PRIMARY, result[0].role) + assertEquals(0, result[0].index) + assertEquals(newPrimary, result[0].settings) + assertEquals(Channel.Role.DISABLED, result[1].role) + assertEquals(Channel.Role.DISABLED, result[2].role) + assertEquals(ChannelSettings(), result[1].settings) + assertEquals(ChannelSettings(), result[2].settings) + } + + @Test + fun new_larger_than_old_emits_primary_plus_secondaries_for_every_new_index() { + val primary = ChannelSettings(name = "Main") + val secondaryA = ChannelSettings(name = "Chat") + val secondaryB = ChannelSettings(name = "Data") + + val result = + getChannelReplacementList(new = listOf(primary, secondaryA, secondaryB), currentSettings = listOf(primary)) + + assertEquals(3, result.size) + assertEquals(Channel.Role.PRIMARY, result[0].role) + assertEquals(0, result[0].index) + assertEquals(primary, result[0].settings) + assertEquals(Channel.Role.SECONDARY, result[1].role) + assertEquals(1, result[1].index) + assertEquals(secondaryA, result[1].settings) + assertEquals(Channel.Role.SECONDARY, result[2].role) + assertEquals(2, result[2].index) + assertEquals(secondaryB, result[2].settings) + } +} diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/channel/ChannelViewModel.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/channel/ChannelViewModel.kt index 1d8b54287..d3fa5026f 100644 --- a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/channel/ChannelViewModel.kt +++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/channel/ChannelViewModel.kt @@ -27,10 +27,9 @@ import org.meshtastic.core.repository.DataPair import org.meshtastic.core.repository.PlatformAnalytics import org.meshtastic.core.repository.RadioConfigRepository import org.meshtastic.core.repository.RadioController -import org.meshtastic.core.ui.util.getChannelList +import org.meshtastic.core.ui.util.applyReplacementChannelSet import org.meshtastic.core.ui.viewmodel.safeLaunch import org.meshtastic.core.ui.viewmodel.stateInWhileSubscribed -import org.meshtastic.proto.Channel import org.meshtastic.proto.ChannelSet import org.meshtastic.proto.Config import org.meshtastic.proto.LocalConfig @@ -86,8 +85,7 @@ class ChannelViewModel( /** Set the radio config (also updates our saved copy in preferences). */ fun setChannels(channelSet: ChannelSet) = safeLaunch(tag = "setChannels") { - getChannelList(channelSet.settings, channels.value.settings).forEach(::setChannel) - radioConfigRepository.replaceAllSettings(channelSet.settings) + applyReplacementChannelSet(channelSet, radioController, radioConfigRepository) val newLoraConfig = channelSet.lora_config if (localConfig.value.lora != newLoraConfig) { @@ -95,10 +93,6 @@ class ChannelViewModel( } } - fun setChannel(channel: Channel) { - safeLaunch(tag = "setChannel") { radioController.setLocalChannel(channel) } - } - // Set the radio config (also updates our saved copy in preferences) fun setConfig(config: Config) { safeLaunch(tag = "setConfig") { radioController.setLocalConfig(config) }