fix(settings): floor the beacon broadcast-target list at one row (#7010)

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
James RichandClaude Sonnet 5 authored and GitHub committed 2026-09-02 18:20:26 +00:00
1 parent 4e2b74a048
commit ea5fd32991
6 files changed
+325 -25

No files matched your search

+1
View File
@@ -1098,6 +1098,7 @@ mesh_beacon_region_required
mesh_beacon_target
mesh_beacon_target_add
mesh_beacon_target_channel_index
mesh_beacon_target_default
mesh_beacon_target_remove
mesh_beacon_targets
mesh_map_location
@@ -1134,6 +1134,7 @@
<string name="mesh_beacon_target">Target %1$d</string>
<string name="mesh_beacon_target_add">Add target</string>
<string name="mesh_beacon_target_channel_index">Channel</string>
<string name="mesh_beacon_target_default">Default</string>
<string name="mesh_beacon_target_remove">Remove target</string>
<string name="mesh_beacon_targets">Broadcast targets</string>
<string name="mesh_map_location">Mesh Map Location</string>
@@ -57,6 +57,7 @@ import org.meshtastic.core.resources.mesh_beacon_region_required
import org.meshtastic.core.resources.mesh_beacon_target
import org.meshtastic.core.resources.mesh_beacon_target_add
import org.meshtastic.core.resources.mesh_beacon_target_channel_index
import org.meshtastic.core.resources.mesh_beacon_target_default
import org.meshtastic.core.resources.mesh_beacon_target_remove
import org.meshtastic.core.resources.mesh_beacon_targets
import org.meshtastic.core.resources.plurals_seconds
@@ -95,9 +96,13 @@ private fun Int.hasFlag(flag: Int): Boolean = (this and flag) != 0
*
* The region and offered/transmit preset are never user-chosen here: the radio's own LoRa region and configured preset
* are always stamped in on save (`stampBeaconConfigForSave`), so the beacon can never transmit region or preset
* information the radio itself does not use. The repeated `broadcast_targets` list ([BroadcastTargetsCard]) is the only
* way to name extra beacon destinations beyond the offered channel; an empty list sends one beacon on that channel
* alone.
* information the radio itself does not use. `broadcast_offer_*` is the invitation payload content shown to listeners
* (what channel/preset they could join); the repeated `broadcast_targets` list ([BroadcastTargetsCard]) is the
* separate, only, TX destination list -- which radio settings the beacon packet itself is actually transmitted on.
* Firmware sends one beacon on the node's running preset and primary channel when this list is empty
* (`MeshBeaconModule.cpp::sendBeacon`), so the editor seeds and floors the list at one row ([seedBeaconTargets],
* [removeBeaconTarget]) rather than ever showing zero rows -- design#140 behavior 6 keeps that implicit default visible
* and editable instead of hidden.
*/
@Suppress("LongMethod")
@Composable
@@ -130,7 +135,7 @@ fun MeshBeaconConfigScreen(viewModel: RadioConfigViewModel, onBack: () -> Unit,
return
}
val formState = rememberConfigState(initialValue = meshBeaconConfig)
val formState = rememberConfigState(initialValue = initialBeaconFormState(meshBeaconConfig))
val listenFlag = MeshBeaconConfig.Flags.FLAG_LISTEN_ENABLED.value
val broadcastFlag = MeshBeaconConfig.Flags.FLAG_BROADCAST_ENABLED.value
@@ -362,12 +367,14 @@ internal fun OfferChannelPreference(
}
/**
* Editor for the repeated `broadcast_targets` list: extra beacon destinations beyond the offered channel. Each row
* picks one of the radio's own channels ([channelItems]) and a preset filtered by [presetConstraint] (design#140
* behaviors 2 and 7); region is no longer a row concept (behavior 1), the radio's own region applies to every target.
* Editor for the repeated `broadcast_targets` list: the beacon's actual TX destinations (design#140 behavior 6). Each
* row picks one of the radio's own channels ([channelItems]) or the "Default" sentinel, and a preset filtered by
* [presetConstraint] (design#140 behaviors 2 and 7) or "Default"; region is no longer a row concept (behavior 1), the
* radio's own region applies to every target. Internal (not private): unit-testable directly, mirroring
* [OfferChannelPreference].
*/
@Composable
private fun BroadcastTargetsCard(
internal fun BroadcastTargetsCard(
targets: List<MeshBeaconConfig.BroadcastTarget>,
enabled: Boolean,
channelItems: List<DropDownItem<Int>>,
@@ -390,7 +397,7 @@ private fun BroadcastTargetsCard(
presetsGated = presetsGated,
capabilities = capabilities,
onChange = { updated -> onChange(targets.mapIndexed { i, t -> if (i == index) updated(t) else t }) },
onRemove = { onChange(targets.filterIndexed { i, _ -> i != index }) },
onRemove = { onChange(removeBeaconTarget(targets, index)) },
)
}
HorizontalDivider()
@@ -421,18 +428,24 @@ private fun BroadcastTargetRow(
modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp),
style = MaterialTheme.typography.titleSmall,
)
val rowChannelIndex = target.channel_index ?: 0
// Nullable value type throughout (design#140 behavior 6): `null` is the "Default" sentinel, matching the wire
// format where an unset channel_index/preset falls back to the running config (module_config.proto's
// BroadcastTarget). A row is never forced to a resolved value the way it was before this row supported "Default".
val defaultLabel = stringResource(Res.string.mesh_beacon_target_default)
val rowChannelIndex = target.channel_index
val nullableChannelItems: List<DropDownItem<Int?>> =
listOf(DropDownItem<Int?>(value = null, label = defaultLabel)) +
channelItems.map { DropDownItem<Int?>(it.value, it.label, it.icon, it.color, it.enabled, it.testTag) }
val rowChannelItems =
if (channelItems.none { it.value == rowChannelIndex }) {
val fallback =
DropDownItem(
if (rowChannelIndex != null && nullableChannelItems.none { it.value == rowChannelIndex }) {
nullableChannelItems +
DropDownItem<Int?>(
value = rowChannelIndex,
label = stringResource(Res.string.mesh_beacon_channel_number, rowChannelIndex),
enabled = false,
)
channelItems + fallback
} else {
channelItems
nullableChannelItems
}
DropDownPreference(
title = stringResource(Res.string.mesh_beacon_target_channel_index),
@@ -441,17 +454,22 @@ private fun BroadcastTargetRow(
enabled = enabled,
onItemSelected = { channelIndex -> onChange { selectBeaconTargetChannel(it, channelIndex, currentPreset) } },
)
val selectedPreset = target.preset ?: presetConstraint.defaultPreset
val rowPreset = target.preset
val presetItems =
remember(presetConstraint, presetsGated, selectedPreset, capabilities) {
buildPresetItems(presetConstraint, presetsGated, selectedPreset, capabilities)
remember(presetConstraint, presetsGated, rowPreset, capabilities) {
buildPresetItems(presetConstraint, presetsGated, rowPreset ?: presetConstraint.defaultPreset, capabilities)
}
val nullablePresetItems: List<DropDownItem<ModemPreset?>> =
listOf(DropDownItem<ModemPreset?>(value = null, label = defaultLabel)) +
presetItems.map {
DropDownItem<ModemPreset?>(it.value, it.label, it.icon, it.color, it.enabled, it.testTag)
}
val presetSummary = if (presetsGated) stringResource(Res.string.config_lora_modem_preset_licensed_summary) else null
DropDownPreference(
title = stringResource(Res.string.mesh_beacon_on_preset),
summary = presetSummary,
items = presetItems,
selectedItem = selectedPreset,
items = nullablePresetItems,
selectedItem = rowPreset,
enabled = enabled,
onItemSelected = { sel -> onChange { it.copy(preset = sel) } },
)
@@ -121,15 +121,50 @@ internal fun stampBeaconConfigForSave(
}
/**
* Applies a channel pick to one broadcast target row (design#140 behavior 7): the channel index is always set, and the
* radio's currently-configured preset is preselected only the first time the row gets a channel, never overwriting a
* preset the user already chose.
* Applies a channel pick to one broadcast target row (design#140 behavior 7): the radio's currently-configured preset
* is preselected only the first time the row gets a concrete channel, never overwriting a preset the user already
* chose. Picking the "Default" sentinel (`channelIndex = null`, design#140 behavior 6) leaves any existing preset
* untouched rather than preselecting -- there is no channel to have "just been picked".
*/
internal fun selectBeaconTargetChannel(
target: MeshBeaconConfig.BroadcastTarget,
channelIndex: Int,
channelIndex: Int?,
currentPreset: ModemPreset,
): MeshBeaconConfig.BroadcastTarget = target.copy(channel_index = channelIndex, preset = target.preset ?: currentPreset)
): MeshBeaconConfig.BroadcastTarget = target.copy(
channel_index = channelIndex,
preset = if (channelIndex != null) target.preset ?: currentPreset else target.preset,
)
/**
* Seeds an empty stored `broadcast_targets` list with one default row (design#140 behavior 6: "one row saves as the
* single broadcast config, added rows save as broadcast targets"). Firmware's own fallback for an empty list is a
* single beacon on the node's running preset and region over the primary channel (`MeshBeaconModule.cpp::sendBeacon`);
* seeding a row here makes that default visible and editable in the editor instead of leaving it implicit. A non-empty
* stored list is returned unchanged.
*/
internal fun seedBeaconTargets(stored: List<MeshBeaconConfig.BroadcastTarget>): List<MeshBeaconConfig.BroadcastTarget> =
stored.ifEmpty { listOf(MeshBeaconConfig.BroadcastTarget()) }
/**
* [MeshBeaconConfigScreen]'s actual `formState` initial value: [seedBeaconTargets] applied to the loaded config's
* `broadcast_targets`, everything else untouched. Pulled out as its own function (rather than inlined at the
* `rememberConfigState` call site) so a test can call the exact production entry point instead of separately calling
* [seedBeaconTargets] and only asserting against it in isolation.
*/
internal fun initialBeaconFormState(loaded: MeshBeaconConfig): MeshBeaconConfig =
loaded.copy(broadcast_targets = seedBeaconTargets(loaded.broadcast_targets))
/**
* Removes the target row at [index], keeping the list at a floor of one row (design#140 behavior 6: "no UI that changes
* shape once targets exist" -- the list must never render with zero rows, since zero rows would hide firmware's
* implicit single-target fallback rather than represent "no beacon"). Removing the only remaining row replaces it with
* a fresh default row instead of emptying the list, mirroring the reference iOS editor.
*/
internal fun removeBeaconTarget(
targets: List<MeshBeaconConfig.BroadcastTarget>,
index: Int,
): List<MeshBeaconConfig.BroadcastTarget> =
targets.filterIndexed { i, _ -> i != index }.ifEmpty { listOf(MeshBeaconConfig.BroadcastTarget()) }
/** The three broadcast-half gating decisions design#140 Q1 hangs off `radioLora.use_preset` and the STORED flag. */
internal data class BeaconBroadcastGate(
@@ -215,6 +215,24 @@ class MeshBeaconConfigPolicyTest {
assertEquals(ModemPreset.SHORT_FAST, stamped.broadcast_targets[1].preset)
}
@Test
fun stampBeaconConfigForSave_defaultTargetRow_nullFieldsSurviveButRegionIsStamped() {
// A seeded/"Default" row (null channel_index, null preset) must reach the outgoing ModuleConfig with those
// fields still null -- that's the whole point of the sentinel, matching firmware's own "unset falls back to
// running config" semantics (module_config.proto). Region is the one field every target always gets, per
// save-time stamping (behavior 1), regardless of the row's own null fields.
val radioLora =
Config.LoRaConfig(region = RegionCode.EU_868, modem_preset = ModemPreset.MEDIUM_FAST, use_preset = true)
val config = MeshBeaconConfig(broadcast_targets = listOf(MeshBeaconConfig.BroadcastTarget()))
val stamped = stampBeaconConfigForSave(config, config, radioLora, channelList = emptyList())
val target = stamped.broadcast_targets.single()
assertNull(target.channel_index)
assertNull(target.preset)
assertEquals(RegionCode.EU_868, target.region)
}
@Test
fun stampBeaconConfigForSave_untouchedOfferChannel_defaultsToPrimary() {
val radioLora =
@@ -326,4 +344,87 @@ class MeshBeaconConfigPolicyTest {
assertEquals(3, updated.channel_index)
assertEquals(ModemPreset.LONG_MODERATE, updated.preset)
}
@Test
fun selectBeaconTargetChannel_defaultSentinel_clearsChannelAndLeavesPresetUntouched() {
val target = MeshBeaconConfig.BroadcastTarget(channel_index = 2, preset = null)
val updated = selectBeaconTargetChannel(target, channelIndex = null, currentPreset = ModemPreset.SHORT_FAST)
assertNull(updated.channel_index)
// Picking "Default" is not "picking a channel" (design#140 behavior 7 only fires on a concrete channel).
assertNull(updated.preset)
}
@Test
fun selectBeaconTargetChannel_defaultSentinelWithConcretePreset_presetUnchanged() {
// Same as the null-preset case above, but with a preset the user has already deliberately chosen -- the
// regression this guards is "Default" silently resetting a concrete preset, not just leaving null alone.
val target = MeshBeaconConfig.BroadcastTarget(channel_index = 2, preset = ModemPreset.SHORT_FAST)
val updated = selectBeaconTargetChannel(target, channelIndex = null, currentPreset = ModemPreset.LONG_FAST)
assertNull(updated.channel_index)
assertEquals(ModemPreset.SHORT_FAST, updated.preset)
}
@Test
fun seedBeaconTargets_emptyStoredList_seedsOneDefaultRow() {
val seeded = seedBeaconTargets(emptyList())
assertEquals(listOf(MeshBeaconConfig.BroadcastTarget()), seeded)
}
@Test
fun seedBeaconTargets_nonEmptyStoredList_isUnchanged() {
val stored = listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 1, preset = ModemPreset.LONG_FAST))
val seeded = seedBeaconTargets(stored)
assertEquals(stored, seeded)
}
@Test
fun initialBeaconFormState_emptyStoredConfig_seedsFormTargetsThroughTheProductionPath() {
// Calls the exact function MeshBeaconConfigScreen calls to build formState's initial value -- not
// seedBeaconTargets directly -- so this breaks if the screen's wiring to it ever comes apart.
val loaded = MeshBeaconConfig(broadcast_message = "hi", broadcast_targets = emptyList())
val initial = initialBeaconFormState(loaded)
assertEquals(listOf(MeshBeaconConfig.BroadcastTarget()), initial.broadcast_targets)
assertEquals("hi", initial.broadcast_message)
}
@Test
fun initialBeaconFormState_nonEmptyStoredConfig_isUnchanged() {
val stored = listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 3))
val loaded = MeshBeaconConfig(broadcast_targets = stored)
val initial = initialBeaconFormState(loaded)
assertEquals(stored, initial.broadcast_targets)
}
@Test
fun removeBeaconTarget_removingOneOfSeveral_dropsOnlyThatRow() {
val targets =
listOf(
MeshBeaconConfig.BroadcastTarget(channel_index = 0),
MeshBeaconConfig.BroadcastTarget(channel_index = 1),
)
val updated = removeBeaconTarget(targets, index = 0)
assertEquals(listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 1)), updated)
}
@Test
fun removeBeaconTarget_removingTheOnlyRow_reseedsADefaultRowInstead() {
val targets = listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 4, preset = ModemPreset.SHORT_FAST))
val updated = removeBeaconTarget(targets, index = 0)
assertEquals(listOf(MeshBeaconConfig.BroadcastTarget()), updated)
}
}
@@ -19,18 +19,28 @@ package org.meshtastic.feature.settings.radio.component
import androidx.compose.foundation.text.KeyboardActions
import androidx.compose.foundation.text.KeyboardOptions
import androidx.compose.material3.Text
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsEnabled
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.onAllNodesWithText
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.v2.runComposeUiTest
import org.meshtastic.core.model.Capabilities
import org.meshtastic.core.model.RegionPresetConstraint
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.getString
import org.meshtastic.core.resources.mesh_beacon_broadcast_requires_preset
import org.meshtastic.core.resources.mesh_beacon_no_channels
import org.meshtastic.core.resources.mesh_beacon_region_required
import org.meshtastic.core.resources.mesh_beacon_target
import org.meshtastic.core.resources.mesh_beacon_target_default
import org.meshtastic.core.resources.mesh_beacon_target_remove
import org.meshtastic.core.resources.save_changes
import org.meshtastic.core.ui.component.DropDownItem
import org.meshtastic.core.ui.component.EditTextPreference
@@ -45,6 +55,7 @@ import org.meshtastic.proto.Config.LoRaConfig.RegionCode
import org.meshtastic.proto.ModuleConfig.MeshBeaconConfig
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
/**
* Mirrors [LoRaBandwidthUiTest]'s shape: composes [RadioConfigScreenList] directly with a hand-built [ConfigState],
@@ -329,4 +340,137 @@ class MeshBeaconConfigUiTest {
runOnIdle { assertEquals(listenFlag, savedConfig?.flags) }
}
@Test
fun emptyStoredConfig_productionSeedPathRendersExactlyOneRow() = runComposeUiTest {
// Goes through initialBeaconFormState -- the exact function MeshBeaconConfigScreen calls to build formState's
// initial value -- rather than calling seedBeaconTargets itself, so this proves the screen's own wiring
// rather than just the policy function in isolation.
val presetConstraint =
RegionPresetConstraint(presets = listOf(ModemPreset.LONG_FAST), ModemPreset.LONG_FAST, false)
val seededTargets = initialBeaconFormState(MeshBeaconConfig()).broadcast_targets
setContent {
AppTheme {
BroadcastTargetsCard(
targets = seededTargets,
enabled = true,
channelItems = listOf(DropDownItem(0, "Primary")),
currentPreset = ModemPreset.LONG_FAST,
presetConstraint = presetConstraint,
presetsGated = false,
capabilities = Capabilities(firmwareVersion = null),
onChange = {},
)
}
}
onNodeWithText(getString(Res.string.mesh_beacon_target, 1)).assertIsDisplayed()
onAllNodesWithText(getString(Res.string.mesh_beacon_target_remove)).assertCountEquals(1)
}
@Test
fun removingTheLastTargetRow_replacesItWithAGenuineDefaultRow() = runComposeUiTest {
val presetConstraint =
RegionPresetConstraint(presets = listOf(ModemPreset.LONG_FAST), ModemPreset.LONG_FAST, false)
lateinit var targetsState: MutableState<List<MeshBeaconConfig.BroadcastTarget>>
setContent {
AppTheme {
targetsState = remember { mutableStateOf(seedBeaconTargets(emptyList())) }
BroadcastTargetsCard(
targets = targetsState.value,
enabled = true,
channelItems = listOf(DropDownItem(0, "Primary")),
currentPreset = ModemPreset.LONG_FAST,
presetConstraint = presetConstraint,
presetsGated = false,
capabilities = Capabilities(firmwareVersion = null),
onChange = { targetsState.value = it },
)
}
}
onNodeWithText(getString(Res.string.mesh_beacon_target_remove)).performClick()
// Not just "one row, one remove button" -- that would also pass for a wrong non-empty result. The replaced
// row must be a genuine fresh default: both fields null, not carried over from the removed row.
runOnIdle { assertEquals(listOf(MeshBeaconConfig.BroadcastTarget()), targetsState.value) }
onNodeWithText(getString(Res.string.mesh_beacon_target, 1)).assertIsDisplayed()
onAllNodesWithText(getString(Res.string.mesh_beacon_target_remove)).assertCountEquals(1)
}
@Test
fun broadcastTargetRow_selectingDefaultChannel_clearsChannelIndexAndLeavesPresetUntouched() = runComposeUiTest {
val presetConstraint =
RegionPresetConstraint(presets = listOf(ModemPreset.LONG_FAST), ModemPreset.LONG_FAST, false)
lateinit var targetsState: MutableState<List<MeshBeaconConfig.BroadcastTarget>>
setContent {
AppTheme {
targetsState = remember {
mutableStateOf(
listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 0, preset = ModemPreset.LONG_FAST)),
)
}
BroadcastTargetsCard(
targets = targetsState.value,
enabled = true,
channelItems = listOf(DropDownItem(0, "Primary")),
currentPreset = ModemPreset.LONG_FAST,
presetConstraint = presetConstraint,
presetsGated = false,
capabilities = Capabilities(firmwareVersion = null),
onChange = { targetsState.value = it },
)
}
}
// Opens the channel picker (currently showing the concrete channel "Primary") and picks "Default".
onNodeWithText("Primary").performClick()
onNodeWithText(getString(Res.string.mesh_beacon_target_default)).performClick()
runOnIdle {
assertNull(targetsState.value.single().channel_index)
assertEquals(ModemPreset.LONG_FAST, targetsState.value.single().preset)
}
onNodeWithText(getString(Res.string.mesh_beacon_target_default)).assertIsDisplayed()
}
@Test
fun broadcastTargetRow_selectingDefaultPreset_clearsPresetAndLeavesChannelUntouched() = runComposeUiTest {
val presetConstraint =
RegionPresetConstraint(presets = listOf(ModemPreset.LONG_FAST), ModemPreset.LONG_FAST, false)
lateinit var targetsState: MutableState<List<MeshBeaconConfig.BroadcastTarget>>
setContent {
AppTheme {
targetsState = remember {
mutableStateOf(
listOf(MeshBeaconConfig.BroadcastTarget(channel_index = 0, preset = ModemPreset.LONG_FAST)),
)
}
BroadcastTargetsCard(
targets = targetsState.value,
enabled = true,
channelItems = listOf(DropDownItem(0, "Primary")),
currentPreset = ModemPreset.LONG_FAST,
presetConstraint = presetConstraint,
presetsGated = false,
capabilities = Capabilities(firmwareVersion = null),
onChange = { targetsState.value = it },
)
}
}
// Opens the preset picker (currently showing the concrete preset "LONG_FAST") and picks "Default".
onNodeWithText(ModemPreset.LONG_FAST.name).performClick()
onNodeWithText(getString(Res.string.mesh_beacon_target_default)).performClick()
runOnIdle {
assertNull(targetsState.value.single().preset)
assertEquals(0, targetsState.value.single().channel_index)
}
onNodeWithText(getString(Res.string.mesh_beacon_target_default)).assertIsDisplayed()
}
}