mirror/Meshtastic-Android
1
0
Fork 0
mirror of https://github.com/meshtastic/Meshtastic-Android.git synced 2026-06-13 08:25:07 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: address review triage — transition-based ham routing, local-only guard, callsign guidance

Browse Source 
- Route to set_ham_mode only when the licensed toggle transitions
  OFF→ON; subsequent saves of an already-licensed node use set_owner so
  other owner edits still propagate and the node doesn't reboot on
  every save (F-1)
- Enforce the local-node-only contract in AdminControllerImpl, not just
  the ViewModel: setHamMode ignores remote destinations (F-3)
- Show 'Your amateur radio call sign, up to 8 characters' as the
  field's supporting text in ham mode, giving sighted and screen-reader
  users the constraint (F-4, F-6)
- Document that saveUserConfig is the preferred entry point over
  setOwner (F-7)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
James Rich
2026-06-12 09:19:59 -05:00
parent 29111bd355
commit 6d4ea50ca3
8 changed files with 58 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.skills/compose-ui/strings-index.txt generated
View File

@@ -123,6 +123,7 @@ button_gpio
buzzer_gpio
calculating
call_sign
call_sign_summary
cancel
cancel_reply
canned_message

12
core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminController.kt
View File

@@ -56,12 +56,12 @@ interface AdminController {
/**
* Enables amateur-radio (ham) mode on a node via `AdminMessage.set_ham_mode`.
*
* Must target only the locally connected node — firmware ham onboarding is a local operation and the UI gates it
* accordingly. The firmware handler rewrites the owner (long_name = call_sign), flips `is_licensed`, disables
* encryption, applies [HamParameters.tx_power]/[HamParameters.frequency] to the LoRa config verbatim, and reboots.
* The implementation echoes the local node's current LoRa values into those two fields so a re-send never wipes the
* node's overrides; caller-supplied [HamParameters.tx_power]/[HamParameters.frequency] are ignored. Intentionally
* absent from [AdminEditScope]: ham enablement is not a batch-edit operation.
* Must target only the locally connected node — firmware ham onboarding is a local operation; the implementation
* ignores requests for any other node. The firmware handler rewrites the owner (long_name = call_sign), flips
* `is_licensed`, disables encryption, applies [HamParameters.tx_power]/[HamParameters.frequency] to the LoRa config
* verbatim, and reboots. The implementation echoes the local node's current LoRa values into those two fields so a
* re-send never wipes the node's overrides; caller-supplied [HamParameters.tx_power]/[HamParameters.frequency] are
* ignored. Intentionally absent from [AdminEditScope]: ham enablement is not a batch-edit operation.
*/
suspend fun setHamMode(destNum: Int, hamParameters: HamParameters, packetId: Int)

1
core/resources/src/commonMain/composeResources/values/strings.xml
View File

@@ -141,6 +141,7 @@
<string name="buzzer_gpio">Buzzer GPIO</string>
<string name="calculating">Calculating…</string>
<string name="call_sign">Call sign</string>
<string name="call_sign_summary">Your amateur radio call sign, up to 8 characters</string>
<string name="cancel">Cancel</string>
<string name="cancel_reply">Cancel reply</string>
<string name="canned_message">Canned Message</string>

4
core/service/src/commonMain/kotlin/org/meshtastic/core/service/AdminControllerImpl.kt
View File

@@ -68,6 +68,10 @@ internal class AdminControllerImpl(
}
override suspend fun setHamMode(destNum: Int, hamParameters: HamParameters, packetId: Int) {
if (destNum != nodeManager.myNodeNum.value) {
Logger.w { "Ignoring setHamMode for node $destNum — ham onboarding targets the local node only" }
return
}
// Firmware applies tx_power/frequency to the LoRa config verbatim, so echo the node's current
// values to keep a re-send (e.g. a callsign edit while already licensed) from wiping overrides.
val lora = radioConfigRepository.localConfigFlow.firstOrNull()?.lora ?: Config.LoRaConfig()

14
core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
View File

@@ -363,7 +363,7 @@ class RadioControllerImplTest {
@Test
fun setHamModeSendsAdminWithEchoedLoraValuesAndUpdatesUser() = runTest {
val controller = createController()
val controller = createController(myNodeNum = 123)
val existingUser = User(id = "!0000007b", long_name = "Old Name", short_name = "OLD")
every { nodeManager.nodeDBbyNodeNum } returns mapOf(123 to Node(num = 123, user = existingUser))
every { radioConfigRepository.localConfigFlow } returns
@@ -396,7 +396,7 @@ class RadioControllerImplTest {
@Test
fun setHamModeWithNoCachedLoraConfigSendsProtoDefaults() = runTest {
val controller = createController()
val controller = createController(myNodeNum = 123)
every { nodeManager.nodeDBbyNodeNum } returns emptyMap()
every { radioConfigRepository.localConfigFlow } returns MutableStateFlow(LocalConfig())
@@ -423,6 +423,16 @@ class RadioControllerImplTest {
}
}
@Test
fun setHamModeIgnoresRemoteDestinations() = runTest {
val controller = createController(myNodeNum = 123)
controller.setHamMode(456, HamParameters(call_sign = "KK7ABC", short_name = "KK7A"), 42)
verifySuspend(exactly(0)) { commandSender.sendAdmin(any(), any(), any(), any()) }
verify(exactly(0)) { nodeManager.handleReceivedUser(any(), any(), any(), any()) }
}
@Test
fun importContactReturnsEarlyWhenDisconnected() = runTest {
val controller = createController(myNodeNum = null)

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

@@ -292,13 +292,16 @@ open class RadioConfigViewModel(
}
/**
* Routes the User config save: ham onboarding (`set_ham_mode`) when the licensed toggle is on and the target is the
* locally connected node, [setOwner] otherwise. The local-node guard is the backstop for the UI gate —
* `set_ham_mode` must never be sent to a remote node.
* Routes the User config save: ham onboarding (`set_ham_mode`) when the licensed toggle transitions OFF→ON on the
* locally connected node, [setOwner] otherwise. Routing on the transition — not the toggle state — keeps subsequent
* saves of an already-licensed node on the `set_owner` path, so edits to other owner fields still reach the device
* and the node doesn't reboot on every save (firmware reboots on `set_ham_mode`). The local-node guard is the
* backstop for the UI gate — `set_ham_mode` must never be sent to a remote node.
*/
fun saveUserConfig(user: User) {
val destNum = destNum ?: destNode.value?.num ?: return
if (user.is_licensed && destNum == myNodeNum) setHamMode(destNum, user) else setOwner(user)
val enablingHam = user.is_licensed && !radioConfigState.value.userConfig.is_licensed
if (enablingHam && destNum == myNodeNum) setHamMode(destNum, user) else setOwner(user)
}
private fun setHamMode(destNum: Int, user: User) {
@@ -315,6 +318,10 @@ open class RadioConfigViewModel(
}
}
/**
* Sends a plain `set_owner` with [user]. Prefer [saveUserConfig] for User config screen saves — it routes ham
* onboarding to `set_ham_mode` when the licensed toggle is first enabled; calling this directly bypasses that.
*/
fun setOwner(user: User) {
val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "setOwner") {

2
feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/UserConfigItemList.kt
View File

@@ -32,6 +32,7 @@ import org.meshtastic.core.model.Capabilities
import org.meshtastic.core.model.isUnmessageableRole
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.call_sign
import org.meshtastic.core.resources.call_sign_summary
import org.meshtastic.core.resources.hardware_model
import org.meshtastic.core.resources.licensed_amateur_radio
import org.meshtastic.core.resources.licensed_amateur_radio_text
@@ -87,6 +88,7 @@ fun UserConfigScreen(viewModel: RadioConfigViewModel, onBack: () -> Unit) {
EditTextPreference(
title = stringResource(if (hamMode) Res.string.call_sign else Res.string.long_name),
value = formState.value.long_name,
summary = if (hamMode) stringResource(Res.string.call_sign_summary) else null,
maxSize = longNameMax,
enabled = state.connected,
isError = !validLongName,

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

@@ -421,6 +421,27 @@ class RadioConfigViewModelTest {
verifySuspend(exactly(0)) { radioConfigUseCase.setHamMode(any(), any()) }
}
@Test
fun `saveUserConfig routes subsequent licensed saves to setOwner`() = runTest {
val node = Node(num = 123, user = User(id = "!123"))
nodeRepository.setNodes(listOf(node))
nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123))
viewModel = createViewModel()
val user = User(long_name = "KK7ABC", short_name = "KK7A", is_licensed = true)
everySuspend { radioConfigUseCase.setHamMode(any(), any()) } returns 42
everySuspend { radioConfigUseCase.setOwner(any(), any()) } returns 43
// First save transitions OFF→ON and onboards via set_ham_mode.
viewModel.saveUserConfig(user)
// A later save while already licensed must use set_owner so other owner fields propagate.
val edited = user.copy(short_name = "KK7B")
viewModel.saveUserConfig(edited)
verifySuspend(exactly(1)) { radioConfigUseCase.setHamMode(any(), any()) }
verifySuspend { radioConfigUseCase.setOwner(123, edited) }
}
@Test
fun `saveUserConfig routes licensed save to setOwner when myNodeInfo is absent`() = runTest {
val node = Node(num = 123, user = User(id = "!123"))