From c7ff9b4ef026188947e5412efaaff25018ab0ea5 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:11:16 +0000 Subject: [PATCH] fix(settings): stop profile import dropping Mesh Beacon settings (#7416) --- .../data/manager/MeshConfigHandlerImpl.kt | 8 +- .../data/manager/ConfigSummaryCoverageTest.kt | 49 ++++++++ .../ModuleConfigSummaryCoverageTest.kt | 50 ++++++++ .../core/datastore/ModuleConfigDataSource.kt | 3 + .../ModuleConfigDataSourceCoverageTest.kt | 63 ++++++++++ .../usecase/settings/InstallProfileUseCase.kt | 3 + .../settings/InstallProfileUseCaseTest.kt | 2 +- .../InstallProfileModuleConfigCoverageTest.kt | 118 ++++++++++++++++++ docs/en/user/settings-module-admin.md | 9 +- .../settings/radio/ModuleConfigMerge.kt | 43 +++++++ .../settings/radio/RadioConfigViewModel.kt | 64 +--------- .../component/MeshBeaconConfigItemList.kt | 2 + .../radio/RadioConfigViewModelTest.kt | 42 +++++++ .../radio/ModuleConfigMergeCoverageTest.kt | 48 +++++++ 14 files changed, 439 insertions(+), 65 deletions(-) create mode 100644 core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ConfigSummaryCoverageTest.kt create mode 100644 core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ModuleConfigSummaryCoverageTest.kt create mode 100644 core/datastore/src/jvmTest/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSourceCoverageTest.kt create mode 100644 core/domain/src/jvmTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileModuleConfigCoverageTest.kt create mode 100644 feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMerge.kt create mode 100644 feature/settings/src/jvmTest/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMergeCoverageTest.kt diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt index 4acd3294ef..ea29ae5443 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt @@ -172,7 +172,7 @@ class MeshConfigHandlerImpl( } /** Returns a short summary of which Config variant is set. */ -private fun Config.summarize(): String = when { +internal fun Config.summarize(): String = when { device != null -> "device" position != null -> "position" power != null -> "power" @@ -181,12 +181,14 @@ private fun Config.summarize(): String = when { lora != null -> "lora" bluetooth != null -> "bluetooth" security != null -> "security" + sessionkey != null -> "sessionkey" + device_ui != null -> "device_ui" else -> "unknown" } /** Returns a short summary of which ModuleConfig variant is set. */ @Suppress("CyclomaticComplexMethod") -private fun ModuleConfig.summarize(): String = when { +internal fun ModuleConfig.summarize(): String = when { mqtt != null -> "mqtt" serial != null -> "serial" external_notification != null -> "external_notification" @@ -201,6 +203,8 @@ private fun ModuleConfig.summarize(): String = when { detection_sensor != null -> "detection_sensor" paxcounter != null -> "paxcounter" statusmessage != null -> "statusmessage" + traffic_management != null -> "traffic_management" tak != null -> "tak" + mesh_beacon != null -> "mesh_beacon" else -> "unknown" } diff --git a/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ConfigSummaryCoverageTest.kt b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ConfigSummaryCoverageTest.kt new file mode 100644 index 0000000000..e27423a892 --- /dev/null +++ b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ConfigSummaryCoverageTest.kt @@ -0,0 +1,49 @@ +/* + * 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.data.manager + +import com.squareup.wire.ProtoAdapter +import com.squareup.wire.WireField +import org.meshtastic.proto.Config +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** Walks Wire's generated oneof, so a variant the proto grows fails here by name. JVM-only: reads `@WireField`. */ +class ConfigSummaryCoverageTest { + + @Test + fun `every Config variant is summarized by its field name`() { + val variants = + Config::class.java.declaredFields.filter { + it.getAnnotation(WireField::class.java)?.oneofName == "payload_variant" + } + assertTrue(variants.isNotEmpty(), "found no @WireField fields in the Config payload_variant oneof") + assertTrue(variants.any { it.name == "lora" }, "lora is not among the oneof fields found") + + val mislabelled = + variants + .associate { field -> + val value = (field.type.getField("ADAPTER").get(null) as ProtoAdapter<*>).decode(ByteArray(0)) + val config = Config.Builder().also { it.javaClass.getField(field.name).set(it, value) }.build() + field.name to config.summarize() + } + .filter { (name, summary) -> name != summary } + + assertEquals(emptyMap(), mislabelled, "these variants are logged under the wrong name") + } +} diff --git a/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ModuleConfigSummaryCoverageTest.kt b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ModuleConfigSummaryCoverageTest.kt new file mode 100644 index 0000000000..45e79138f0 --- /dev/null +++ b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/ModuleConfigSummaryCoverageTest.kt @@ -0,0 +1,50 @@ +/* + * 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.data.manager + +import com.squareup.wire.ProtoAdapter +import com.squareup.wire.WireField +import org.meshtastic.proto.ModuleConfig +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** Walks Wire's generated oneof, so a variant the proto grows fails here by name. JVM-only: reads `@WireField`. */ +class ModuleConfigSummaryCoverageTest { + + @Test + fun `every ModuleConfig variant is summarized by its field name`() { + val variants = + ModuleConfig::class.java.declaredFields.filter { + it.getAnnotation(WireField::class.java)?.oneofName == "payload_variant" + } + assertTrue(variants.isNotEmpty(), "found no @WireField fields in the ModuleConfig payload_variant oneof") + assertTrue(variants.any { it.name == "mesh_beacon" }, "mesh_beacon is not among the oneof fields found") + + val mislabelled = + variants + .associate { field -> + val value = (field.type.getField("ADAPTER").get(null) as ProtoAdapter<*>).decode(ByteArray(0)) + val config = + ModuleConfig.Builder().also { it.javaClass.getField(field.name).set(it, value) }.build() + field.name to config.summarize() + } + .filter { (name, summary) -> name != summary } + + assertEquals(emptyMap(), mislabelled, "these variants are logged under the wrong name") + } +} diff --git a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSource.kt b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSource.kt index 8352730667..c58fcf6e8a 100644 --- a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSource.kt +++ b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSource.kt @@ -85,6 +85,9 @@ class ModuleConfigDataSource(private val moduleConfigStore: CoreModuleConfigData config.statusmessage != null -> current.newBuilder().also { wb -> wb.statusmessage = config.statusmessage }.build() + config.traffic_management != null -> + current.newBuilder().also { wb -> wb.traffic_management = config.traffic_management }.build() + config.tak != null -> current.newBuilder().also { wb -> wb.tak = config.tak }.build() config.mesh_beacon != null -> diff --git a/core/datastore/src/jvmTest/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSourceCoverageTest.kt b/core/datastore/src/jvmTest/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSourceCoverageTest.kt new file mode 100644 index 0000000000..11cb02af46 --- /dev/null +++ b/core/datastore/src/jvmTest/kotlin/org/meshtastic/core/datastore/ModuleConfigDataSourceCoverageTest.kt @@ -0,0 +1,63 @@ +/* + * 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.datastore + +import com.squareup.wire.ProtoAdapter +import com.squareup.wire.WireField +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import org.meshtastic.core.datastore.di.CoreModuleConfigDataStore +import org.meshtastic.proto.LocalModuleConfig +import org.meshtastic.proto.ModuleConfig +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** Walks Wire's generated oneof, so a variant the proto grows fails here by name. JVM-only: reads `@WireField`. */ +class ModuleConfigDataSourceCoverageTest { + + private class InMemoryModuleConfigStore : CoreModuleConfigDataStore { + override val data = MutableStateFlow(LocalModuleConfig.Builder().build()) + + override suspend fun updateData(transform: suspend (t: LocalModuleConfig) -> LocalModuleConfig) = + transform(data.value).also { data.value = it } + } + + @Test + fun `every ModuleConfig variant is persisted into its LocalModuleConfig section`() = runTest { + val store = InMemoryModuleConfigStore() + val dataSource = ModuleConfigDataSource(store) + val variants = + ModuleConfig::class.java.declaredFields.filter { + it.getAnnotation(WireField::class.java)?.oneofName == "payload_variant" + } + assertTrue(variants.isNotEmpty(), "found no @WireField fields in the ModuleConfig payload_variant oneof") + assertTrue(variants.any { it.name == "mesh_beacon" }, "mesh_beacon is not among the oneof fields found") + + variants.forEach { field -> + val value = (field.type.getField("ADAPTER").get(null) as ProtoAdapter<*>).decode(ByteArray(0)) + dataSource.setLocalModuleConfig( + ModuleConfig.Builder().also { it.javaClass.getField(field.name).set(it, value) }.build(), + ) + } + + val stored = store.data.first() + val dropped = variants.map { it.name }.filter { LocalModuleConfig::class.java.getField(it).get(stored) == null } + assertEquals(emptyList(), dropped, "these variants are never persisted, or a later one cleared them") + } +} diff --git a/core/domain/src/commonMain/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCase.kt b/core/domain/src/commonMain/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCase.kt index 9bc2415591..5cea6d86ae 100644 --- a/core/domain/src/commonMain/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCase.kt +++ b/core/domain/src/commonMain/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCase.kt @@ -177,7 +177,10 @@ constructor( } lmc.paxcounter?.let { setModuleConfig(ModuleConfig.Builder().also { wb -> wb.paxcounter = it }.build()) } lmc.statusmessage?.let { setModuleConfig(ModuleConfig.Builder().also { wb -> wb.statusmessage = it }.build()) } + // traffic_management is not installed: it has no settings screen, and a node built without the module exports + // position_min_interval_secs = 0, which would silently turn off this node's default-on position dedup. lmc.tak?.let { setModuleConfig(ModuleConfig.Builder().also { wb -> wb.tak = it }.build()) } + lmc.mesh_beacon?.let { setModuleConfig(ModuleConfig.Builder().also { wb -> wb.mesh_beacon = it }.build()) } } private suspend fun AdminEditScope.installChannelsAndLora( diff --git a/core/domain/src/commonTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCaseTest.kt b/core/domain/src/commonTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCaseTest.kt index bfefe1b32a..f3bc028e27 100644 --- a/core/domain/src/commonTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCaseTest.kt +++ b/core/domain/src/commonTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileUseCaseTest.kt @@ -87,7 +87,7 @@ class InstallProfileUseCaseTest { } @Test - fun `invoke installs all sections of a full profile`() = runTest { + fun `invoke opens an edit session for a populated profile`() = runTest { val profile = DeviceProfile.Builder() .also { wb -> diff --git a/core/domain/src/jvmTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileModuleConfigCoverageTest.kt b/core/domain/src/jvmTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileModuleConfigCoverageTest.kt new file mode 100644 index 0000000000..2cf4a9f267 --- /dev/null +++ b/core/domain/src/jvmTest/kotlin/org/meshtastic/core/domain/usecase/settings/InstallProfileModuleConfigCoverageTest.kt @@ -0,0 +1,118 @@ +/* + * 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.domain.usecase.settings + +import com.squareup.wire.Message +import com.squareup.wire.ProtoAdapter +import com.squareup.wire.WireField +import kotlinx.coroutines.test.runTest +import org.meshtastic.core.testing.FakeRadioConfigRepository +import org.meshtastic.core.testing.FakeRadioController +import org.meshtastic.proto.DeviceProfile +import org.meshtastic.proto.LocalModuleConfig +import org.meshtastic.proto.ModuleConfig +import java.lang.reflect.Field +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Walks Wire's generated fields, so a module config section the proto grows fails here by name until profile install + * writes it or [notInstalled] names it. JVM-only because it reads `@WireField`. + */ +class InstallProfileModuleConfigCoverageTest { + + private val notInstalled = + mapOf( + "traffic_management" to + "no settings screen, and a node built without the module exports its position dedup switched off", + ) + + @Test + fun `every section named as not installed is still a LocalModuleConfig section`() { + val stale = notInstalled.keys - localModuleConfigSections().map { it.name }.toSet() + + assertEquals(emptySet(), stale, "these sections left LocalModuleConfig, so drop them from notInstalled") + } + + @Test + fun `every LocalModuleConfig section has a ModuleConfig variant of the same name and type`() { + val variantTypes = moduleConfigVariants().associate { it.name to it.type } + + val unmatched = localModuleConfigSections().filter { variantTypes[it.name] != it.type }.map { it.name } + + assertEquals(emptyList(), unmatched, "these LocalModuleConfig sections have no matching ModuleConfig variant") + } + + @Test + fun `profile install writes every section except those named as not installed`() = runTest { + val radioController = FakeRadioController() + val sections = localModuleConfigSections() + val moduleConfig = + LocalModuleConfig.Builder() + .also { builder -> + sections.forEach { builder.javaClass.getField(it.name).set(builder, defaultOf(it)) } + } + .build() + + InstallProfileUseCase(radioController, FakeRadioConfigRepository())( + destNum = 1234, + profile = DeviceProfile.Builder().also { it.module_config = moduleConfig }.build(), + currentUser = null, + currentLoraConfig = null, + isLocal = false, + ) + + val written = radioController.allModuleConfigs.associate { it.onlyVariant() } + val installed = sections.filter { it.name !in notInstalled } + assertEquals( + emptyList(), + installed.map { it.name } - written.keys, + "profile install never writes these module config sections", + ) + assertEquals(emptySet(), notInstalled.keys intersect written.keys, "profile install writes these after all") + assertEquals(radioController.allModuleConfigs.size, written.size, "a section was written more than once") + installed.forEach { assertEquals(it.get(moduleConfig), written[it.name], "${it.name} was written changed") } + } + + private fun ModuleConfig.onlyVariant(): Pair = + moduleConfigVariants().mapNotNull { field -> field.get(this)?.let { field.name to it } }.single() + + private fun localModuleConfigSections(): List = LocalModuleConfig::class + .java + .declaredFields + .filter { it.isAnnotationPresent(WireField::class.java) && Message::class.java.isAssignableFrom(it.type) } + .also { sections -> + assertTrue(sections.isNotEmpty(), "found no @WireField message fields on LocalModuleConfig") + assertTrue(sections.any { it.name == "mesh_beacon" }, "mesh_beacon is not among the sections found") + } + + private fun moduleConfigVariants(): List = ModuleConfig::class + .java + .declaredFields + .filter { it.getAnnotation(WireField::class.java)?.oneofName == "payload_variant" } + .also { variants -> + assertTrue( + variants.isNotEmpty(), + "found no @WireField fields in the ModuleConfig payload_variant oneof", + ) + assertTrue(variants.any { it.name == "mesh_beacon" }, "mesh_beacon is not among the oneof fields found") + } + + private fun defaultOf(field: Field): Any? = + (field.type.getField("ADAPTER").get(null) as ProtoAdapter<*>).decode(ByteArray(0)) +} diff --git a/docs/en/user/settings-module-admin.md b/docs/en/user/settings-module-admin.md index 82451c6043..01a09b7aa0 100644 --- a/docs/en/user/settings-module-admin.md +++ b/docs/en/user/settings-module-admin.md @@ -2,7 +2,7 @@ title: Settings — Modules & Admin parent: User Guide nav_order: 8 -last_updated: 2026-09-19 +last_updated: 2026-09-28 description: Configure optional feature modules (MQTT, telemetry, canned messages, TAK, and more) and perform device administration. aliases: - modules @@ -30,7 +30,7 @@ Module settings use a card-based layout with toggle switches, dropdowns, text fi Every module lives under **Settings → Module configuration**. -> ⚠️ **Important:** Saving a module screen restarts the node — the button reads **Save & restart**, and the node is unreachable for a few seconds afterwards. External Notification and Mesh Beacon are the exceptions: their button reads **Save**, and the node may still restart for some changes. +> ⚠️ **Important:** Saving a module screen restarts the node: the button reads **Save & restart**, and the node is unreachable for a few seconds afterwards. External Notification and Mesh Beacon are the exceptions: their button reads **Save**. External Notification may still restart the node for some changes, while a Mesh Beacon change applies without a restart. ### MQTT module @@ -292,8 +292,9 @@ Remotely configure nodes that share your admin key: ### Backup & Restore **Settings → Backup & Restore** writes the connected node's whole configuration to a file with -**Export configuration**, and reads a saved file back in with **Import configuration**. Export -before a factory reset, or to copy one node's setup onto another. The section is shown for your +**Export configuration**, and reads a saved file back in with **Import configuration**. +Traffic Management settings are exported but not applied on import. Export before a factory +reset, or to copy one node's setup onto another. The section is shown for your own node only, not over remote admin. ### Advanced diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMerge.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMerge.kt new file mode 100644 index 0000000000..a88f8073f7 --- /dev/null +++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMerge.kt @@ -0,0 +1,43 @@ +/* + * 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.feature.settings.radio + +import org.meshtastic.proto.LocalModuleConfig +import org.meshtastic.proto.ModuleConfig + +/** Replaces the section [config] carries and keeps every other section. */ +internal fun LocalModuleConfig.mergedWith(config: ModuleConfig): LocalModuleConfig = newBuilder() + .also { wb -> + wb.mqtt = config.mqtt ?: mqtt + wb.serial = config.serial ?: serial + wb.external_notification = config.external_notification ?: external_notification + wb.store_forward = config.store_forward ?: store_forward + wb.range_test = config.range_test ?: range_test + wb.telemetry = config.telemetry ?: telemetry + wb.canned_message = config.canned_message ?: canned_message + wb.audio = config.audio ?: audio + wb.remote_hardware = config.remote_hardware ?: remote_hardware + wb.neighbor_info = config.neighbor_info ?: neighbor_info + wb.ambient_lighting = config.ambient_lighting ?: ambient_lighting + wb.detection_sensor = config.detection_sensor ?: detection_sensor + wb.paxcounter = config.paxcounter ?: paxcounter + wb.statusmessage = config.statusmessage ?: statusmessage + wb.traffic_management = config.traffic_management ?: traffic_management + wb.tak = config.tak ?: tak + wb.mesh_beacon = config.mesh_beacon ?: mesh_beacon + } + .build() diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt index d6e241ed36..081fafbb6e 100644 --- a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt +++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt @@ -599,37 +599,10 @@ open class RadioConfigViewModel( } } - @Suppress("CyclomaticComplexMethod") fun setModuleConfig(config: ModuleConfig) { val destNum = destNum ?: destNode.value?.num ?: return safeLaunch(tag = "setModuleConfig") { - _radioConfigState.update { state -> - state.copy( - moduleConfig = - state.moduleConfig - .newBuilder() - .also { wb -> - wb.mqtt = config.mqtt ?: state.moduleConfig.mqtt - wb.serial = config.serial ?: state.moduleConfig.serial - wb.external_notification = - config.external_notification ?: state.moduleConfig.external_notification - wb.store_forward = config.store_forward ?: state.moduleConfig.store_forward - wb.range_test = config.range_test ?: state.moduleConfig.range_test - wb.telemetry = config.telemetry ?: state.moduleConfig.telemetry - wb.canned_message = config.canned_message ?: state.moduleConfig.canned_message - wb.audio = config.audio ?: state.moduleConfig.audio - wb.remote_hardware = config.remote_hardware ?: state.moduleConfig.remote_hardware - wb.neighbor_info = config.neighbor_info ?: state.moduleConfig.neighbor_info - wb.ambient_lighting = config.ambient_lighting ?: state.moduleConfig.ambient_lighting - wb.detection_sensor = config.detection_sensor ?: state.moduleConfig.detection_sensor - wb.paxcounter = config.paxcounter ?: state.moduleConfig.paxcounter - wb.statusmessage = config.statusmessage ?: state.moduleConfig.statusmessage - wb.tak = config.tak ?: state.moduleConfig.tak - wb.mesh_beacon = config.mesh_beacon ?: state.moduleConfig.mesh_beacon - } - .build(), - ) - } + _radioConfigState.update { state -> state.copy(moduleConfig = state.moduleConfig.mergedWith(config)) } expectRestartIfLocal(config.saveRebootBehavior()) radioConfigUseCase.setModuleConfig(destNum, config, onRequestId = ::registerWriteRequestId) } @@ -1342,35 +1315,8 @@ open class RadioConfigViewModel( } is RadioResponseResult.ModuleConfigResponse -> { - val response = result.config _radioConfigState.update { state -> - state.copy( - moduleConfig = - state.moduleConfig - .newBuilder() - .also { wb -> - wb.mqtt = response.mqtt ?: state.moduleConfig.mqtt - wb.serial = response.serial ?: state.moduleConfig.serial - wb.external_notification = - response.external_notification ?: state.moduleConfig.external_notification - wb.store_forward = response.store_forward ?: state.moduleConfig.store_forward - wb.range_test = response.range_test ?: state.moduleConfig.range_test - wb.telemetry = response.telemetry ?: state.moduleConfig.telemetry - wb.canned_message = response.canned_message ?: state.moduleConfig.canned_message - wb.audio = response.audio ?: state.moduleConfig.audio - wb.remote_hardware = response.remote_hardware ?: state.moduleConfig.remote_hardware - wb.neighbor_info = response.neighbor_info ?: state.moduleConfig.neighbor_info - wb.ambient_lighting = - response.ambient_lighting ?: state.moduleConfig.ambient_lighting - wb.detection_sensor = - response.detection_sensor ?: state.moduleConfig.detection_sensor - wb.paxcounter = response.paxcounter ?: state.moduleConfig.paxcounter - wb.statusmessage = response.statusmessage ?: state.moduleConfig.statusmessage - wb.tak = response.tak ?: state.moduleConfig.tak - wb.mesh_beacon = response.mesh_beacon ?: state.moduleConfig.mesh_beacon - } - .build(), - ) + state.copy(moduleConfig = state.moduleConfig.mergedWith(result.config)) } if (!isLateRemoteRead) incrementCompleted() } @@ -1536,6 +1482,8 @@ internal fun Config.saveRebootBehavior(): RebootBehavior = when { else -> RebootBehavior.MAY_RESTART } -/** Firmware `AdminModule::handleSetModuleConfig` reboots for every module section except status message. */ +/** + * Firmware `AdminModule::handleSetModuleConfig` reboots for every module section except status message and Mesh Beacon. + */ internal fun ModuleConfig.saveRebootBehavior(): RebootBehavior = - if (statusmessage != null) RebootBehavior.NEVER else RebootBehavior.ALWAYS + if (statusmessage != null || mesh_beacon != null) RebootBehavior.NEVER else RebootBehavior.ALWAYS diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/MeshBeaconConfigItemList.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/MeshBeaconConfigItemList.kt index cd979e4f49..41c14a3795 100644 --- a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/MeshBeaconConfigItemList.kt +++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/MeshBeaconConfigItemList.kt @@ -68,6 +68,7 @@ import org.meshtastic.core.ui.component.RegularPreference import org.meshtastic.core.ui.component.SwitchPreference import org.meshtastic.core.ui.component.TitledCard import org.meshtastic.feature.settings.radio.RadioConfigViewModel +import org.meshtastic.feature.settings.radio.RebootBehavior import org.meshtastic.feature.settings.util.FixedUpdateIntervals import org.meshtastic.feature.settings.util.IntervalConfiguration import org.meshtastic.feature.settings.util.toDisplayString @@ -181,6 +182,7 @@ fun MeshBeaconConfigScreen(viewModel: RadioConfigViewModel, onBack: () -> Unit, // broadcast fields verbatim. saveEnabled = meshBeaconSaveEnabled(state.connected, radioLora.use_preset, intervalValid, state.channelList.isNotEmpty()), + rebootBehavior = RebootBehavior.NEVER, responseState = state.responseState, onDismissPacketResponse = viewModel::clearPacketResponse, onSave = { diff --git a/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt b/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt index 692bb449ac..e8deda1018 100644 --- a/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt +++ b/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt @@ -2804,6 +2804,48 @@ class RadioConfigViewModelTest { assertFalse(nodeRestartTracker.restartExpected.value) } + + @Test + fun `local module save that reboots opens the restart window`() = runTest { + val node = Node(num = 123, user = User.Builder().also { wb -> wb.id = "!123" }.build()) + nodeRepository.setNodes(listOf(node)) + nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123)) + viewModel = createViewModel() + runCurrent() + everySuspend { radioConfigUseCase.setModuleConfig(any(), any(), any()) } returns 42 + + nodeRestartTracker.onConnected() + viewModel.setModuleConfig( + ModuleConfig.Builder() + .also { wb -> wb.mqtt = ModuleConfig.MQTTConfig.Builder().also { wb -> wb.enabled = true }.build() } + .build(), + ) + runCurrent() + + assertTrue(nodeRestartTracker.restartExpected.value) + } + + @Test + fun `local Mesh Beacon save does not open the restart window`() = runTest { + val node = Node(num = 123, user = User.Builder().also { wb -> wb.id = "!123" }.build()) + nodeRepository.setNodes(listOf(node)) + nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123)) + viewModel = createViewModel() + runCurrent() + everySuspend { radioConfigUseCase.setModuleConfig(any(), any(), any()) } returns 42 + + nodeRestartTracker.onConnected() + viewModel.setModuleConfig( + ModuleConfig.Builder() + .also { wb -> + wb.mesh_beacon = MeshBeaconConfig.Builder().also { wb -> wb.broadcast_message = "hi" }.build() + } + .build(), + ) + runCurrent() + + assertFalse(nodeRestartTracker.restartExpected.value) + } } /** Extracts the trailing `onRequestId` callback from a mocked request method's args. */ diff --git a/feature/settings/src/jvmTest/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMergeCoverageTest.kt b/feature/settings/src/jvmTest/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMergeCoverageTest.kt new file mode 100644 index 0000000000..f2a6c60388 --- /dev/null +++ b/feature/settings/src/jvmTest/kotlin/org/meshtastic/feature/settings/radio/ModuleConfigMergeCoverageTest.kt @@ -0,0 +1,48 @@ +/* + * 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.feature.settings.radio + +import com.squareup.wire.ProtoAdapter +import com.squareup.wire.WireField +import org.meshtastic.proto.LocalModuleConfig +import org.meshtastic.proto.ModuleConfig +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** Walks Wire's generated oneof, so a variant the proto grows fails here by name. JVM-only: reads `@WireField`. */ +class ModuleConfigMergeCoverageTest { + + @Test + fun `merging every ModuleConfig variant in turn fills and keeps every LocalModuleConfig section`() { + val variants = + ModuleConfig::class.java.declaredFields.filter { + it.getAnnotation(WireField::class.java)?.oneofName == "payload_variant" + } + assertTrue(variants.isNotEmpty(), "found no @WireField fields in the ModuleConfig payload_variant oneof") + assertTrue(variants.any { it.name == "mesh_beacon" }, "mesh_beacon is not among the oneof fields found") + + val merged = + variants.fold(LocalModuleConfig.Builder().build()) { acc, field -> + val value = (field.type.getField("ADAPTER").get(null) as ProtoAdapter<*>).decode(ByteArray(0)) + acc.mergedWith(ModuleConfig.Builder().also { it.javaClass.getField(field.name).set(it, value) }.build()) + } + + val dropped = variants.map { it.name }.filter { LocalModuleConfig::class.java.getField(it).get(merged) == null } + assertEquals(emptyList(), dropped, "these variants are never merged, or a later merge cleared them") + } +}