Skip to content

Commit fe56793

Browse files
committed
fix(settings): apply manual channel writes sequentially
Apply manual channel saves through a single ordered channel update sequence instead of launching one coroutine per changed slot. The update plan is sorted by channel index so delete, re-add, and reorder-style edits enqueue radio writes deterministically while preserving each row's ChannelSettings payload, including its PSK. Register every returned request ID from the ordered write loop so existing packet response tracking remains in place. Local channel migration and cache replacement now run after the ordered send loop for local nodes, and the visible channel list is updated after the same sequence completes. Add focused ViewModel coverage that records write order and verifies changed channel writes do not overlap when setRemoteChannel suspends.
1 parent d8afe84 commit fe56793

2 files changed

Lines changed: 47 additions & 6 deletions

File tree

feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -393,22 +393,26 @@ open class RadioConfigViewModel(
393393

394394
fun updateChannels(new: List<ChannelSettings>, old: List<ChannelSettings>) {
395395
val destNum = destNum ?: destNode.value?.num ?: return
396-
getChannelList(new, old).forEach { channel ->
397-
safeLaunch(tag = "setRemoteChannel") {
396+
397+
val updatePlan = getManualChannelUpdatePlan(new, old)
398+
if (updatePlan.isEmpty()) return
399+
safeLaunch(tag = "setRemoteChannels") {
400+
for (channel in updatePlan) {
398401
val packetId = radioConfigUseCase.setRemoteChannel(destNum, channel)
399402
registerRequestId(packetId)
400403
}
401-
}
402404

403-
if (destNum == myNodeNum) {
404-
safeLaunch(tag = "migrateChannels") {
405+
if (destNum == myNodeNum) {
405406
packetRepository.migrateChannelsByPSK(old, new)
406407
radioConfigRepository.replaceAllSettings(new)
407408
}
409+
_radioConfigState.update { it.copy(channelList = new) }
408410
}
409-
_radioConfigState.update { it.copy(channelList = new) }
410411
}
411412

413+
private fun getManualChannelUpdatePlan(new: List<ChannelSettings>, old: List<ChannelSettings>): List<Channel> =
414+
getChannelList(new, old).sortedBy { it.index }
415+
412416
fun setConfig(config: Config) {
413417
val destNum = destNum ?: destNode.value?.num ?: return
414418
safeLaunch(tag = "setConfig") {

feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import kotlinx.coroutines.flow.MutableSharedFlow
3434
import kotlinx.coroutines.flow.MutableStateFlow
3535
import kotlinx.coroutines.test.UnconfinedTestDispatcher
3636
import kotlinx.coroutines.test.advanceTimeBy
37+
import kotlinx.coroutines.test.advanceUntilIdle
3738
import kotlinx.coroutines.test.resetMain
3839
import kotlinx.coroutines.test.runCurrent
3940
import kotlinx.coroutines.test.runTest
@@ -63,6 +64,7 @@ import org.meshtastic.core.repository.UiPrefs
6364
import org.meshtastic.core.testing.FakeLockdownCoordinator
6465
import org.meshtastic.core.testing.FakeNodeRepository
6566
import org.meshtastic.feature.settings.navigation.ConfigRoute
67+
import org.meshtastic.proto.Channel
6668
import org.meshtastic.proto.ChannelSet
6769
import org.meshtastic.proto.ChannelSettings
6870
import org.meshtastic.proto.Config
@@ -351,6 +353,41 @@ class RadioConfigViewModelTest {
351353
assertEquals(new, viewModel.radioConfigState.value.channelList)
352354
}
353355

356+
@Test
357+
fun `updateChannels writes changed channels sequentially in index order`() = runTest {
358+
val node = Node(num = 123, user = User(id = "!123"))
359+
nodeRepository.setNodes(listOf(node))
360+
viewModel = createViewModel()
361+
362+
val channelA = ChannelSettings(name = "A")
363+
val channelB = ChannelSettings(name = "B")
364+
val channelC = ChannelSettings(name = "C")
365+
val old = listOf(channelA, channelB, channelC)
366+
val new = listOf(channelA, channelC, channelB)
367+
val writtenIndexes = mutableListOf<Int>()
368+
var activeWrites = 0
369+
var maxConcurrentWrites = 0
370+
371+
everySuspend { radioConfigUseCase.setRemoteChannel(any(), any()) } calls
372+
{
373+
val channel = it.args[1] as Channel
374+
activeWrites++
375+
maxConcurrentWrites = maxOf(maxConcurrentWrites, activeWrites)
376+
writtenIndexes.add(channel.index)
377+
delay(1)
378+
activeWrites--
379+
writtenIndexes.size
380+
}
381+
382+
viewModel.updateChannels(new, old)
383+
advanceUntilIdle()
384+
385+
assertEquals(listOf(1, 2), writtenIndexes)
386+
assertEquals(1, maxConcurrentWrites)
387+
assertEquals(new, viewModel.radioConfigState.value.channelList)
388+
verifySuspend(exactly(2)) { radioConfigUseCase.setRemoteChannel(123, any()) }
389+
}
390+
354391
@Test
355392
fun `setResponseStateLoading for REBOOT calls useCase after config response`() = runTest {
356393
val node = Node(num = 123, user = User(id = "!123"))

0 commit comments

Comments
 (0)