From 3364895a9f26e34e7081791fdb721f11f376b1fc Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Sat, 7 Nov 2026 04:30:51 -0500 Subject: [PATCH] feat(#523): display mode, status LED and rotation over 0xC5 0x1A/0x1B Lands the #523 indicator controls, separated from the factory reset (#1057) that happened to share their branch. #1057 is destructive, untested and the owner did not ask for it, so it stays behind rather than riding along. Rotation is 0xC5 0x1A GET / 0x1B SET, gated on TAG_DISPLAY_ROTATION in the 0x1C advert, with no caps-bit fallback and no probing. It is NOT 0x18/0x19: those are per-channel channel notify, and an earlier build that put rotation there corrupted silently, because the firmware's 0x19 handler validates a channel-notify mode and nothing else. The hazard commit 89f1768e is not in this history at all; this is the post-fix state only. Display mode and the status LED read device-info bytes 86 and 85. Nothing reads byte 87, which contract section 2 retired permanently. Provenance, since the route matters: cherry-picked ea09d2f3 with -m 1. That commit is the merge of the #523 work into the #1095 advert work, and the rotation fix lives in its merge resolution rather than in a commit of its own, so its first-parent diff is exactly what #523 adds on top of what dev already carries. The result is 956 insertions across 28 files, matching ea09d2f3's own stat, and offband_indicators.dart, indicators_view.dart and indicators_connector_test.dart are byte-identical to the old branch tip. Adversarial review (gemini-2.5-pro) found a crash, fixed here: the rotation control force-unwrapped displayRotation, but the advert tag that ENABLES the control arrives before the rotation GET reply that supplies the value, so the pane threw on any slow link. It now takes an empty selection, matching the display-mode control beside it, which was already null-safe. Proved by probe: restoring the unwrap yields "Null check operator used on a null value" and turns 2 of the new widget tests red. The review's second finding, a read reply overwriting a newer confirmed write, is real but deliberately NOT fixed here. The obvious guard is to drop read replies while a write is outstanding, and _sendIndicatorSet sets that flag with no timeout and no cleanup, unlike _awaitDeviceUi which clears it in a finally. That guard would wedge rotation reads for the rest of the session after any SET that went unanswered, which is worse than the stale value it prevents. Giving _sendIndicatorSet the same timeout discipline is the real fix and is follow-up work, not something to change minutes before a cut. flutter analyze clean, 1945 tests pass and 1 skipped, including 25 indicators connector tests and 4 new widget tests. Not hardware-tested. Co-Authored-By: Claude Opus 5 --- lib/connector/meshcore_connector.dart | 143 ++++++- lib/connector/offband_device_ui.dart | 8 +- lib/connector/offband_indicators.dart | 160 ++++++++ lib/l10n/app_en.arb | 2 + lib/l10n/app_localizations.dart | 12 + lib/l10n/app_localizations_bg.dart | 7 + lib/l10n/app_localizations_de.dart | 7 + lib/l10n/app_localizations_en.dart | 7 + lib/l10n/app_localizations_es.dart | 7 + lib/l10n/app_localizations_fr.dart | 7 + lib/l10n/app_localizations_hu.dart | 7 + lib/l10n/app_localizations_it.dart | 7 + lib/l10n/app_localizations_ja.dart | 7 + lib/l10n/app_localizations_ko.dart | 7 + lib/l10n/app_localizations_nl.dart | 7 + lib/l10n/app_localizations_pl.dart | 7 + lib/l10n/app_localizations_pt.dart | 7 + lib/l10n/app_localizations_ru.dart | 7 + lib/l10n/app_localizations_sk.dart | 7 + lib/l10n/app_localizations_sl.dart | 7 + lib/l10n/app_localizations_sv.dart | 7 + lib/l10n/app_localizations_uk.dart | 7 + lib/l10n/app_localizations_zh.dart | 7 + lib/screens/settings/device_ui_view.dart | 12 +- lib/screens/settings/indicators_view.dart | 104 ++++++ lib/screens/settings_screen.dart | 17 + test/connector/indicators_connector_test.dart | 353 ++++++++++++++++++ test/widgets/indicators_view_test.dart | 95 +++++ untranslated.json | 34 ++ 29 files changed, 1058 insertions(+), 8 deletions(-) create mode 100644 lib/connector/offband_indicators.dart create mode 100644 lib/screens/settings/indicators_view.dart create mode 100644 test/connector/indicators_connector_test.dart create mode 100644 test/widgets/indicators_view_test.dart diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 48427c75..d55b43cd 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -69,6 +69,7 @@ import '../storage/channel_region_scope_store.dart'; import 'offband_capability_advert.dart'; import 'offband_channel_notify.dart'; import 'offband_device_ui.dart'; +import 'offband_indicators.dart'; import 'offband_screen_settings.dart'; import 'offband_tones.dart'; import 'caplog_reassembler.dart'; @@ -463,6 +464,9 @@ class MeshCoreConnector extends ChangeNotifier { final Map _eventTones = {}; final Map _tuneSlots = {}; ScreenSettingsState? _screenSettings; + bool? _ledEnabled; + DisplayMode? _displayMode; + int? _displayRotation; bool _tuneSlotTransferActive = false; final List<_DeviceUiWaiter> _deviceUiWaiters = []; int _pathHashByteWidth = 1; @@ -1114,6 +1118,23 @@ class MeshCoreConnector extends ChangeNotifier { bool get supportsScreenSettings => firmwareSupportsScreenSettings(_offbandCaps2); + /// Whether this radio has a settable display or LED (#523, caps byte 2 + /// `0x04`). Which of the two it has, it says only by refusing the other. + bool get supportsIndicators => firmwareSupportsIndicators(_offbandCaps2); + + /// The LED and display state the radio last reported, from device-info or a + /// reply. Null when it has not said. + bool? get ledEnabled => _ledEnabled; + DisplayMode? get displayMode => _displayMode; + + /// Degrees, or null until the radio answers the rotation GET (`0x1A`). + /// + /// Null is "not read yet", NOT "cannot rotate". Capability is + /// [supportsDisplayRotation], which reads the advert tag; conflating the two + /// is what the old `supportsIndicators && _displayRotation != null` did, and + /// it made an unanswered read indistinguishable from an unsupported board. + int? get displayRotation => _displayRotation; + /// The tune the radio reported for [channel], or null if not read yet. ChannelTune? channelTuneFor(int channel) => _channelTunes[channel]; @@ -1420,6 +1441,9 @@ class MeshCoreConnector extends ChangeNotifier { /// calling it before the list is synced is a no-op rather than a guess. void _reconcileAdvertisedDeviceUi() { unawaited(_readChannelNotifyModes()); + // Rotation's gate is TAG_DISPLAY_ROTATION, so this is the first point at + // which it can be asked for at all (#1098). + if (supportsDisplayRotation) unawaited(requestDisplayRotation()); } /// Ask the radio which notify mode [channel] has for its own buzzer (#544). @@ -1817,6 +1841,66 @@ class MeshCoreConnector extends ChangeNotifier { Future setBatteryFull({required bool pin}) => _sendScreenSet('batt SET', buildBatteryFullSetFrame(pin: pin)); + Future _sendIndicatorSet( + String what, + DeviceUiSurface surface, + Uint8List frame, + ) async { + if (!_deviceUiGateOpen(what, advertised: supportsIndicators)) return; + _deviceUiError = null; + _deviceUiErrorSurface = null; + _pendingDeviceUiWrite = surface; + _appDebugLogService?.info('$what -> $frame', tag: 'DeviceUI'); + await sendFrame(frame); + } + + Future setDisplayMode(DisplayMode mode) => _sendIndicatorSet( + 'display SET', + DeviceUiSurface.display, + buildDisplaySetFrame(mode), + ); + + /// Refused locally until the radio has answered the rotation GET, so + /// firmware that cannot rotate is never sent a SET. + Future setDisplayRotation(int degrees) async { + if (!supportsDisplayRotation) { + _setDeviceUiError( + DeviceUiSurface.rotation, + 'This radio has not reported a rotation it can change', + ); + return; + } + await _sendIndicatorSet( + 'rotation SET', + DeviceUiSurface.rotation, + buildRotationSetFrame(degrees), + ); + } + + /// Read this radio's current rotation. + /// + /// This is a plain GET, not a capability probe. Rotation support comes from + /// `TAG_DISPLAY_ROTATION` in the `0x1C` advert, so the old probe (send + /// `0x18`, treat a refusal as "cannot rotate") is gone: contract §3 forbids + /// "discovering support by trying each feature's sub-code and watching for + /// errors", and the probe also had to borrow `_pendingDeviceUiWrite` for a + /// read, which meant a genuine write refusal could be eaten by it. + Future requestDisplayRotation() async { + if (!_deviceUiGateOpen( + 'rotation GET', + advertised: supportsDisplayRotation, + )) { + return; + } + await sendFrame(buildRotationGetFrame()); + } + + Future setLedEnabled(bool on) => _sendIndicatorSet( + 'led SET', + DeviceUiSurface.led, + buildLedSetFrame(on: on), + ); + /// Re-read whatever headless-UI state this radio supports. Called after every /// device-info reply so a scope changed by triple-press on the device is /// never displayed stale, and so frame-arrival order does not matter. (#475) @@ -1855,6 +1939,8 @@ class MeshCoreConnector extends ChangeNotifier { if (supportsGpsMode) requestGpsMode(); // Node Settings names the battery full point in its Battery row. if (supportsScreenSettings) requestScreenSettings(); + // Rotation is NOT driven from here: its gate is an advert tag, which is + // not known yet at this point. It is driven from the advert reply instead. } /// Single entry point for `0xC5`: both surfaces and the shared error sub-code @@ -1907,6 +1993,11 @@ class MeshCoreConnector extends ChangeNotifier { _handleScreenSettingsReply(screen); return; } + final indicator = parseIndicatorReply(frame); + if (indicator != null) { + _handleIndicatorReply(indicator); + return; + } _appDebugLogService?.warn( '0xC5 reply not understood (sub=0x' '${frame.length > 1 ? frame[1].toRadixString(16).padLeft(2, '0') : '??'}, ' @@ -1948,6 +2039,9 @@ class MeshCoreConnector extends ChangeNotifier { DeviceUiSurface.tuneSlot => 'Tune slot', DeviceUiSurface.screenSettings => 'Screen settings', DeviceUiSurface.channelNotify => 'Radio channel notifications', + DeviceUiSurface.display => 'Display', + DeviceUiSurface.led => 'LED', + DeviceUiSurface.rotation => 'Rotation', null => 'Device UI request', }; _appDebugLogService?.warn( @@ -2040,6 +2134,26 @@ class MeshCoreConnector extends ChangeNotifier { notifyListeners(); } + void _handleIndicatorReply(IndicatorReply reply) { + final surface = reply.surface; + if (reply.isWrite && _pendingDeviceUiWrite == surface) { + _pendingDeviceUiWrite = null; + } + if (reply.errorMessage != null) { + _setDeviceUiError(surface, reply.errorMessage!); + return; + } + if (reply.displayMode != null) _displayMode = reply.displayMode; + if (reply.rotation != null) _displayRotation = reply.rotation; + if (reply.ledOn != null) _ledEnabled = reply.ledOn; + _appDebugLogService?.info( + 'indicator reply: display=${_displayMode?.name} ' + 'rotation=$_displayRotation led=$_ledEnabled', + tag: 'DeviceUI', + ); + notifyListeners(); + } + void _handleChannelTuneReply(OffbandUiReply reply) { final t = reply.channelTune; if (t == null) { @@ -4919,6 +5033,9 @@ class MeshCoreConnector extends ChangeNotifier { _advertRequestOutstanding = false; _advertContinuationRound = 0; _advertTags.clear(); + _ledEnabled = null; + _displayMode = null; + _displayRotation = null; _blockListRetryTimer?.cancel(); // Per-connection debounce timers must die with the connection so a timer // scheduled against this radio can't fire against the next one (#838 review). @@ -8645,6 +8762,21 @@ class MeshCoreConnector extends ChangeNotifier { ? frame[4] | (frame[5] << 8) | (frame[6] << 16) | (frame[7] << 24) : null; + /// LED (byte 85) and display mode (byte 86), appended unconditionally on + /// v21+ (firmware #542). Each is null when the frame stops short of it. Like + /// the FEM byte, presence says nothing about capability: + /// [supportsIndicators] does. + /// + /// Bytes 82 to 86 keep working and are explicitly untouched by the advert + /// contract (§7). Nothing here reads 87, and nothing may: rotation is read + /// over `0x1A`/`0x1B` and advertised by `TAG_DISPLAY_ROTATION`. + static ({bool? ledOn, DisplayMode? mode}) parseIndicatorBytes( + Uint8List frame, + ) => ( + ledOn: frame.length >= 86 ? frame[85] != 0 : null, + mode: frame.length >= 87 ? DisplayMode.fromCode(frame[86]) : null, + ); + void _handleDeviceInfo(Uint8List frame) { if (frame.length < 4) return; if (_shouldGateInitialChannelSync) { @@ -8714,12 +8846,19 @@ class MeshCoreConnector extends ChangeNotifier { _femLnaEnabled = parseFemLnaState(frame); // Second caps byte rides at offset 84, past the FEM state byte (#480). _offbandCaps2 = parseOffbandCaps2(frame); - // Nothing is read past byte 84 here. Offset 87 is where three features - // collided; capabilities beyond caps2 live in the 0x1C advert (#1096). + // LED (85) and display mode (86) ride past caps2 (#523). Bytes 82 to 86 + // keep working; offset 87 is where three features collided and is read + // by nothing. Capabilities beyond caps2 live in the 0x1C advert (#1096). + final indicators = parseIndicatorBytes(frame); + _ledEnabled = indicators.ledOn; + _displayMode = indicators.mode; } else { _offbandCaps = null; _femLnaEnabled = null; _offbandCaps2 = null; + _ledEnabled = null; + _displayMode = null; + _displayRotation = null; } // Capability-gated features are invisible when a bit is clear, which looks // identical to a bug. Log the raw inputs so "the toggle didn't appear" can diff --git a/lib/connector/offband_device_ui.dart b/lib/connector/offband_device_ui.dart index 2e889d7d..6cf78f84 100644 --- a/lib/connector/offband_device_ui.dart +++ b/lib/connector/offband_device_ui.dart @@ -101,7 +101,8 @@ enum ButtonConfigError { 0x09, 'Not charged yet. Charge the radio fully, then set 100% again. Nothing ' 'was changed.', - ); + ), + noRotation(0x0A, "This radio's display cannot be rotated"); const ButtonConfigError(this.code, this.message); final int code; @@ -134,6 +135,11 @@ enum DeviceUiSurface { /// The radio's own per-channel notify mode (#544). Distinct from the app's /// per-channel notify setting, which is not a device-UI surface at all. channelNotify, + display, + led, + + /// Display rotation over `0xC5` `0x1A`/`0x1B` (#1098, firmware #1990). + rotation, } /// The refusal reason for a GPS write. Reason 1 means Power save on a build diff --git a/lib/connector/offband_indicators.dart b/lib/connector/offband_indicators.dart new file mode 100644 index 00000000..8b7ed3ad --- /dev/null +++ b/lib/connector/offband_indicators.dart @@ -0,0 +1,160 @@ +import 'dart:typed_data'; + +import 'offband_device_ui.dart'; + +/// The radio's display and LED over `0xC5` (#523, #944): display mode, display +/// rotation and the status LED. Gated on caps byte 2 `0x04` INDICATORS +/// (meshcore-firmware #542, rotation #1990). +/// +/// display GET `[0xC5][0x05]` -> `[0xC5][0x05][mode]` +/// SET `[0xC5][0x06][mode]` -> the same +/// led GET `[0xC5][0x07]` -> `[0xC5][0x07][on]` +/// SET `[0xC5][0x08][on]` -> the same +/// rotation GET `[0xC5][0x1A]` -> `[0xC5][0x1A][deg]` +/// SET `[0xC5][0x1B][deg]` -> the same +/// +/// ROTATION IS `0x1A`/`0x1B`, NOT `0x18`/`0x19`. Those are CHNOTIFY, and this +/// file held them until 2026-11-04. The collision was not theoretical: +/// firmware `4e5a6c85`, already flashed on the owner's T1000-E, answers +/// `0x18`/`0x19` as per-channel channel-notify and its SET validates only +/// `mode < 4`, so a rotation SET of `0` landed as a valid notify-mode write on +/// channel slot 0, which is Public and undeletable, and BOTH sides reported +/// success. No error, no banner, nothing in any log. +/// +/// The numbers here are transcribed from firmware +/// `examples/companion_radio/OffbandConfigProtocol.h` on `firmware-base` at +/// `28785ebf` (PR OffbandMesh/meshcore-firmware#2000). The header is the +/// registry. Do not take a sub-code from an Agent Mail message or from this +/// comment: read the header, because taking numbers from mail is what caused +/// the collision. +/// +/// Rotation is advertised by `TAG_DISPLAY_ROTATION` in the `0xC5` `0x1C` +/// advert, NOT by caps2 `0x04` INDICATORS and NOT by probing `0x1A` and +/// watching for a refusal. Contract §3 forbids that probing outright: +/// "what is forbidden is discovering support by trying each feature's sub-code +/// and watching for errors." Display mode and the LED keep the INDICATORS bit. +/// +/// A board without a display, or without a controllable LED, refuses that +/// half with reason 6; a display that cannot turn refuses rotation with 10. + +const int offbandCap2Indicators = 0x04; + +const int offbandUiDisplayGet = 0x05; +const int offbandUiDisplaySet = 0x06; +const int offbandUiLedGet = 0x07; +const int offbandUiLedSet = 0x08; +const int offbandUiRotationGet = 0x1A; +const int offbandUiRotationSet = 0x1B; + +/// The rotations the radio accepts, in degrees. +const List displayRotations = [0, 180]; + +bool firmwareSupportsIndicators(int? offbandCaps2) => + offbandCaps2 != null && (offbandCaps2 & offbandCap2Indicators) != 0; + +/// When the radio's screen is lit. Values are the wire encoding. +enum DisplayMode { + auto(0, 'Auto', 'On while in use, then off after the screen timeout'), + alwaysOn(1, 'Always on', 'Never turns off by itself'), + alwaysOff(2, 'Always off', 'Stays dark'); + + const DisplayMode(this.code, this.label, this.description); + final int code; + final String label; + final String description; + + static DisplayMode? fromCode(int code) { + for (final m in DisplayMode.values) { + if (m.code == code) return m; + } + return null; + } +} + +Uint8List buildDisplayGetFrame() => + Uint8List.fromList([cmdOffbandDeviceUi, offbandUiDisplayGet]); + +Uint8List buildDisplaySetFrame(DisplayMode mode) => + Uint8List.fromList([cmdOffbandDeviceUi, offbandUiDisplaySet, mode.code]); + +Uint8List buildLedGetFrame() => + Uint8List.fromList([cmdOffbandDeviceUi, offbandUiLedGet]); + +Uint8List buildLedSetFrame({required bool on}) => + Uint8List.fromList([cmdOffbandDeviceUi, offbandUiLedSet, on ? 1 : 0]); + +Uint8List buildRotationGetFrame() => + Uint8List.fromList([cmdOffbandDeviceUi, offbandUiRotationGet]); + +Uint8List buildRotationSetFrame(int degrees) { + if (!displayRotations.contains(degrees)) { + throw ArgumentError.value(degrees, 'degrees', 'must be 0 or 180'); + } + return Uint8List.fromList([ + cmdOffbandDeviceUi, + offbandUiRotationSet, + degrees, + ]); +} + +/// Parse a display, LED or rotation reply. Null if this is not one, or if it +/// is truncated. The shared 0x7F error is owned by [parseButtonMatrixReply]. +IndicatorReply? parseIndicatorReply(Uint8List frame) { + if (frame.length < 3 || frame[0] != respCodeOffbandDeviceUi) return null; + final sub = frame[1]; + final value = frame[2]; + switch (sub) { + case offbandUiLedGet || offbandUiLedSet: + return IndicatorReply(sub: sub, ledOn: value != 0); + case offbandUiDisplayGet || offbandUiDisplaySet: + final mode = DisplayMode.fromCode(value); + if (mode == null) { + return IndicatorReply( + sub: sub, + errorMessage: + 'The radio reported an unknown display mode (${_hex(value)})', + ); + } + return IndicatorReply(sub: sub, displayMode: mode); + case offbandUiRotationGet || offbandUiRotationSet: + if (!displayRotations.contains(value)) { + return IndicatorReply( + sub: sub, + errorMessage: 'The radio reported an unknown rotation ($value)', + ); + } + return IndicatorReply(sub: sub, rotation: value); + } + return null; +} + +String _hex(int b) => '0x${b.toRadixString(16).padLeft(2, '0')}'; + +class IndicatorReply { + const IndicatorReply({ + required this.sub, + this.displayMode, + this.ledOn, + this.rotation, + this.errorMessage, + }); + + final int sub; + final DisplayMode? displayMode; + final bool? ledOn; + + /// Degrees, always one of [displayRotations]. + final int? rotation; + final String? errorMessage; + + DeviceUiSurface get surface => switch (sub) { + offbandUiLedGet || offbandUiLedSet => DeviceUiSurface.led, + offbandUiRotationGet || offbandUiRotationSet => DeviceUiSurface.rotation, + _ => DeviceUiSurface.display, + }; + + bool get isWrite => + sub == offbandUiDisplaySet || + sub == offbandUiLedSet || + sub == offbandUiRotationSet; +} diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index b2aaab79..c9584788 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -385,6 +385,8 @@ "settings_buttonActionsSubtitle": "Assign what the button does", "settings_badgeScreen": "Screen", "settings_badgeScreenSubtitle": "Text size, screen off, time zone", + "settings_indicators": "Display and LED", + "settings_indicatorsSubtitle": "Display mode, rotation, status LED", "settings_badgeBattery": "Battery", "settings_badgeBatterySubtitle": "100% at {mv} mV", "@settings_badgeBatterySubtitle": { diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index a4ccf137..ef8aa1f4 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -1240,6 +1240,18 @@ abstract class AppLocalizations { /// **'Text size, screen off, time zone'** String get settings_badgeScreenSubtitle; + /// No description provided for @settings_indicators. + /// + /// In en, this message translates to: + /// **'Display and LED'** + String get settings_indicators; + + /// No description provided for @settings_indicatorsSubtitle. + /// + /// In en, this message translates to: + /// **'Display mode, rotation, status LED'** + String get settings_indicatorsSubtitle; + /// No description provided for @settings_badgeBattery. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_bg.dart b/lib/l10n/app_localizations_bg.dart index 206874ed..90863dd3 100644 --- a/lib/l10n/app_localizations_bg.dart +++ b/lib/l10n/app_localizations_bg.dart @@ -668,6 +668,13 @@ class AppLocalizationsBg extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index 64218ad7..4783561f 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -670,6 +670,13 @@ class AppLocalizationsDe extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index 387377af..55cc9ca7 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -664,6 +664,13 @@ class AppLocalizationsEn extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_es.dart b/lib/l10n/app_localizations_es.dart index 584c5e74..755a8e4c 100644 --- a/lib/l10n/app_localizations_es.dart +++ b/lib/l10n/app_localizations_es.dart @@ -668,6 +668,13 @@ class AppLocalizationsEs extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_fr.dart b/lib/l10n/app_localizations_fr.dart index 32b275bd..554ef261 100644 --- a/lib/l10n/app_localizations_fr.dart +++ b/lib/l10n/app_localizations_fr.dart @@ -671,6 +671,13 @@ class AppLocalizationsFr extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_hu.dart b/lib/l10n/app_localizations_hu.dart index 101bc5a3..75dce93e 100644 --- a/lib/l10n/app_localizations_hu.dart +++ b/lib/l10n/app_localizations_hu.dart @@ -670,6 +670,13 @@ class AppLocalizationsHu extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_it.dart b/lib/l10n/app_localizations_it.dart index e5b46ef0..c009c111 100644 --- a/lib/l10n/app_localizations_it.dart +++ b/lib/l10n/app_localizations_it.dart @@ -670,6 +670,13 @@ class AppLocalizationsIt extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_ja.dart b/lib/l10n/app_localizations_ja.dart index 383272af..afb53252 100644 --- a/lib/l10n/app_localizations_ja.dart +++ b/lib/l10n/app_localizations_ja.dart @@ -653,6 +653,13 @@ class AppLocalizationsJa extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_ko.dart b/lib/l10n/app_localizations_ko.dart index c964a07a..bc4cdf39 100644 --- a/lib/l10n/app_localizations_ko.dart +++ b/lib/l10n/app_localizations_ko.dart @@ -653,6 +653,13 @@ class AppLocalizationsKo extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_nl.dart b/lib/l10n/app_localizations_nl.dart index f479ee8c..5b0d9b3b 100644 --- a/lib/l10n/app_localizations_nl.dart +++ b/lib/l10n/app_localizations_nl.dart @@ -667,6 +667,13 @@ class AppLocalizationsNl extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_pl.dart b/lib/l10n/app_localizations_pl.dart index cb65cec0..c209edc4 100644 --- a/lib/l10n/app_localizations_pl.dart +++ b/lib/l10n/app_localizations_pl.dart @@ -673,6 +673,13 @@ class AppLocalizationsPl extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_pt.dart b/lib/l10n/app_localizations_pt.dart index 67645255..f63eb395 100644 --- a/lib/l10n/app_localizations_pt.dart +++ b/lib/l10n/app_localizations_pt.dart @@ -670,6 +670,13 @@ class AppLocalizationsPt extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_ru.dart b/lib/l10n/app_localizations_ru.dart index f9ee91d4..f6f00478 100644 --- a/lib/l10n/app_localizations_ru.dart +++ b/lib/l10n/app_localizations_ru.dart @@ -669,6 +669,13 @@ class AppLocalizationsRu extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_sk.dart b/lib/l10n/app_localizations_sk.dart index c32adf04..076a7d9f 100644 --- a/lib/l10n/app_localizations_sk.dart +++ b/lib/l10n/app_localizations_sk.dart @@ -668,6 +668,13 @@ class AppLocalizationsSk extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_sl.dart b/lib/l10n/app_localizations_sl.dart index 2bb668c3..dd594b0f 100644 --- a/lib/l10n/app_localizations_sl.dart +++ b/lib/l10n/app_localizations_sl.dart @@ -666,6 +666,13 @@ class AppLocalizationsSl extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_sv.dart b/lib/l10n/app_localizations_sv.dart index 69a8eb6f..c803fb43 100644 --- a/lib/l10n/app_localizations_sv.dart +++ b/lib/l10n/app_localizations_sv.dart @@ -665,6 +665,13 @@ class AppLocalizationsSv extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_uk.dart b/lib/l10n/app_localizations_uk.dart index 440e8a0b..aeade4c7 100644 --- a/lib/l10n/app_localizations_uk.dart +++ b/lib/l10n/app_localizations_uk.dart @@ -667,6 +667,13 @@ class AppLocalizationsUk extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/l10n/app_localizations_zh.dart b/lib/l10n/app_localizations_zh.dart index f2d13c01..b5f19995 100644 --- a/lib/l10n/app_localizations_zh.dart +++ b/lib/l10n/app_localizations_zh.dart @@ -649,6 +649,13 @@ class AppLocalizationsZh extends AppLocalizations { @override String get settings_badgeScreenSubtitle => 'Text size, screen off, time zone'; + @override + String get settings_indicators => 'Display and LED'; + + @override + String get settings_indicatorsSubtitle => + 'Display mode, rotation, status LED'; + @override String get settings_badgeBattery => 'Battery'; diff --git a/lib/screens/settings/device_ui_view.dart b/lib/screens/settings/device_ui_view.dart index 9497bc9a..736c5eea 100644 --- a/lib/screens/settings/device_ui_view.dart +++ b/lib/screens/settings/device_ui_view.dart @@ -65,13 +65,15 @@ class _DeviceUiViewState extends State { DeviceUiSurface.eventTone || DeviceUiSurface.tunePlay || DeviceUiSurface.tuneSlot => widget.part == DeviceUiPart.buzzer, - DeviceUiSurface.gpsMode || DeviceUiSurface.screenSettings => false, // The radio's per-channel notify control lives beside the app's own // per-channel control on the Channels screen, not in this pane (#544 - // Epic C), so a refusal for it is not this screen's to show. Epic C1 - // surfaces it there; until then nothing on this pane can emit it, because - // nothing here writes it. - DeviceUiSurface.channelNotify => false, + // Epic C), so a refusal for it is not this screen's to show. + DeviceUiSurface.gpsMode || + DeviceUiSurface.screenSettings || + DeviceUiSurface.channelNotify || + DeviceUiSurface.display || + DeviceUiSurface.led || + DeviceUiSurface.rotation => false, }; @override diff --git a/lib/screens/settings/indicators_view.dart b/lib/screens/settings/indicators_view.dart new file mode 100644 index 00000000..e6d6a607 --- /dev/null +++ b/lib/screens/settings/indicators_view.dart @@ -0,0 +1,104 @@ +import 'package:flutter/material.dart'; +import 'package:provider/provider.dart'; + +import '../../connector/meshcore_connector.dart'; +import '../../connector/offband_device_ui.dart'; +import '../../connector/offband_indicators.dart'; +import '../../widgets/device_ui_error_banner.dart'; + +/// The radio's display and LED (#523, #944): display mode, rotation and the +/// status LED. Gated on caps byte 2 `0x04`; the Node Settings row that leads +/// here is gated the same way. +/// +/// Every control draws from what the radio reported, never from the tap. A +/// board without a display or without a controllable LED refuses that write +/// and says so in the banner. English-only, matching [DeviceUiView]. +class IndicatorsView extends StatelessWidget { + const IndicatorsView({super.key}); + + @override + Widget build(BuildContext context) { + final connector = context.watch(); + final error = connector.deviceUiError; + final errorSurface = connector.deviceUiErrorSurface; + final mode = connector.displayMode; + final led = connector.ledEnabled; + return ListView( + children: [ + if (error != null && + (errorSurface == DeviceUiSurface.display || + errorSurface == DeviceUiSurface.led || + errorSurface == DeviceUiSurface.rotation)) + DeviceUiErrorBanner( + message: error, + onDismiss: connector.clearDeviceUiError, + ), + const _Header('Display'), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: SegmentedButton( + segments: [ + for (final m in DisplayMode.values) + ButtonSegment(value: m, label: Text(m.label)), + ], + selected: {?mode}, + emptySelectionAllowed: true, + onSelectionChanged: (v) { + if (v.isNotEmpty) connector.setDisplayMode(v.first); + }, + ), + ), + Padding( + padding: const EdgeInsets.fromLTRB(16, 8, 16, 0), + child: Text( + mode?.description ?? 'The radio has not reported its display mode', + style: Theme.of(context).textTheme.bodySmall, + ), + ), + if (connector.supportsDisplayRotation) ...[ + const _Header('Rotation'), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: SegmentedButton( + segments: const [ + ButtonSegment(value: 0, label: Text('Normal')), + ButtonSegment(value: 180, label: Text('Flipped')), + ], + // Null until the radio answers the rotation GET, and the + // capability tag arrives BEFORE that reply, so on any slow link + // this builds with no value at all. Empty selection, never a + // force unwrap, matching the display-mode control above. + selected: {?connector.displayRotation}, + emptySelectionAllowed: true, + onSelectionChanged: (v) { + if (v.isNotEmpty) connector.setDisplayRotation(v.first); + }, + ), + ), + ], + const _Header('LED'), + SwitchListTile( + title: const Text('Status LED'), + subtitle: Text( + led == null + ? 'The radio has not reported its LED' + : 'Turn off to save battery or to stay unseen', + ), + value: led ?? false, + onChanged: connector.setLedEnabled, + ), + ], + ); + } +} + +class _Header extends StatelessWidget { + const _Header(this.title); + final String title; + + @override + Widget build(BuildContext context) => Padding( + padding: const EdgeInsets.fromLTRB(16, 16, 16, 8), + child: Text(title, style: Theme.of(context).textTheme.titleMedium), + ); +} diff --git a/lib/screens/settings_screen.dart b/lib/screens/settings_screen.dart index 198605e5..7528c7c8 100644 --- a/lib/screens/settings_screen.dart +++ b/lib/screens/settings_screen.dart @@ -28,6 +28,7 @@ import 'settings/observer_settings_view.dart'; import 'settings/blocked_view.dart'; import 'settings/badge_battery_view.dart'; import 'settings/badge_screen_view.dart'; +import 'settings/indicators_view.dart'; import 'settings/device_ui_view.dart'; import 'settings/location_view.dart'; import '../helpers/device_name_validator.dart'; @@ -496,6 +497,22 @@ class _SettingsScreenState extends State { ), ), ], + // Display mode, rotation and LED (#523). Strictly on the 0x04 + // bit: no bit, no row, no command. + if (connector.supportsIndicators) ...[ + const Divider(height: 1), + ListTile( + leading: const Icon(Icons.brightness_6_outlined), + title: Text(l10n.settings_indicators), + subtitle: Text(l10n.settings_indicatorsSubtitle), + trailing: const Icon(Icons.chevron_right), + onTap: () => _pushBadgePage( + context, + l10n.settings_indicators, + const IndicatorsView(), + ), + ), + ], if (connector.selfPublicKey != null) ...[ const Divider(height: 1), Padding( diff --git a/test/connector/indicators_connector_test.dart b/test/connector/indicators_connector_test.dart new file mode 100644 index 00000000..2b1109c3 --- /dev/null +++ b/test/connector/indicators_connector_test.dart @@ -0,0 +1,353 @@ +// #523 / #944: display mode, rotation and the status LED over 0xC5. Display and +// LED are 0x05 to 0x08 (#542) under the INDICATORS caps bit. +// +// ROTATION IS 0x1A / 0x1B, advertised by TAG_DISPLAY_ROTATION in the 0xC5 0x1C +// capability advert (#1098, firmware #1990). It was 0x18 / 0x19 under the +// INDICATORS bit until 2026-11-04, and both of those were wrong: +// +// 0x18 / 0x19 are CHNOTIFY. Firmware 4e5a6c85, already flashed on the +// owner's T1000-E, answers them as per-channel notify and its SET validates +// only `mode < 4`, so a rotation SET of 0 landed as a valid notify-mode +// write on channel slot 0, which is Public, with success reported on BOTH +// sides. Silent corruption, no error anywhere. +// +// Discovering rotation by sending the GET and watching for a refusal is +// forbidden by contract section 3: "what is forbidden is discovering support +// by trying each feature's sub-code and watching for errors." The tag +// replaces the probe, so capability and value are no longer conflated. +// +// Pins the gate (the tag, not 0x04 and not a probe), the device-info bytes 85 +// and 86, that offset 87 is read by nothing, and that every value follows the +// radio's reply rather than the tap. + +import 'dart:convert'; +import 'dart:typed_data'; + +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/connector/meshcore_connector.dart'; +import 'package:meshcore_open/connector/offband_capability_advert.dart'; +import 'package:meshcore_open/connector/offband_device_ui.dart'; +import 'package:meshcore_open/connector/offband_indicators.dart'; +import 'package:meshcore_open/storage/drift/blob_store.dart'; +import 'package:meshcore_open/storage/drift/offband_database.dart'; +import 'package:meshcore_open/storage/prefs_manager.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// An Offband device-info reply: caps byte 2 at 84, then LED and display mode. +/// [extra] appends bytes past 86, to prove nothing there is read as rotation. +List deviceInfo({ + int caps2 = offbandCap2Indicators, + int led = 1, + int mode = 0, + List extra = const [], +}) { + final v = ascii.encode('1.5.0-1.17.0'); + final f = Uint8List(87 + extra.length); + f[0] = 13; + f[1] = 22; + f.setRange(60, 60 + v.length, v); + f[84] = caps2; + f[85] = led; + f[86] = mode; + f.setRange(87, 87 + extra.length, extra); + return f; +} + +void main() { + late OffbandDatabase db; + late MeshCoreConnector connector; + late List> sent; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + PrefsManager.reset(); + await PrefsManager.initialize(); + db = OffbandDatabase(NativeDatabase.memory()); + BlobStore.overrideForTest(BlobStore(db)); + sent = []; + connector = MeshCoreConnector(); + connector.sendFrameOverrideForTest = (data) => sent.add(data.toList()); + connector.setConnectedForTest(); + }); + + tearDown(() async { + BlobStore.clearTestOverride(); + await db.close(); + }); + + bool sentSub(int sub) => sent.any( + (f) => f.length >= 2 && f[0] == cmdOffbandDeviceUi && f[1] == sub, + ); + + /// A `0xC5` `0x1C` advert reply carrying [tags] as zero-length records. + List advert(List tags) => [ + cmdOffbandDeviceUi, + offbandUiInfoGet, + for (final t in tags) ...[t, 0x00], + ]; + + /// `[0xC5][0x7F][0x05]`: this radio predates the advert. + List advertUnsupported() => [ + cmdOffbandDeviceUi, + offbandUiErr, + offbandUiErrReasonMalformed, + ]; + + /// Connect a radio that ADVERTISES rotation, then answer the GET with + /// [rotation] degrees. Capability now comes from the tag, and the GET + /// reports the current value; the two are separate steps on purpose. + void connectWithRotation(int rotation, {int mode = 0}) { + connector.handleFrameForTest(deviceInfo(mode: mode)); + connector.handleFrameForTest( + Uint8List.fromList(advert([offbandInfoTagDisplayRotation])), + ); + connector.handleFrameForTest([ + cmdOffbandDeviceUi, + offbandUiRotationGet, + rotation, + ]); + sent.clear(); + } + + group('frames', () { + test('a mode SET is three bytes, with no rotation in it', () { + expect(buildDisplaySetFrame(DisplayMode.alwaysOff), [0xC5, 0x06, 2]); + }); + + test('rotation is 0x1A / 0x1B, and NEVER the CHNOTIFY pair', () { + // The collision, pinned at the wire. 0x18/0x19 against firmware + // 4e5a6c85 writes a channel-notify mode onto Public with a success + // reply on both sides. + expect(buildRotationGetFrame(), [0xC5, 0x1A]); + expect(buildRotationSetFrame(180), [0xC5, 0x1B, 180]); + expect(offbandUiRotationGet, isNot(0x18)); + expect(offbandUiRotationSet, isNot(0x19)); + }); + + test('a rotation other than 0 or 180 is refused before sending', () { + expect(() => buildRotationSetFrame(90), throwsArgumentError); + }); + + test('the LED SET is one byte', () { + expect(buildLedSetFrame(on: false), [0xC5, 0x08, 0]); + }); + + test('a rotation reply carries degrees', () { + final r = parseIndicatorReply(Uint8List.fromList([0xC5, 0x1B, 180])); + expect(r!.rotation, 180); + expect(r.surface, DeviceUiSurface.rotation); + expect(r.isWrite, isTrue); + }); + + test('an unknown mode or rotation is an error, not a guess', () { + final mode = parseIndicatorReply(Uint8List.fromList([0xC5, 0x05, 9])); + expect(mode!.errorMessage, contains('unknown display mode')); + final rot = parseIndicatorReply(Uint8List.fromList([0xC5, 0x1A, 90])); + expect(rot!.errorMessage, contains('unknown rotation')); + }); + + test('the indicator parser does not claim the CHNOTIFY pair', () { + // Belt and braces on the collision: even if a 0x18/0x19 frame reaches + // this parser, it must not be read as a rotation reply. + expect(parseIndicatorReply(Uint8List.fromList([0xC5, 0x18, 0])), isNull); + expect(parseIndicatorReply(Uint8List.fromList([0xC5, 0x19, 0])), isNull); + }); + }); + + test('device-info bytes 85 and 86 render on connect', () { + connector.handleFrameForTest(deviceInfo(led: 0, mode: 2)); + + expect(connector.supportsIndicators, isTrue); + expect(connector.ledEnabled, isFalse); + expect(connector.displayMode, DisplayMode.alwaysOff); + }); + + test('a radio without 0x04 never sees an indicator frame', () async { + connector.handleFrameForTest(deviceInfo(caps2: 0x01)); + await connector.setDisplayMode(DisplayMode.alwaysOn); + await connector.setLedEnabled(false); + + expect(connector.supportsIndicators, isFalse); + expect(sentSub(offbandUiDisplaySet), isFalse); + expect(sentSub(offbandUiLedSet), isFalse); + expect(sentSub(offbandUiRotationGet), isFalse); + }); + + group('rotation', () { + test('the INDICATORS bit alone does NOT open rotation', () { + // The gate moved to the advert tag. 0x04 means indicators, which is a + // different capability; gating rotation on it was the old contract. + connector.handleFrameForTest(deviceInfo()); + + expect(connector.supportsIndicators, isTrue); + expect(connector.supportsDisplayRotation, isFalse); + expect(sentSub(offbandUiRotationGet), isFalse); + }); + + test('rotation is NOT probed, ever', () { + // Contract section 3: "what is forbidden is discovering support by + // trying each feature's sub-code and watching for errors." An advert + // without the tag must produce no 0x1A on the wire at all. + connector.handleFrameForTest(deviceInfo()); + connector.handleFrameForTest( + Uint8List.fromList(advert([offbandInfoTagChanNotify])), + ); + + expect(connector.supportsDisplayRotation, isFalse); + expect(sentSub(offbandUiRotationGet), isFalse); + }); + + test('a radio that predates the advert gets no rotation and no frame', () { + connector.handleFrameForTest(deviceInfo()); + connector.handleFrameForTest(Uint8List.fromList(advertUnsupported())); + + expect(connector.supportsDisplayRotation, isFalse); + expect(sentSub(offbandUiRotationGet), isFalse); + expect(connector.deviceUiError, isNull, reason: 'not a failure to show'); + }); + + test('the tag opens it, and the GET follows', () { + connector.handleFrameForTest(deviceInfo()); + connector.handleFrameForTest( + Uint8List.fromList(advert([offbandInfoTagDisplayRotation])), + ); + + expect(connector.supportsDisplayRotation, isTrue); + expect(sentSub(offbandUiRotationGet), isTrue); + }); + + test( + 'capability and value are separate: the tag does not imply a value', + () { + // The old gate was `supportsIndicators && _displayRotation != null`, + // which made "not answered yet" indistinguishable from "cannot rotate". + connector.handleFrameForTest(deviceInfo()); + connector.handleFrameForTest( + Uint8List.fromList(advert([offbandInfoTagDisplayRotation])), + ); + + expect(connector.supportsDisplayRotation, isTrue); + expect(connector.displayRotation, isNull); + }, + ); + + test('a byte past 86 in device-info is never read as rotation', () { + connector.handleFrameForTest(deviceInfo(extra: [180])); + + expect(connector.supportsDisplayRotation, isFalse); + expect(connector.displayRotation, isNull); + }); + + test('the value arrives from the GET reply', () { + connectWithRotation(180); + + expect(connector.supportsDisplayRotation, isTrue); + expect(connector.displayRotation, 180); + }); + + test('an advert refusal does not swallow the next write refusal', () { + connector.handleFrameForTest(deviceInfo()); + connector.handleFrameForTest(Uint8List.fromList(advertUnsupported())); + connector.handleFrameForTest( + Uint8List.fromList(advert([offbandInfoTagDisplayRotation])), + ); + sent.clear(); + + expect(connector.deviceUiError, isNull); + }); + + test('a rotation write before the tag sends nothing and says so', () async { + connector.handleFrameForTest(deviceInfo()); + sent.clear(); + + await connector.setDisplayRotation(180); + + expect(sent, isEmpty); + expect(connector.deviceUiErrorSurface, DeviceUiSurface.rotation); + expect(connector.deviceUiError, isNotNull); + }); + + test('a rotation write sends 0x1B and follows the echo', () async { + connectWithRotation(0); + + await connector.setDisplayRotation(180); + expect(sent, [buildRotationSetFrame(180)]); + expect(connector.displayRotation, 0); + + connector.handleFrameForTest([0xC5, 0x1B, 180]); + expect(connector.displayRotation, 180); + }); + + test( + 'a rotation SET refused with reason 10 says so on its surface', + () async { + connectWithRotation(0); + + await connector.setDisplayRotation(180); + connector.handleFrameForTest([0xC5, 0x7F, 0x0A]); + + expect(connector.deviceUiErrorSurface, DeviceUiSurface.rotation); + expect( + connector.deviceUiError, + "This radio's display cannot be rotated", + ); + expect(connector.displayRotation, 0); + }, + ); + }); + + test('a mode write never touches rotation', () async { + connectWithRotation(180); + + await connector.setDisplayMode(DisplayMode.alwaysOff); + expect(sent, [buildDisplaySetFrame(DisplayMode.alwaysOff)]); + + connector.handleFrameForTest([0xC5, 0x06, 2]); + expect(connector.displayMode, DisplayMode.alwaysOff); + expect(connector.displayRotation, 180); + }); + + test('state follows the echo, not the request', () async { + connectWithRotation(0); + await connector.setDisplayMode(DisplayMode.alwaysOn); + + expect(connector.displayMode, DisplayMode.auto); + + connector.handleFrameForTest([0xC5, 0x06, 1]); + expect(connector.displayMode, DisplayMode.alwaysOn); + + connector.handleFrameForTest([0xC5, 0x08, 0]); + expect(connector.ledEnabled, isFalse); + }); + + test('a board without a display refuses on the display surface', () async { + connectWithRotation(0); + await connector.setDisplayMode(DisplayMode.alwaysOff); + connector.handleFrameForTest([0xC5, 0x7F, 0x06]); + + expect(connector.deviceUiErrorSurface, DeviceUiSurface.display); + expect(connector.deviceUiError, 'This radio has no display or LED to set'); + expect(connector.displayMode, DisplayMode.auto); + }); + + test('an LED refusal lands on the LED surface', () async { + connectWithRotation(0); + await connector.setLedEnabled(false); + connector.handleFrameForTest([0xC5, 0x7F, 0x06]); + + expect(connector.deviceUiErrorSurface, DeviceUiSurface.led); + expect(connector.ledEnabled, isTrue); + }); + + test('a stock-shape frame reads no indicator bytes', () { + final stock = Uint8List(82); + stock[0] = 13; + stock[1] = 22; + connector.handleFrameForTest(stock); + + expect(connector.supportsIndicators, isFalse); + expect(connector.displayMode, isNull); + expect(connector.ledEnabled, isNull); + }); +} diff --git a/test/widgets/indicators_view_test.dart b/test/widgets/indicators_view_test.dart new file mode 100644 index 00000000..1f8d8815 --- /dev/null +++ b/test/widgets/indicators_view_test.dart @@ -0,0 +1,95 @@ +// Widget tests for the Display and LED pane (#523/#1098). +// +// These pin the crash the adversarial review caught before this was pushed: +// the rotation control force-unwrapped `displayRotation`, but the capability +// tag that ENABLES the control arrives in the `0xC5` `0x1C` advert, which lands +// BEFORE the rotation GET reply that supplies the value. So on any slow link +// the pane was built with the control enabled and the value still null, and it +// threw. The display-mode control beside it was already null-safe; only +// rotation was not. + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; +import 'package:meshcore_open/connector/meshcore_connector.dart'; +import 'package:meshcore_open/screens/settings/indicators_view.dart'; + +/// Rotation is advertised, but the radio has not answered the GET yet. This is +/// the real window between the advert reply and the rotation reply. +class _RotationUnanswered extends MeshCoreConnector { + @override + bool get supportsIndicators => true; + @override + bool get supportsDisplayRotation => true; + @override + int? get displayRotation => null; +} + +class _RotationAnswered extends MeshCoreConnector { + _RotationAnswered(this._degrees); + final int _degrees; + @override + bool get supportsIndicators => true; + @override + bool get supportsDisplayRotation => true; + @override + int? get displayRotation => _degrees; +} + +Future _pump(WidgetTester tester, MeshCoreConnector connector) => + tester.pumpWidget( + ChangeNotifierProvider.value( + value: connector, + child: const MaterialApp(home: Scaffold(body: IndicatorsView())), + ), + ); + +void main() { + testWidgets( + 'the pane builds before the radio has answered the rotation GET', + (tester) async { + await _pump(tester, _RotationUnanswered()); + + // If this ever fails again, something is force-unwrapping a value that is + // null for the whole window between the advert and the GET reply. + expect(tester.takeException(), isNull); + expect(find.text('Rotation'), findsOneWidget); + }, + ); + + testWidgets('no rotation is shown as chosen until the radio answers', ( + tester, + ) async { + await _pump(tester, _RotationUnanswered()); + + final button = tester.widget>( + find.byType(SegmentedButton), + ); + // An empty selection, not a guessed default: showing "Normal" here would + // claim a rotation the radio never reported. + expect(button.selected, isEmpty); + expect(button.emptySelectionAllowed, isTrue); + }); + + testWidgets('the rotation the radio reported is the one shown', ( + tester, + ) async { + await _pump(tester, _RotationAnswered(180)); + + final button = tester.widget>( + find.byType(SegmentedButton), + ); + expect(button.selected, {180}); + }); + + testWidgets('a radio that does not advertise rotation gets no control', ( + tester, + ) async { + await _pump(tester, _DummyConnector()); + + expect(find.text('Rotation'), findsNothing); + expect(find.byType(SegmentedButton), findsNothing); + }); +} + +class _DummyConnector extends MeshCoreConnector {} diff --git a/untranslated.json b/untranslated.json index 6ed34529..8a149493 100644 --- a/untranslated.json +++ b/untranslated.json @@ -72,6 +72,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -440,6 +442,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -808,6 +812,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -1176,6 +1182,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -1544,6 +1552,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -1912,6 +1922,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -2280,6 +2292,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -2648,6 +2662,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -3016,6 +3032,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -3384,6 +3402,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -3752,6 +3772,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -4120,6 +4142,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -4488,6 +4512,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -4856,6 +4882,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -5224,6 +5252,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -5592,6 +5622,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading", @@ -5960,6 +5992,8 @@ "settings_buttonActionsSubtitle", "settings_badgeScreen", "settings_badgeScreenSubtitle", + "settings_indicators", + "settings_indicatorsSubtitle", "settings_badgeBattery", "settings_badgeBatterySubtitle", "settings_badgeBatteryReading",