fix: ensure proper channel updates to `ChannelSetRepository`

app/src/main/java/com/geeksville/mesh/model/UIState.kt

@@ -15,6 +15,7 @@ import androidx.lifecycle.asLiveData
 import androidx.lifecycle.viewModelScope
 import com.geeksville.mesh.android.Logging
 import com.geeksville.mesh.*
+import com.geeksville.mesh.ChannelProtos.ChannelSettings
 import com.geeksville.mesh.ClientOnlyProtos.DeviceProfile
 import com.geeksville.mesh.ConfigProtos.Config
 import com.geeksville.mesh.ModuleConfigProtos.ModuleConfig
@@ -56,7 +57,6 @@ import java.io.InputStream
 import java.text.SimpleDateFormat
 import java.util.Locale
 import javax.inject.Inject
-import kotlin.math.max
 import kotlin.math.roundToInt
 
 /// Given a human name, strip out the first letter of the first three words and return that as the initials for
@@ -85,7 +85,7 @@ fun getInitials(nameIn: String): String {
 @HiltViewModel
 class UIViewModel @Inject constructor(
     private val app: Application,
-    radioConfigRepository: RadioConfigRepository,
+    private val radioConfigRepository: RadioConfigRepository,
     private val meshLogRepository: MeshLogRepository,
     private val packetRepository: PacketRepository,
     private val quickChatActionRepository: QuickChatActionRepository,
@@ -451,23 +451,51 @@ class UIViewModel @Inject constructor(
         setModuleConfig(myNodeNum ?: return, config)
     }
 
-    /// Convert the channels array to and from [AppOnlyProtos.ChannelSet]
-    private var _channelSet: AppOnlyProtos.ChannelSet
-        get() = channels.value.protobuf
-        set(value) {
-            (0 until max(_channelSet.settingsCount, value.settingsCount)).map { i ->
-                channel {
+    /**
+     * Updates channels to match the [new] list. Only channels with changes are updated.
+     *
+     * @param destNum Destination node number.
+     * @param old The current [ChannelSettings] list.
+     * @param new The updated [ChannelSettings] list.
+     */
+    fun updateChannels(
+        destNum: Int,
+        old: List<ChannelSettings>,
+        new: List<ChannelSettings>,
+    ) {
+        buildList {
+            for (i in 0..maxOf(old.lastIndex, new.lastIndex)) {
+                if (old.getOrNull(i) != new.getOrNull(i)) add(channel {
                     role = when (i) {
                         0 -> ChannelProtos.Channel.Role.PRIMARY
-                        in 1 until value.settingsCount -> ChannelProtos.Channel.Role.SECONDARY
+                        in 1..new.lastIndex -> ChannelProtos.Channel.Role.SECONDARY
                         else -> ChannelProtos.Channel.Role.DISABLED
                     }
                     index = i
-                    settings = value.settingsList.getOrNull(i) ?: channelSettings { }
+                    settings = new.getOrNull(i) ?: channelSettings { }
+                })
             }
-            }.forEach {
-                meshService?.setChannel(it.toByteArray())
+        }.forEach { setRemoteChannel(destNum, it) }
+
+        if (destNum == myNodeNum) viewModelScope.launch {
+            radioConfigRepository.replaceAllSettings(new)
         }
+    }
+
+    private fun updateChannels(
+        old: List<ChannelSettings>,
+        new: List<ChannelSettings>
+    ) {
+        updateChannels(myNodeNum ?: return, old, new)
+    }
+
+    /**
+     * Convert the [channels] array to and from [ChannelSet]
+     */
+    private var _channelSet: AppOnlyProtos.ChannelSet
+        get() = channels.value.protobuf
+        set(value) {
+            updateChannels(channelSet.settingsList, value.settingsList)
 
             val newConfig = config { lora = value.loraConfig }
             if (config.lora != newConfig.lora) setConfig(newConfig)
@@ -476,12 +504,16 @@ class UIViewModel @Inject constructor(
 
     /// Set the radio config (also updates our saved copy in preferences)
     fun setChannels(channelSet: ChannelSet) {
-        debug("Setting new channels!")
         this._channelSet = channelSet.protobuf
     }
 
-    fun setRemoteChannel(destNum: Int, channel: ChannelProtos.Channel) {
+    private fun setRemoteChannel(destNum: Int, channel: ChannelProtos.Channel) {
+        try {
+            debug("Sending channel ${channel.index} to $destNum")
             meshService?.setRemoteChannel(destNum, channel.toByteArray())
+        } catch (ex: RemoteException) {
+            errormsg("Can't set channel on radio ${ex.message}")
+        }
     }
 
     /// our name in hte radio

app/src/main/java/com/geeksville/mesh/repository/datastore/ChannelSetRepository.kt

@@ -35,6 +35,18 @@ class ChannelSetRepository @Inject constructor(
         }
     }
 
+    suspend fun clearSettings() {
+        channelSetStore.updateData { preference ->
+            preference.toBuilder().clearSettings().build()
+        }
+    }
+
+    suspend fun addAllSettings(settingsList: List<ChannelSettings>) {
+        channelSetStore.updateData { preference ->
+            preference.toBuilder().addAllSettings(settingsList).build()
+        }
+    }
+
     /**
      * Updates the [ChannelSettings] list with the provided channel and returns the index of the
      * admin channel after the update (if not found, returns 0).
@@ -42,11 +54,7 @@ class ChannelSetRepository @Inject constructor(
     suspend fun updateChannelSettings(channel: Channel): Int {
         channelSetStore.updateData { preference ->
             if (preference.settingsCount > channel.index) {
-                if (channel.role == Channel.Role.DISABLED) {
-                    preference.toBuilder().removeSettings(channel.index).build()
-                } else {
                 preference.toBuilder().setSettings(channel.index, channel.settings).build()
-                }
             } else {
                 preference.toBuilder().addSettings(channel.settings).build()
             }

app/src/main/java/com/geeksville/mesh/repository/datastore/RadioConfigRepository.kt

@@ -31,6 +31,14 @@ class RadioConfigRepository @Inject constructor(
         channelSetRepository.clearChannelSet()
     }
 
+    /**
+     * Replaces the [ChannelSettings] list with a new [settingsList].
+     */
+    suspend fun replaceAllSettings(settingsList: List<ChannelSettings>) {
+        channelSetRepository.clearSettings()
+        channelSetRepository.addAllSettings(settingsList)
+    }
+
     /**
      * Updates the [ChannelSettings] list with the provided channel and returns the index of the
      * admin channel after the update (if not found, returns 0).

app/src/main/java/com/geeksville/mesh/service/MeshService.kt

@@ -1715,7 +1715,6 @@ class MeshService : Service(), Logging {
         override fun setRemoteChannel(destNum: Int, payload: ByteArray?) = toRemoteExceptions {
             val channel = ChannelProtos.Channel.parseFrom(payload)
             sendToRadio(newMeshPacketTo(destNum).buildAdminPacket { setChannel = channel })
-            if (destNum == myNodeNum) updateChannelSettings(channel) // Update our local copy
         }
 
         override fun getRemoteChannel(id: Int, destNum: Int, index: Int) = toRemoteExceptions {

app/src/main/java/com/geeksville/mesh/ui/DeviceSettingsFragment.kt

@@ -67,8 +67,6 @@ import com.geeksville.mesh.NodeInfo
 import com.geeksville.mesh.Portnums
 import com.geeksville.mesh.R
 import com.geeksville.mesh.android.Logging
-import com.geeksville.mesh.channel
-import com.geeksville.mesh.channelSettings
 import com.geeksville.mesh.config
 import com.geeksville.mesh.deviceProfile
 import com.geeksville.mesh.model.UIViewModel
@@ -391,20 +389,7 @@ fun RadioConfigNavHost(node: NodeInfo, viewModel: UIViewModel = viewModel()) {
                 focusManager = focusManager,
                 onSaveClicked = { channelListInput ->
                     focusManager.clearFocus()
-                    (0 until channelList.size.coerceAtLeast(channelListInput.size)).map { i ->
-                        channel {
-                            role = when (i) {
-                                0 -> ChannelProtos.Channel.Role.PRIMARY
-                                in 1 until channelListInput.size -> ChannelProtos.Channel.Role.SECONDARY
-                                else -> ChannelProtos.Channel.Role.DISABLED
-                            }
-                            index = i
-                            settings = channelListInput.getOrNull(i) ?: channelSettings { }
-                        }
-                    }.forEach { newChannel ->
-                        if (newChannel.settings != channelList.getOrNull(newChannel.index))
-                            viewModel.setRemoteChannel(destNum, newChannel)
-                    }
+                    viewModel.updateChannels(destNum, channelList, channelListInput)
                     channelList.clear()
                     channelList.addAll(channelListInput)
                 }