diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt index f314bf26d..ea69e75fb 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt @@ -106,8 +106,8 @@ class PacketHandlerImpl( /** * Enqueue [packet] for transmission. Order is preserved for sequential calls from the same coroutine (mutex * acquisition is uncontested between sequential calls). Transactional sequences that require strict ordering across - * multiple calls — e.g. `beginEditSettings` → … → `commitEditSettings` — MUST be issued from a single coroutine; - * concurrent senders share FIFO only at the per-call grain. + * multiple calls — e.g. an `editSettings { … }` begin → writes → commit sequence — MUST be issued from a single + * coroutine; concurrent senders share FIFO only at the per-call grain. */ override suspend fun sendToRadio(packet: MeshPacket) { queueMutex.withLock { 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 f276b5e31..c48ba054d 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 @@ -18,6 +18,7 @@ package org.meshtastic.core.domain.usecase.settings import org.koin.core.annotation.Single import org.meshtastic.core.model.Position +import org.meshtastic.core.repository.AdminEditScope import org.meshtastic.core.repository.RadioController import org.meshtastic.proto.Config import org.meshtastic.proto.DeviceProfile @@ -37,118 +38,72 @@ open class InstallProfileUseCase constructor(private val radioController: RadioC * @param currentUser The current user configuration of the destination node (to preserve names if not in profile). */ open suspend operator fun invoke(destNum: Int, profile: DeviceProfile, currentUser: User?) { - radioController.beginEditSettings(destNum) - - installOwner(destNum, profile, currentUser) - installConfig(destNum, profile.config) - installFixedPosition(destNum, profile.fixed_position) - installModuleConfig(destNum, profile.module_config) - - radioController.commitEditSettings(destNum) + radioController.editSettings(destNum) { + installOwner(profile, currentUser) + installConfig(profile.config) + installFixedPosition(profile.fixed_position) + installModuleConfig(profile.module_config) + } } - private suspend fun installOwner(destNum: Int, profile: DeviceProfile, currentUser: User?) { + private suspend fun AdminEditScope.installOwner(profile: DeviceProfile, currentUser: User?) { if (profile.long_name != null || profile.short_name != null) { currentUser?.let { - val user = + setOwner( it.copy( long_name = profile.long_name ?: it.long_name, short_name = profile.short_name ?: it.short_name, - ) - radioController.setOwner(destNum, user, radioController.getPacketId()) + ), + ) } } } - private suspend fun installConfig(destNum: Int, config: LocalConfig?) { + private suspend fun AdminEditScope.installConfig(config: LocalConfig?) { config?.let { lc -> - lc.device?.let { radioController.setConfig(destNum, Config(device = it), radioController.getPacketId()) } - lc.position?.let { - radioController.setConfig(destNum, Config(position = it), radioController.getPacketId()) - } - lc.power?.let { radioController.setConfig(destNum, Config(power = it), radioController.getPacketId()) } - lc.network?.let { radioController.setConfig(destNum, Config(network = it), radioController.getPacketId()) } - lc.display?.let { radioController.setConfig(destNum, Config(display = it), radioController.getPacketId()) } - lc.lora?.let { radioController.setConfig(destNum, Config(lora = it), radioController.getPacketId()) } - lc.bluetooth?.let { - radioController.setConfig(destNum, Config(bluetooth = it), radioController.getPacketId()) - } - lc.security?.let { - radioController.setConfig(destNum, Config(security = it), radioController.getPacketId()) - } + lc.device?.let { setConfig(Config(device = it)) } + lc.position?.let { setConfig(Config(position = it)) } + lc.power?.let { setConfig(Config(power = it)) } + lc.network?.let { setConfig(Config(network = it)) } + lc.display?.let { setConfig(Config(display = it)) } + lc.lora?.let { setConfig(Config(lora = it)) } + lc.bluetooth?.let { setConfig(Config(bluetooth = it)) } + lc.security?.let { setConfig(Config(security = it)) } } } - private suspend fun installFixedPosition(destNum: Int, fixedPosition: org.meshtastic.proto.Position?) { + private suspend fun AdminEditScope.installFixedPosition(fixedPosition: org.meshtastic.proto.Position?) { if (fixedPosition != null) { - radioController.setFixedPosition(destNum, Position(fixedPosition)) + setFixedPosition(Position(fixedPosition)) } } - private suspend fun installModuleConfig(destNum: Int, moduleConfig: LocalModuleConfig?) { + private suspend fun AdminEditScope.installModuleConfig(moduleConfig: LocalModuleConfig?) { moduleConfig?.let { lmc -> - installModuleConfigPart1(destNum, lmc) - installModuleConfigPart2(destNum, lmc) + installModuleConfigPart1(lmc) + installModuleConfigPart2(lmc) } } - private suspend fun installModuleConfigPart1(destNum: Int, lmc: LocalModuleConfig) { - lmc.mqtt?.let { - radioController.setModuleConfig(destNum, ModuleConfig(mqtt = it), radioController.getPacketId()) - } - lmc.serial?.let { - radioController.setModuleConfig(destNum, ModuleConfig(serial = it), radioController.getPacketId()) - } - lmc.external_notification?.let { - radioController.setModuleConfig( - destNum, - ModuleConfig(external_notification = it), - radioController.getPacketId(), - ) - } - lmc.store_forward?.let { - radioController.setModuleConfig(destNum, ModuleConfig(store_forward = it), radioController.getPacketId()) - } - lmc.range_test?.let { - radioController.setModuleConfig(destNum, ModuleConfig(range_test = it), radioController.getPacketId()) - } - lmc.telemetry?.let { - radioController.setModuleConfig(destNum, ModuleConfig(telemetry = it), radioController.getPacketId()) - } - lmc.canned_message?.let { - radioController.setModuleConfig(destNum, ModuleConfig(canned_message = it), radioController.getPacketId()) - } - lmc.audio?.let { - radioController.setModuleConfig(destNum, ModuleConfig(audio = it), radioController.getPacketId()) - } + private suspend fun AdminEditScope.installModuleConfigPart1(lmc: LocalModuleConfig) { + lmc.mqtt?.let { setModuleConfig(ModuleConfig(mqtt = it)) } + lmc.serial?.let { setModuleConfig(ModuleConfig(serial = it)) } + lmc.external_notification?.let { setModuleConfig(ModuleConfig(external_notification = it)) } + lmc.store_forward?.let { setModuleConfig(ModuleConfig(store_forward = it)) } + lmc.range_test?.let { setModuleConfig(ModuleConfig(range_test = it)) } + lmc.telemetry?.let { setModuleConfig(ModuleConfig(telemetry = it)) } + lmc.canned_message?.let { setModuleConfig(ModuleConfig(canned_message = it)) } + lmc.audio?.let { setModuleConfig(ModuleConfig(audio = it)) } } - private suspend fun installModuleConfigPart2(destNum: Int, lmc: LocalModuleConfig) { - lmc.remote_hardware?.let { - radioController.setModuleConfig(destNum, ModuleConfig(remote_hardware = it), radioController.getPacketId()) - } - lmc.neighbor_info?.let { - radioController.setModuleConfig(destNum, ModuleConfig(neighbor_info = it), radioController.getPacketId()) - } - lmc.ambient_lighting?.let { - radioController.setModuleConfig(destNum, ModuleConfig(ambient_lighting = it), radioController.getPacketId()) - } - lmc.detection_sensor?.let { - radioController.setModuleConfig(destNum, ModuleConfig(detection_sensor = it), radioController.getPacketId()) - } - lmc.paxcounter?.let { - radioController.setModuleConfig(destNum, ModuleConfig(paxcounter = it), radioController.getPacketId()) - } - lmc.statusmessage?.let { - radioController.setModuleConfig(destNum, ModuleConfig(statusmessage = it), radioController.getPacketId()) - } - lmc.traffic_management?.let { - radioController.setModuleConfig( - destNum, - ModuleConfig(traffic_management = it), - radioController.getPacketId(), - ) - } - lmc.tak?.let { radioController.setModuleConfig(destNum, ModuleConfig(tak = it), radioController.getPacketId()) } + private suspend fun AdminEditScope.installModuleConfigPart2(lmc: LocalModuleConfig) { + lmc.remote_hardware?.let { setModuleConfig(ModuleConfig(remote_hardware = it)) } + lmc.neighbor_info?.let { setModuleConfig(ModuleConfig(neighbor_info = it)) } + lmc.ambient_lighting?.let { setModuleConfig(ModuleConfig(ambient_lighting = it)) } + lmc.detection_sensor?.let { setModuleConfig(ModuleConfig(detection_sensor = it)) } + lmc.paxcounter?.let { setModuleConfig(ModuleConfig(paxcounter = it)) } + lmc.statusmessage?.let { setModuleConfig(ModuleConfig(statusmessage = it)) } + lmc.traffic_management?.let { setModuleConfig(ModuleConfig(traffic_management = it)) } + lmc.tak?.let { setModuleConfig(ModuleConfig(tak = it)) } } } 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 2c449344a..fc191f210 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 @@ -63,8 +63,7 @@ class InstallProfileUseCaseTest { fun `invoke calls begin and commit edit settings`() = runTest { useCase(1234, DeviceProfile(), User()) - assertTrue(radioController.beginEditSettingsCalled) - assertTrue(radioController.commitEditSettingsCalled) + assertTrue(radioController.editSettingsCalled) } @Test @@ -108,7 +107,6 @@ class InstallProfileUseCaseTest { useCase(1234, profile, org.meshtastic.proto.User(long_name = "Old")) - assertTrue(radioController.beginEditSettingsCalled) - assertTrue(radioController.commitEditSettingsCalled) + assertTrue(radioController.editSettingsCalled) } } 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 493180721..b5466797d 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 @@ -115,9 +115,36 @@ interface AdminController { // ── Batch edit ────────────────────────────────────────────────────────── - /** Signals the start of a batch configuration session. */ - suspend fun beginEditSettings(destNum: Int) - - /** Commits all pending configuration changes in a batch session. */ - suspend fun commitEditSettings(destNum: Int) + /** + * Runs [block] inside a begin/commit edit-settings transaction on [destNum]. + * + * The session is opened before [block] runs and committed after it returns normally, so callers can neither forget + * to commit nor leak a half-open session. Operations inside the block target [destNum] implicitly. Mirrors the + * SDK's `AdminApi.editSettings { }`. + * + * All admin packets for the session — begin, the [block]'s writes, and commit — are issued from the calling + * coroutine, which is required for the firmware to associate them with one transaction. + */ + suspend fun editSettings(destNum: Int, block: suspend AdminEditScope.() -> Unit) +} + +/** + * Configuration operations valid inside an [AdminController.editSettings] transaction, scoped to a single destination + * node so callers don't repeat the node number or manage packet IDs. Mirrors the SDK's `AdminEdit`. + */ +interface AdminEditScope { + /** Updates the owner (user info) on the session's node. */ + suspend fun setOwner(user: User) + + /** Updates the general configuration on the session's node. */ + suspend fun setConfig(config: Config) + + /** Updates a module configuration on the session's node. */ + suspend fun setModuleConfig(config: ModuleConfig) + + /** Updates a channel configuration on the session's node. */ + suspend fun setChannel(channel: Channel) + + /** Sets a fixed position on the session's node. */ + suspend fun setFixedPosition(position: Position) } diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt index 3dec3b288..7ebe4e74e 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt @@ -32,6 +32,7 @@ import org.meshtastic.core.model.MessageStatus import org.meshtastic.core.model.NodeAddress import org.meshtastic.core.model.Position import org.meshtastic.core.model.Reaction +import org.meshtastic.core.repository.AdminEditScope import org.meshtastic.core.repository.CommandSender import org.meshtastic.core.repository.DataPair import org.meshtastic.core.repository.MeshDataHandler @@ -379,12 +380,24 @@ class DirectRadioControllerImpl( // ── Edit Settings (transactional) ─────────────────────────────────────── - override suspend fun beginEditSettings(destNum: Int) { + override suspend fun editSettings(destNum: Int, block: suspend AdminEditScope.() -> Unit) { commandSender.sendAdmin(destNum) { AdminMessage(begin_edit_settings = true) } + EditSettingsSession(destNum).block() + commandSender.sendAdmin(destNum) { AdminMessage(commit_edit_settings = true) } } - override suspend fun commitEditSettings(destNum: Int) { - commandSender.sendAdmin(destNum) { AdminMessage(commit_edit_settings = true) } + /** Binds the [AdminEditScope] operations to a fixed destination, delegating to the controller's set* methods. */ + private inner class EditSettingsSession(private val destNum: Int) : AdminEditScope { + override suspend fun setOwner(user: User) = setOwner(destNum, user, getPacketId()) + + override suspend fun setConfig(config: Config) = setConfig(destNum, config, getPacketId()) + + override suspend fun setModuleConfig(config: ModuleConfig) = setModuleConfig(destNum, config, getPacketId()) + + override suspend fun setChannel(channel: Channel) = setRemoteChannel(destNum, channel, getPacketId()) + + override suspend fun setFixedPosition(position: Position) = + this@DirectRadioControllerImpl.setFixedPosition(destNum, position) } // ── Telemetry & Discovery ─────────────────────────────────────────────── 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 5fa459f0b..2880ed556 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 @@ -20,6 +20,7 @@ import kotlinx.coroutines.flow.StateFlow import org.meshtastic.core.model.ConnectionState import org.meshtastic.core.model.DataPacket import org.meshtastic.core.model.Position +import org.meshtastic.core.repository.AdminEditScope import org.meshtastic.core.repository.RadioController import org.meshtastic.proto.Channel import org.meshtastic.proto.ClientNotification @@ -47,8 +48,7 @@ class FakeRadioController : val sentSharedContacts = mutableListOf() var throwOnSend: Boolean = false var lastSetDeviceAddress: String? = null - var beginEditSettingsCalled = false - var commitEditSettingsCalled = false + var editSettingsCalled = false var startProvideLocationCalled = false var stopProvideLocationCalled = false @@ -59,8 +59,7 @@ class FakeRadioController : sentSharedContacts.clear() throwOnSend = false lastSetDeviceAddress = null - beginEditSettingsCalled = false - commitEditSettingsCalled = false + editSettingsCalled = false startProvideLocationCalled = false stopProvideLocationCalled = false } @@ -150,12 +149,23 @@ class FakeRadioController : override suspend fun requestNeighborInfo(requestId: Int, destNum: Int) {} - override suspend fun beginEditSettings(destNum: Int) { - beginEditSettingsCalled = true - } + override suspend fun editSettings(destNum: Int, block: suspend AdminEditScope.() -> Unit) { + editSettingsCalled = true + val scope = + object : AdminEditScope { + override suspend fun setOwner(user: User) = setOwner(destNum, user, getPacketId()) - override suspend fun commitEditSettings(destNum: Int) { - commitEditSettingsCalled = true + override suspend fun setConfig(config: Config) = setConfig(destNum, config, getPacketId()) + + override suspend fun setModuleConfig(config: ModuleConfig) = + setModuleConfig(destNum, config, getPacketId()) + + override suspend fun setChannel(channel: Channel) = setRemoteChannel(destNum, channel, getPacketId()) + + override suspend fun setFixedPosition(position: Position) = + this@FakeRadioController.setFixedPosition(destNum, position) + } + scope.block() } override fun getPacketId(): Int = 1