mirror of
https://github.com/meshtastic/Meshtastic-Android.git
synced 2026-06-02 03:01:28 -04:00
refactor: Replace begin/commitEditSettings with editSettings DSL
The transactional config-write API exposed raw beginEditSettings /
commitEditSettings pairs, so callers had to remember to commit and could
leak a half-open session. Replace with a scoped builder mirroring the
SDK's AdminApi.editSettings { }:
radioController.editSettings(destNum) {
setOwner(user); setConfig(config); setModuleConfig(...)
}
The new AdminEditScope binds operations to the session's destination, so
InstallProfileUseCase no longer threads destNum + getPacketId() through
every call — its install* helpers become AdminEditScope extensions,
dropping a large amount of boilerplate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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)) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 ───────────────────────────────────────────────
|
||||
|
||||
@@ -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<Int>()
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user