Skip to content

Commit 08885b7

Browse files
jamesarichCopilot
jamesarich
and
Copilot
committed
fix(discovery): address design standards audit findings
- Use locale-aware DateFormatter.formatDateTimeShort() instead of hardcoded YYYY-MM-DD HH:mm format in history screen - Filter deprecated presets (VERY_LONG_SLOW, LONG_SLOW) from picker - Add minimum packet threshold (5) for traffic mix classification to avoid noise from trivial counts - Add LITE_FAST, LITE_SLOW, NARROW_FAST, NARROW_SLOW entries to LoRaPresetReference for 2.4 GHz preset AI prompt enrichment - Add lowTrafficCountsNoMixNote test case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8556fcc commit 08885b7

5 files changed

Lines changed: 93 additions & 51 deletions

File tree

‎feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoverySummaryGenerator.kt‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class DiscoverySummaryGenerator {
6565
append(" (congested)")
6666
}
6767
}
68-
if (result.messageCount > 0 || result.sensorPacketCount > 0) {
68+
if (result.messageCount + result.sensorPacketCount >= TRAFFIC_MIN_PACKET_THRESHOLD) {
6969
val dominant = if (result.messageCount >= result.sensorPacketCount) "chat" else "sensor"
7070
append(", $dominant-dominated traffic")
7171
}
@@ -159,8 +159,10 @@ class DiscoverySummaryGenerator {
159159
}
160160

161161
private fun buildTrafficMixNote(results: List<DiscoveryPresetResultEntity>): String? {
162-
val chatDominant = results.filter { it.messageCount > it.sensorPacketCount }
163-
val sensorDominant = results.filter { it.sensorPacketCount > it.messageCount }
162+
val significantResults =
163+
results.filter { it.messageCount + it.sensorPacketCount >= TRAFFIC_MIN_PACKET_THRESHOLD }
164+
val chatDominant = significantResults.filter { it.messageCount > it.sensorPacketCount }
165+
val sensorDominant = significantResults.filter { it.sensorPacketCount > it.messageCount }
164166
val parts = buildList {
165167
if (chatDominant.isNotEmpty()) {
166168
add("chat-dominated on ${chatDominant.joinToString { it.presetName }}")
@@ -193,5 +195,6 @@ class DiscoverySummaryGenerator {
193195
private const val HIGH_UTIL_THRESHOLD = 75.0
194196
private const val HIGH_CONGESTION_THRESHOLD = 25.0
195197
private const val PERCENT_MULTIPLIER = 100.0
198+
private const val TRAFFIC_MIN_PACKET_THRESHOLD = 5
196199
}
197200
}

‎feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ai/LoRaPresetReference.kt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,38 @@ internal object LoRaPresetReference {
104104
"140dB",
105105
"Maximum speed, minimum range. Only for very dense, close-proximity deployments.",
106106
),
107+
"Lite Fast" to
108+
PresetInfo(
109+
"500kHz",
110+
"SF9",
111+
"7.03kbps",
112+
"148dB",
113+
"2.4 GHz band. Fast with moderate range; requires SX1280 hardware.",
114+
),
115+
"Lite Slow" to
116+
PresetInfo(
117+
"250kHz",
118+
"SF11",
119+
"1.07kbps",
120+
"153dB",
121+
"2.4 GHz band. Longer range at lower speed; requires SX1280 hardware.",
122+
),
123+
"Narrow Fast" to
124+
PresetInfo(
125+
"125kHz",
126+
"SF7",
127+
"5.47kbps",
128+
"146dB",
129+
"2.4 GHz band. Narrow bandwidth, fast speed; requires SX1280 hardware.",
130+
),
131+
"Narrow Slow" to
132+
PresetInfo(
133+
"125kHz",
134+
"SF11",
135+
"0.54kbps",
136+
"155.5dB",
137+
"2.4 GHz band. Narrow bandwidth, max range; requires SX1280 hardware.",
138+
),
107139
)
108140

109141
/** Get reference data for a preset, matching by substring (e.g. "Long Fast" matches "Long Fast"). */

‎feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/DiscoveryHistoryScreen.kt‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,8 @@ import androidx.compose.ui.semantics.contentDescription
5151
import androidx.compose.ui.semantics.semantics
5252
import androidx.compose.ui.unit.dp
5353
import androidx.lifecycle.compose.collectAsStateWithLifecycle
54-
import kotlinx.datetime.TimeZone
55-
import kotlinx.datetime.toLocalDateTime
5654
import org.jetbrains.compose.resources.stringResource
57-
import org.meshtastic.core.common.util.toInstant
55+
import org.meshtastic.core.common.util.DateFormatter
5856
import org.meshtastic.core.database.entity.DiscoverySessionEntity
5957
import org.meshtastic.core.resources.Res
6058
import org.meshtastic.core.resources.back
@@ -226,10 +224,4 @@ private fun DeleteConfirmationDialog(onConfirm: () -> Unit, onDismiss: () -> Uni
226224
}
227225

228226
@Suppress("MagicNumber")
229-
internal fun formatTimestamp(epochMillis: Long): String {
230-
val instant = epochMillis.toInstant()
231-
val local = instant.toLocalDateTime(TimeZone.currentSystemDefault())
232-
return "${local.year}-${local.monthNumber.toString().padStart(2, '0')}-" +
233-
"${local.dayOfMonth.toString().padStart(2, '0')} " +
234-
"${local.hour.toString().padStart(2, '0')}:${local.minute.toString().padStart(2, '0')}"
235-
}
227+
internal fun formatTimestamp(epochMillis: Long): String = DateFormatter.formatDateTimeShort(epochMillis)

‎feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/component/PresetPickerCard.kt‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ private val CARD_PADDING = 16.dp
5454
internal fun ChannelOption.displayName(): String =
5555
name.split("_").joinToString(" ") { word -> word.lowercase().replaceFirstChar { it.uppercase() } }
5656

57+
/** Deprecated modem presets that should not appear in the discovery picker. */
58+
private val DEPRECATED_PRESETS = setOf(ChannelOption.VERY_LONG_SLOW, ChannelOption.LONG_SLOW)
59+
5760
/** A card containing a [FlowRow] of [FilterChip] items for preset selection. */
5861
@OptIn(ExperimentalLayoutApi::class)
5962
@Composable
@@ -82,38 +85,42 @@ fun PresetPickerCard(
8285
verticalArrangement = Arrangement.spacedBy(CHIP_SPACING),
8386
modifier = Modifier.fillMaxWidth(),
8487
) {
85-
ChannelOption.entries.forEach { preset ->
86-
val selected = preset in selectedPresets
87-
val isHome = preset == homePreset
88-
val label =
89-
if (isHome) {
90-
stringResource(Res.string.discovery_preset_home_label, preset.displayName())
91-
} else {
92-
preset.displayName()
93-
}
94-
val selectedDesc = stringResource(Res.string.discovery_stat_selected)
95-
val unselectedDesc = stringResource(Res.string.discovery_stat_unselected)
96-
FilterChip(
97-
selected = selected,
98-
onClick = { onTogglePreset(preset) },
99-
label = { Text(label) },
100-
enabled = enabled,
101-
modifier =
102-
Modifier.semantics { stateDescription = if (selected) selectedDesc else unselectedDesc },
103-
leadingIcon =
104-
if (selected) {
105-
{
106-
Icon(
107-
imageVector = MeshtasticIcons.Check,
108-
contentDescription = null,
109-
modifier = Modifier.size(FilterChipDefaults.IconSize),
110-
)
88+
ChannelOption.entries
89+
.filter { it !in DEPRECATED_PRESETS }
90+
.forEach { preset ->
91+
val selected = preset in selectedPresets
92+
val isHome = preset == homePreset
93+
val label =
94+
if (isHome) {
95+
stringResource(Res.string.discovery_preset_home_label, preset.displayName())
96+
} else {
97+
preset.displayName()
11198
}
112-
} else {
113-
null
114-
},
115-
)
116-
}
99+
val selectedDesc = stringResource(Res.string.discovery_stat_selected)
100+
val unselectedDesc = stringResource(Res.string.discovery_stat_unselected)
101+
FilterChip(
102+
selected = selected,
103+
onClick = { onTogglePreset(preset) },
104+
label = { Text(label) },
105+
enabled = enabled,
106+
modifier =
107+
Modifier.semantics {
108+
stateDescription = if (selected) selectedDesc else unselectedDesc
109+
},
110+
leadingIcon =
111+
if (selected) {
112+
{
113+
Icon(
114+
imageVector = MeshtasticIcons.Check,
115+
contentDescription = null,
116+
modifier = Modifier.size(FilterChipDefaults.IconSize),
117+
)
118+
}
119+
} else {
120+
null
121+
},
122+
)
123+
}
117124
}
118125
}
119126
}

‎feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoverySummaryGeneratorTest.kt‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ class DiscoverySummaryGeneratorTest {
145145
assertContains(result, "sensor-dominated")
146146
}
147147

148+
@Test
149+
fun lowTrafficCountsNoMixNote() {
150+
val lowTraffic = preset(name = "LongFast", messageCount = 3, sensorPacketCount = 1)
151+
val result = generator.generateSessionSummary(session(), listOf(lowTraffic))
152+
assertFalse(result.contains("dominated"), "Should not classify traffic mix below threshold")
153+
}
154+
148155
@Test
149156
fun equalTrafficMixNoNote() {
150157
val balanced = preset(name = "LongFast", messageCount = 0, sensorPacketCount = 0)
@@ -292,13 +299,14 @@ class DiscoverySummaryGeneratorTest {
292299

293300
@Test
294301
fun presetPromptContainsMetrics() {
295-
val p = preset(
296-
name = "LongFast",
297-
uniqueNodes = 6,
298-
directNeighborCount = 4,
299-
meshNeighborCount = 2,
300-
avgChannelUtilization = 18.0,
301-
)
302+
val p =
303+
preset(
304+
name = "LongFast",
305+
uniqueNodes = 6,
306+
directNeighborCount = 4,
307+
meshNeighborCount = 2,
308+
avgChannelUtilization = 18.0,
309+
)
302310
val result = generator.buildPresetPrompt(p)
303311
assertContains(result, "Nodes: 6")
304312
assertContains(result, "Direct: 4")

0 commit comments

Comments
 (0)