From 9bb4ccc23ca3aa34dab9108303fa8807f2975bb7 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:20:47 -0500 Subject: [PATCH] refactor(qr): apply channel imports atomically via edit-settings transaction (#6170) Co-authored-by: Claude Opus 4.8 --- .../core/repository/AdminController.kt | 3 + .../core/service/AdminControllerImpl.kt | 8 +- .../core/service/RadioControllerImplTest.kt | 22 +++ .../core/testing/FakeRadioController.kt | 18 +- .../core/ui/qr/ScannedQrCodeViewModel.kt | 13 +- .../core/ui/util/ProtoExtensions.kt | 121 +++--------- .../core/ui/util/ProtoExtensionsTest.kt | 181 +++++------------- .../settings/channel/ChannelViewModel.kt | 13 +- 8 files changed, 131 insertions(+), 248 deletions(-) diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminController.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminController.kt index 08f37c7e0..35051e31e 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminController.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminController.kt @@ -148,6 +148,9 @@ interface AdminController { * coroutine, which is required for the firmware to associate them with one transaction. */ suspend fun editSettings(destNum: Int, block: suspend AdminEditScope.() -> Unit) + + /** Runs [block] as an [editSettings] transaction against the local node. */ + suspend fun editLocalSettings(block: suspend AdminEditScope.() -> Unit) } /** diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/AdminControllerImpl.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/AdminControllerImpl.kt index 3422ae3c8..454a3771a 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/AdminControllerImpl.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/AdminControllerImpl.kt @@ -224,6 +224,8 @@ internal class AdminControllerImpl( commandSender.sendAdmin(destNum) { AdminMessage(commit_edit_settings = true) } } + override suspend fun editLocalSettings(block: suspend AdminEditScope.() -> Unit) = editSettings(myNodeNum, block) + /** Binds the [AdminEditScope] operations to a fixed destination, delegating to this controller's set* methods. */ private inner class EditSettingsSession(private val destNum: Int) : AdminEditScope { override suspend fun setOwner(user: User) = setOwner(destNum, user, commandSender.generatePacketId()) @@ -233,8 +235,12 @@ internal class AdminControllerImpl( override suspend fun setModuleConfig(config: ModuleConfig) = setModuleConfig(destNum, config, commandSender.generatePacketId()) + // Unlike the one-shot setRemoteChannel, a transactional channel write does NOT mirror to the local cache per + // slot: importChannelSet owns the cache and writes it once after commit (replaceAllSettings), so an import + // interrupted before commit leaves the local channel cache untouched. (Firmware still writes each set_channel + // into its in-memory channel table on arrival; only disk persist/reload/reboot is deferred to commit.) override suspend fun setChannel(channel: Channel) = - setRemoteChannel(destNum, channel, commandSender.generatePacketId()) + commandSender.sendAdmin(destNum) { AdminMessage(set_channel = channel) } override suspend fun setFixedPosition(position: Position) = this@AdminControllerImpl.setFixedPosition(destNum, position) diff --git a/core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt b/core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt index 62e3f17f7..c29387a45 100644 --- a/core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt +++ b/core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt @@ -51,6 +51,8 @@ import org.meshtastic.core.repository.RadioInterfaceService import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.core.repository.UiPrefs import org.meshtastic.proto.AdminMessage +import org.meshtastic.proto.Channel +import org.meshtastic.proto.ChannelSettings import org.meshtastic.proto.ClientNotification import org.meshtastic.proto.Config import org.meshtastic.proto.HamParameters @@ -367,6 +369,26 @@ class RadioControllerImplTest { verifySuspend { commandSender.sendAdmin(any(), any(), any(), any()) } } + @Test + fun editLocalSettingsChannelWritesDoNotMirrorToLocalCache() = runTest { + val controller = createController(myNodeNum = 1234) + + controller.editLocalSettings { + setChannel(Channel(index = 0, role = Channel.Role.PRIMARY, settings = ChannelSettings(name = "A"))) + setChannel(Channel(index = 1, role = Channel.Role.SECONDARY, settings = ChannelSettings(name = "B"))) + } + testScope.advanceUntilIdle() + + // Exactly 4 admin packets: begin + 2 channel writes + commit. The tight count also catches a duplicated + // begin/commit or an accidental double-write per channel. + verifySuspend(exactly(4)) { commandSender.sendAdmin(any(), any(), any(), any()) } + // A transactional channel write must NOT eagerly mirror to the local cache the way one-shot + // setRemoteChannel does for the local node. importChannelSet owns the cache and writes it once after commit + // (replaceAllSettings), so an interrupted import can't leave partial channels cached. A regression to + // per-slot mirroring inside the session would make this call count non-zero. + verifySuspend(exactly(0)) { radioConfigRepository.updateChannelSettings(any()) } + } + @Test fun importContactSendsAdminAndUpdatesNodeManager() = runTest { val controller = createController() diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt index 2600a269e..6db848bee 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt @@ -57,6 +57,12 @@ class FakeRadioController : val localChannels = mutableListOf() var throwOnSend: Boolean = false + + /** + * When set, a channel write throws once [localChannels] has reached this many entries — simulates a mid-write + * failure. + */ + var failChannelWriteAfter: Int? = null var lastSetDeviceAddress: String? = null var lastSetOwnerUser: User? = null var editSettingsCalled = false @@ -73,6 +79,7 @@ class FakeRadioController : localConfigs.clear() localChannels.clear() throwOnSend = false + failChannelWriteAfter = null lastSetDeviceAddress = null lastSetOwnerUser = null editSettingsCalled = false @@ -124,11 +131,16 @@ class FakeRadioController : override suspend fun setHamMode(destNum: Int, hamParameters: HamParameters, packetId: Int) {} - override suspend fun setConfig(destNum: Int, config: Config, packetId: Int) {} + override suspend fun setConfig(destNum: Int, config: Config, packetId: Int) { + localConfigs.add(config) + } override suspend fun setModuleConfig(destNum: Int, config: ModuleConfig, packetId: Int) {} - override suspend fun setRemoteChannel(destNum: Int, channel: Channel, packetId: Int) {} + override suspend fun setRemoteChannel(destNum: Int, channel: Channel, packetId: Int) { + failChannelWriteAfter?.let { if (localChannels.size >= it) error("Fake channel write failure") } + localChannels.add(channel) + } override suspend fun setFixedPosition(destNum: Int, position: Position) {} @@ -196,6 +208,8 @@ class FakeRadioController : scope.block() } + override suspend fun editLocalSettings(block: suspend AdminEditScope.() -> Unit) = editSettings(0, block) + override fun generatePacketId(): Int = 1 override fun startProvideLocation() { 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 1b8cbc7d7..7c4cb89f3 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 @@ -22,8 +22,7 @@ import org.koin.core.annotation.KoinViewModel import org.meshtastic.core.repository.NodeRepository import org.meshtastic.core.repository.RadioConfigRepository import org.meshtastic.core.repository.RadioController -import org.meshtastic.core.ui.util.applyImportedLoraConfigAfterChannelReplacement -import org.meshtastic.core.ui.util.applyReplacementChannelSet +import org.meshtastic.core.ui.util.importChannelSet import org.meshtastic.core.ui.viewmodel.safeLaunch import org.meshtastic.core.ui.viewmodel.stateInWhileSubscribed import org.meshtastic.proto.ChannelSet @@ -47,12 +46,6 @@ class ScannedQrCodeViewModel( ) /** Set the radio config (also updates our saved copy in preferences). */ - fun setChannels(channelSet: ChannelSet) = safeLaunch(tag = "setChannels") { - val currentLoraConfig = applyReplacementChannelSet(channelSet, radioController, radioConfigRepository) - applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig = channelSet.lora_config, - currentLoraConfig = currentLoraConfig, - radioController = radioController, - ) - } + fun setChannels(channelSet: ChannelSet) = + safeLaunch(tag = "setChannels") { importChannelSet(channelSet, radioController, radioConfigRepository) } } 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 f4d239dac..909f27da3 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 @@ -19,7 +19,6 @@ package org.meshtastic.core.ui.util import androidx.compose.runtime.Composable import co.touchlab.kermit.Logger import kotlinx.coroutines.NonCancellable -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext import okio.ByteString @@ -36,9 +35,7 @@ import org.meshtastic.proto.ChannelSettings import org.meshtastic.proto.Config import org.meshtastic.proto.MeshPacket import org.meshtastic.proto.Position -import kotlin.time.Duration import kotlin.time.Duration.Companion.days -import kotlin.time.Duration.Companion.seconds import org.meshtastic.core.model.Channel as ModelChannel private const val SECONDS_TO_MILLIS = 1000L @@ -46,10 +43,6 @@ private const val SECONDS_TO_MILLIS = 1000L // Firmware channel files expose eight slots: one primary plus up to seven secondary channels. private const val CHANNEL_REPLACEMENT_SLOT_COUNT = 8 -// Full channel replacement writes need conservative settle windows so hardware can persist each slot. -private val CHANNEL_REPLACEMENT_WRITE_DELAY = 1.seconds -private val LORA_CONFIG_SETTLE_DELAY = 2.seconds - @Composable fun Position.formatPositionTime(): String { val currentTime = nowMillis @@ -198,37 +191,38 @@ fun normalizeReplacementSettings( private fun ChannelSettings.isPlaceholder(): Boolean = name.isNullOrBlank() && psk.size == 0 /** - * Applies an imported [ChannelSet] as an authoritative replacement to the radio and local cache. + * Imports a [ChannelSet] as an authoritative REPLACE: writes every channel and — when present and actually different — + * the imported LoRa config, all inside one [RadioController.editLocalSettings] transaction, then replaces the local + * channel cache. * * Reads the current LoRa config and channel set from [radioConfigRepository]'s flows (avoiding the StateFlow - * placeholder window), builds the authoritative replacement list via [getChannelReplacementList], enqueues each channel - * write to the radio via [radioController], pauses between writes so the radio can persist and reconfigure each slot, - * then atomically replaces the local cached settings. + * placeholder window) and builds the authoritative replacement list via [getChannelReplacementList]. The edit-settings + * transaction defers disk persistence, radio reload/reconfiguration, and reboot until the closing commit, so channels + + * LoRa land in a single reboot with no per-slot reconfigure to pace against. (Firmware still writes each `set_channel` + * into its in-memory channel table as it arrives — the transaction is not a full staging of channel state — but the + * expensive persist/reload path runs once at commit.) Writing LoRa inside the same session mirrors + * `InstallProfileUseCase` and is why the old pre/post settle delays are gone: the begin/commit boundary is the settle. * - * setLocalChannel returns once the packet is enqueued, not after firmware ACK. The pacing avoids enqueueing a complete - * channel replacement plus LoRa reconfiguration faster than real hardware can materialize the later channel slots. If - * the sequence is interrupted after one or more successful writes, the local cache is reconciled to the successfully - * enqueued channel settings before the original cancellation or failure continues. + * The local channel cache is commit-shaped: transactional channel writes deliberately do not mirror per slot (see + * `AdminControllerImpl.EditSettingsSession.setChannel`), and this function replaces the cached channel list once, after + * the session succeeds — so an import interrupted before that point leaves the local channel cache untouched. (The + * imported LoRa config is the one exception: it still writes through the cache-mirroring `setConfig`, so its local + * cache update is not itself deferred to commit — a single trailing write that self-heals on the device's next config + * re-send. Making `setConfig` transaction-aware is future work.) * * Imported settings are normalized via [normalizeReplacementSettings] before any write or bounds check, so blank * placeholder secondaries and semantic duplicates never reach the radio or the local cache. * - * 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. - * @param writeDelay Delay after each channel write. Exposed for fast unit tests. - * @param delayFn Delay implementation. Exposed for fast unit tests. - * @return The device's current LoRa config snapshot used by callers to compare against an imported LoRa config. + * @param channelSet The imported [ChannelSet] to apply as a replacement. Its `lora_config`, if present and different + * from the device's current LoRa config, is written inside the same transaction. + * @param radioController The [RadioController] used to run the edit transaction. + * @param radioConfigRepository The [RadioConfigRepository] providing the current channel/LoRa flows and cache. */ -suspend fun applyReplacementChannelSet( +suspend fun importChannelSet( channelSet: ChannelSet, radioController: RadioController, radioConfigRepository: RadioConfigRepository, - writeDelay: Duration = CHANNEL_REPLACEMENT_WRITE_DELAY, - delayFn: suspend (Duration) -> Unit = { delay(it) }, -): Config.LoRaConfig? { +) { // Resolve the LoRa preset used for semantic identity: prefer the imported config, fall back to the device's current // local config so duplicate detection stays correct when the import omits lora_config (e.g. a non-default preset). val currentLoraConfig = radioConfigRepository.localConfigFlow.first().lora @@ -245,82 +239,25 @@ suspend fun applyReplacementChannelSet( minimumSlotCount = CHANNEL_REPLACEMENT_SLOT_COUNT, maximumSlotCount = CHANNEL_REPLACEMENT_SLOT_COUNT, ) + // Only write LoRa when the import carries one that actually differs from the device — avoids a redundant + // reconfigure. + val importedLoraConfig = channelSet.lora_config?.takeIf { it != currentLoraConfig } Logger.i { "Applying imported channel replacement writes=${replacements.size} " + - "importedSettings=${channelSet.settings.size} normalizedSettings=${normalizedSettings.size}" + "importedSettings=${channelSet.settings.size} normalizedSettings=${normalizedSettings.size} " + + "writesLora=${importedLoraConfig != null}" } - val appliedSettings = currentSettings.take(CHANNEL_REPLACEMENT_SLOT_COUNT).toMutableList() - var appliedWriteCount = 0 - var replacementComplete = false - try { + radioController.editLocalSettings { for (channel in replacements) { Logger.i { "Writing imported channel index=${channel.index} role=${channel.role} " + "hasName=${channel.settings?.name?.isNotBlank() == true}" } - radioController.setLocalChannel(channel) - while (appliedSettings.size <= channel.index) { - appliedSettings.add(ChannelSettings()) - } - appliedSettings[channel.index] = - if (channel.role == Channel.Role.DISABLED) { - ChannelSettings() - } else { - channel.settings ?: ChannelSettings() - } - appliedWriteCount++ - delayFn(writeDelay) - } - replacementComplete = true - } finally { - if (!replacementComplete) { - radioConfigRepository.reconcileInterruptedReplacement( - appliedWriteCount = appliedWriteCount, - totalWriteCount = replacements.size, - appliedSettings = appliedSettings, - normalizedSettings = normalizedSettings, - ) + setChannel(channel) } + importedLoraConfig?.let { setConfig(Config(lora = it)) } } withContext(NonCancellable) { radioConfigRepository.replaceAllSettings(normalizedSettings) } - return currentLoraConfig -} - -private suspend fun RadioConfigRepository.reconcileInterruptedReplacement( - appliedWriteCount: Int, - totalWriteCount: Int, - appliedSettings: List, - normalizedSettings: List, -) { - if (appliedWriteCount == 0) return - val replacementSettings = if (appliedWriteCount == totalWriteCount) normalizedSettings else appliedSettings - Logger.w { - "Reconciling interrupted channel replacement appliedWrites=$appliedWriteCount totalWrites=$totalWriteCount" - } - withContext(NonCancellable) { replaceAllSettings(replacementSettings) } -} - -/** - * Applies an imported LoRa config after channel replacement writes have had time to settle. - * - * LoRa reconfiguration is expensive on firmware and can race with channel persistence if sent immediately after a full - * channel replacement. The pre/post settle delays give the radio time to materialize the imported channels before and - * after the LoRa write. - */ -suspend fun applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig: Config.LoRaConfig?, - currentLoraConfig: Config.LoRaConfig?, - radioController: RadioController, - settleDelay: Duration = LORA_CONFIG_SETTLE_DELAY, - delayFn: suspend (Duration) -> Unit = { delay(it) }, -) { - if (importedLoraConfig == null || currentLoraConfig == importedLoraConfig) return - - Logger.i { "Settling before imported LoRa config write" } - delayFn(settleDelay) - radioController.setLocalConfig(Config(lora = importedLoraConfig)) - Logger.i { "Settling after imported LoRa config write" } - delayFn(settleDelay) } /** 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 index e5f069c0d..6e99e3b11 100644 --- 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 @@ -16,9 +16,6 @@ */ package org.meshtastic.core.ui.util -import kotlinx.coroutines.cancel -import kotlinx.coroutines.currentCoroutineContext -import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import okio.ByteString.Companion.toByteString import org.meshtastic.core.testing.FakeRadioConfigRepository @@ -33,8 +30,6 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue -import kotlin.time.Duration -import kotlin.time.Duration.Companion.seconds import org.meshtastic.core.model.Channel as ModelChannel /** @@ -169,7 +164,7 @@ class ProtoExtensionsTest { } @Test - fun replacement_apply_paces_every_write_before_replacing_cached_settings() = runTest { + fun import_writes_all_eight_slots_with_replacement_roles() = runTest { val radioController = FakeRadioController() val radioConfigRepository = FakeRadioConfigRepository() val oldSettings = @@ -179,15 +174,12 @@ class ProtoExtensionsTest { ChannelSettings(name = "Old Tertiary"), ) val importedSettings = listOf(ChannelSettings(name = "Imported"), ChannelSettings(name = "Private")) - val cacheSnapshotsAtDelay = mutableListOf>() radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = importedSettings), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = { cacheSnapshotsAtDelay.add(radioConfigRepository.currentChannelSet.settings) }, ) assertEquals((0..7).toList(), radioController.localChannels.map { it.index }) @@ -205,47 +197,11 @@ class ProtoExtensionsTest { radioController.localChannels.map { it.role }, ) assertEquals(importedSettings, radioConfigRepository.currentChannelSet.settings) - assertEquals(List(size = 8) { oldSettings }, cacheSnapshotsAtDelay) } @Test - fun replacement_apply_reconciles_successful_writes_when_interrupted_during_pacing() = runTest { - val radioController = FakeRadioController() - val radioConfigRepository = FakeRadioConfigRepository() - val oldSettings = - listOf( - ChannelSettings(name = "Old Primary"), - ChannelSettings(name = "Old Secondary"), - ChannelSettings(name = "Old Tertiary"), - ) - val importedPrimary = ChannelSettings(name = "Imported") - val importedSecondary = ChannelSettings(name = "Private") - var delayCount = 0 - radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) - - assertFailsWith { - applyReplacementChannelSet( - channelSet = ChannelSet(settings = listOf(importedPrimary, importedSecondary)), - radioController = radioController, - radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = { - delayCount++ - if (delayCount == 2) error("stop") - }, - ) - } - - assertEquals(listOf(0, 1), radioController.localChannels.map { it.index }) - assertEquals( - listOf(importedPrimary, importedSecondary, oldSettings[2]), - radioConfigRepository.currentChannelSet.settings, - ) - } - - @Test - fun replacement_apply_compacts_cache_when_interrupted_after_all_channel_writes() = runTest { - val radioController = FakeRadioController() + fun import_leaves_cache_untouched_when_a_channel_write_fails_mid_session() = runTest { + val radioController = FakeRadioController().apply { failChannelWriteAfter = 2 } val radioConfigRepository = FakeRadioConfigRepository() val oldSettings = listOf( @@ -254,58 +210,23 @@ class ProtoExtensionsTest { ChannelSettings(name = "Old Tertiary"), ) val importedSettings = listOf(ChannelSettings(name = "Imported"), ChannelSettings(name = "Private")) - var delayCount = 0 radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) + // A write failing inside the editLocalSettings session propagates out before the post-session cache + // replace, so the local cache stays exactly as it was — nothing partially applied. assertFailsWith { - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = importedSettings), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = { - delayCount++ - if (delayCount == 8) error("stop") - }, ) } - assertEquals((0..7).toList(), radioController.localChannels.map { it.index }) - assertEquals(importedSettings, radioConfigRepository.currentChannelSet.settings) + assertEquals(oldSettings, radioConfigRepository.currentChannelSet.settings) } @Test - fun replacement_apply_final_cache_update_survives_cancellation() = runTest { - val radioController = FakeRadioController() - val radioConfigRepository = FakeRadioConfigRepository() - val oldSettings = listOf(ChannelSettings(name = "Old Primary")) - val importedSettings = listOf(ChannelSettings(name = "Imported"), ChannelSettings(name = "Private")) - var delayCount = 0 - radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) - - val applyJob = launch { - applyReplacementChannelSet( - channelSet = ChannelSet(settings = importedSettings), - radioController = radioController, - radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = { - delayCount++ - if (delayCount == 8) { - currentCoroutineContext().cancel() - } - }, - ) - } - applyJob.join() - - assertTrue(applyJob.isCancelled) - assertEquals((0..7).toList(), radioController.localChannels.map { it.index }) - assertEquals(importedSettings, radioConfigRepository.currentChannelSet.settings) - } - - @Test - fun replacement_apply_rejects_imported_settings_beyond_slot_count_before_writing() = runTest { + fun import_rejects_imported_settings_beyond_slot_count_before_writing() = runTest { val radioController = FakeRadioController() val radioConfigRepository = FakeRadioConfigRepository() val oldSettings = listOf(ChannelSettings(name = "Old")) @@ -313,12 +234,10 @@ class ProtoExtensionsTest { radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) assertFailsWith { - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = oversizedSettings), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = {}, ) } @@ -334,12 +253,10 @@ class ProtoExtensionsTest { val importedSettings = listOf(ChannelSettings(name = "Imported")) radioConfigRepository.setChannelSet(ChannelSet(settings = oldSettings)) - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = importedSettings), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = {}, ) assertEquals((0..7).toList(), radioController.localChannels.map { it.index }) @@ -361,12 +278,10 @@ class ProtoExtensionsTest { val ch6 = ChannelSettings(name = "Ch6", psk = byteArrayOf(7).toByteString()) val raw = listOf(ch0, ch1, ChannelSettings(), ch2, ch3, ChannelSettings(), ch4, ch5, ch6) - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = raw), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = {}, ) // Cache holds the normalized 7-entry set, not the raw 9-entry import. @@ -384,12 +299,10 @@ class ProtoExtensionsTest { val unique = (1..9).map { ChannelSettings(name = "Ch$it", psk = byteArrayOf(it.toByte(), 0).toByteString()) } assertFailsWith { - applyReplacementChannelSet( + importChannelSet( channelSet = ChannelSet(settings = unique + ChannelSettings()), radioController = radioController, radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = {}, ) } @@ -416,60 +329,62 @@ class ProtoExtensionsTest { val primary = ChannelSettings(name = "MediumFast", psk = psk) val unnamedSecondary = ChannelSettings(psk = psk) - val currentLoraConfig = - applyReplacementChannelSet( - channelSet = ChannelSet(settings = listOf(primary, unnamedSecondary)), // no lora_config - radioController = radioController, - radioConfigRepository = radioConfigRepository, - writeDelay = 1.seconds, - delayFn = {}, - ) + importChannelSet( + channelSet = ChannelSet(settings = listOf(primary, unnamedSecondary)), // no lora_config + radioController = radioController, + radioConfigRepository = radioConfigRepository, + ) // Secondary dropped as a semantic duplicate of the primary under the device's MEDIUM_FAST preset. - assertEquals(radioConfigRepository.currentLocalConfig.lora, currentLoraConfig) assertEquals(listOf(primary), radioConfigRepository.currentChannelSet.settings) } @Test - fun imported_lora_config_settles_before_and_after_write() = runTest { + fun imported_lora_config_is_written_inside_the_same_transaction_when_it_differs() = runTest { val radioController = FakeRadioController() - val current = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.EU_868) + val radioConfigRepository = FakeRadioConfigRepository() val imported = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US) - val delays = mutableListOf() + radioConfigRepository.setLocalConfigDirect( + LocalConfig(lora = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.EU_868)), + ) + radioConfigRepository.setChannelSet(ChannelSet(settings = listOf(ChannelSettings(name = "Old")))) - applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig = imported, - currentLoraConfig = current, + importChannelSet( + channelSet = ChannelSet(settings = listOf(ChannelSettings(name = "Imported")), lora_config = imported), radioController = radioController, - settleDelay = 2.seconds, - delayFn = { delays.add(it) }, + radioConfigRepository = radioConfigRepository, ) - assertEquals(listOf(2.seconds, 2.seconds), delays) + // LoRa write is the last op in the edit session, with no settle delays around it. assertEquals(listOf(Config(lora = imported)), radioController.localConfigs) } @Test fun imported_lora_config_is_not_written_when_absent_or_unchanged() = runTest { - val radioController = FakeRadioController() val current = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US) - val delays = mutableListOf() - applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig = null, - currentLoraConfig = current, - radioController = radioController, - delayFn = { delays.add(it) }, - ) - applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig = current, - currentLoraConfig = current, - radioController = radioController, - delayFn = { delays.add(it) }, + val absent = FakeRadioController() + val absentRepo = FakeRadioConfigRepository() + absentRepo.setLocalConfigDirect(LocalConfig(lora = current)) + absentRepo.setChannelSet(ChannelSet(settings = listOf(ChannelSettings(name = "Old")))) + importChannelSet( + channelSet = ChannelSet(settings = listOf(ChannelSettings(name = "Imported"))), // no lora_config + radioController = absent, + radioConfigRepository = absentRepo, ) - assertTrue(delays.isEmpty()) - assertTrue(radioController.localConfigs.isEmpty()) + val unchanged = FakeRadioController() + val unchangedRepo = FakeRadioConfigRepository() + unchangedRepo.setLocalConfigDirect(LocalConfig(lora = current)) + unchangedRepo.setChannelSet(ChannelSet(settings = listOf(ChannelSettings(name = "Old")))) + importChannelSet( + channelSet = ChannelSet(settings = listOf(ChannelSettings(name = "Imported")), lora_config = current), + radioController = unchanged, + radioConfigRepository = unchangedRepo, + ) + + assertTrue(absent.localConfigs.isEmpty()) + assertTrue(unchanged.localConfigs.isEmpty()) } // --- getChannelPreviewForAdd tests --- 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 7b9536450..093a67050 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,8 +27,7 @@ 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.applyImportedLoraConfigAfterChannelReplacement -import org.meshtastic.core.ui.util.applyReplacementChannelSet +import org.meshtastic.core.ui.util.importChannelSet import org.meshtastic.core.ui.viewmodel.safeLaunch import org.meshtastic.core.ui.viewmodel.stateInWhileSubscribed import org.meshtastic.proto.ChannelSet @@ -85,14 +84,8 @@ class ChannelViewModel( } /** Set the radio config (also updates our saved copy in preferences). */ - fun setChannels(channelSet: ChannelSet) = safeLaunch(tag = "setChannels") { - val currentLoraConfig = applyReplacementChannelSet(channelSet, radioController, radioConfigRepository) - applyImportedLoraConfigAfterChannelReplacement( - importedLoraConfig = channelSet.lora_config, - currentLoraConfig = currentLoraConfig, - radioController = radioController, - ) - } + fun setChannels(channelSet: ChannelSet) = + safeLaunch(tag = "setChannels") { importChannelSet(channelSet, radioController, radioConfigRepository) } // Set the radio config (also updates our saved copy in preferences) fun setConfig(config: Config) {