From 67158bcc9dd583ec08363cdf724b142ecbb0764e Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:42:53 -0400 Subject: [PATCH] fix(#732): apply pre-PR review to the preset catalog RadioPreset.frequencyHz returned kHz; renamed frequencyKHz (the unit CMD_SET_RADIO_PARAMS carries). The picker is disabled while a refresh runs, so a pick can't vanish when the refreshed list lands, and the selected-item builder uses an id-to-title map. Epic: #726 Agent: MagentaHawk (session 5a8922d6) Co-Authored-By: Claude Opus 5.5 --- lib/models/radio_preset.dart | 3 ++- lib/screens/settings_screen.dart | 3 ++- lib/widgets/radio_preset_picker.dart | 15 ++++++++++----- test/connector/preset_apply_frames_test.dart | 4 ++-- test/models/radio_preset_test.dart | 2 +- test/services/radio_preset_service_test.dart | 2 +- 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/lib/models/radio_preset.dart b/lib/models/radio_preset.dart index 91f52e74..ee705b04 100644 --- a/lib/models/radio_preset.dart +++ b/lib/models/radio_preset.dart @@ -54,7 +54,8 @@ class RadioPreset { /// A client-repeat (off-grid) frequency rather than a regional preset. final bool offGrid; - int get frequencyHz => (frequencyMHz * 1000).round(); + /// Frequency in kHz, the unit CMD_SET_RADIO_PARAMS carries. + int get frequencyKHz => (frequencyMHz * 1000).round(); /// The firmware's path hash mode for [pathHashBytes], or null to leave it. /// diff --git a/lib/screens/settings_screen.dart b/lib/screens/settings_screen.dart index 83c891a6..46c21ceb 100644 --- a/lib/screens/settings_screen.dart +++ b/lib/screens/settings_screen.dart @@ -2152,7 +2152,8 @@ class _RadioSettingsFormState extends State<_RadioSettingsForm> { _RadioSettingsSnapshot snapshot, { int? frequencyHz, }) { - return (frequencyHz ?? preset.frequencyHz) == snapshot.frequencyHz && + // _RadioSettingsSnapshot.frequencyHz is also kHz, despite its name. + return (frequencyHz ?? preset.frequencyKHz) == snapshot.frequencyHz && preset.bandwidth == snapshot.bandwidth && preset.spreadingFactor == snapshot.spreadingFactor && preset.codingRate == snapshot.codingRate && diff --git a/lib/widgets/radio_preset_picker.dart b/lib/widgets/radio_preset_picker.dart index ec8d7996..642f1e92 100644 --- a/lib/widgets/radio_preset_picker.dart +++ b/lib/widgets/radio_preset_picker.dart @@ -36,6 +36,7 @@ class RadioPresetPicker extends StatelessWidget { final l10n = context.l10n; final theme = Theme.of(context); final selected = _byId(selectedId)?.id; + final titlesById = {for (final p in presets) p.id: p.title}; final items = >[]; String? region; for (final preset in presets) { @@ -99,16 +100,20 @@ class RadioPresetPicker extends StatelessWidget { Align( alignment: AlignmentDirectional.centerStart, child: Text( - _byId(item.value)?.title ?? '', + titlesById[item.value] ?? '', overflow: TextOverflow.ellipsis, ), ), ], items: items, - onChanged: (id) { - final preset = _byId(id); - if (preset != null) onSelected(preset); - }, + // Disabled while a refresh runs, so a pick can't vanish when the + // refreshed list lands. + onChanged: service.refreshing + ? null + : (id) { + final preset = _byId(id); + if (preset != null) onSelected(preset); + }, ), _PresetStatus(service: service), ], diff --git a/test/connector/preset_apply_frames_test.dart b/test/connector/preset_apply_frames_test.dart index 74a9ffb6..a810652d 100644 --- a/test/connector/preset_apply_frames_test.dart +++ b/test/connector/preset_apply_frames_test.dart @@ -36,14 +36,14 @@ void main() { final philly = named('USA - Philly Mesh'); // The form sends MHz * 1000, i.e. kHz; stock stores freq / 1000.0 as MHz. final frame = buildSetRadioParamsFrame( - philly.frequencyHz, + philly.frequencyKHz, philly.bandwidth.hz, philly.spreadingFactor.value, philly.codingRate.value, ); expect(frame, [cmdSetRadioParams, ...le32(919500), ...le32(500000), 10, 5]); // Inside stock's accepted ranges (150000-2500000 kHz, BW <= 500000 Hz). - expect(philly.frequencyHz, inInclusiveRange(150000, 2500000)); + expect(philly.frequencyKHz, inInclusiveRange(150000, 2500000)); expect(philly.bandwidth.hz, lessThanOrEqualTo(500000)); }); diff --git a/test/models/radio_preset_test.dart b/test/models/radio_preset_test.dart index ac315a67..852c6bf4 100644 --- a/test/models/radio_preset_test.dart +++ b/test/models/radio_preset_test.dart @@ -60,7 +60,7 @@ void main() { final socal = r.presets[0]; expect(socal.id, 'meshcore:USA - Southern California'); expect(socal.region, 'USA'); - expect(socal.frequencyHz, 927875); + expect(socal.frequencyKHz, 927875); expect(socal.bandwidth, LoRaBandwidth.bw62_5); expect(socal.spreadingFactor, LoRaSpreadingFactor.sf7); expect(socal.codingRate, LoRaCodingRate.cr4_5); diff --git a/test/services/radio_preset_service_test.dart b/test/services/radio_preset_service_test.dart index a792ab82..b385163c 100644 --- a/test/services/radio_preset_service_test.dart +++ b/test/services/radio_preset_service_test.dart @@ -283,7 +283,7 @@ void main() { final merged = mergeRadioPresets(upstream.presets, overlay.presets); expect(merged.length, 59); final philly = merged.firstWhere((p) => p.title == 'USA - Philly Mesh'); - expect(philly.frequencyHz, 919500); + expect(philly.frequencyKHz, 919500); expect(philly.bandwidth, LoRaBandwidth.bw500); expect(philly.pathHashMode, 1); final euNarrow = merged.firstWhere((p) => p.title == 'EU/UK (Narrow)');