fix(#78): surface partial-refresh failures + validate status_interval (G1 follow-up)

Closes the 2 MINOR findings from the G1 re-review (#76):
- MINOR-A: ObserverConfigService.refresh() no longer wipes lastError / clears
  stale on a PARTIAL read. A null from any getFlat is a failed read; the
  snapshot is marked stale and the error kept (error-visibility).
- MINOR-B: the settings pane validates status_interval against the firmware
  range (10-3600) before sending; an out-of-range value is not put on the wire
  and the user is told.

TDD: both tests RED first (current sent '5'; refresh wiped the error), GREEN
after. Full observer suite (37 tests) green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pull/99/head
Strycher 4 weeks ago
parent 708f168d45
commit 7825225e73

@ -78,6 +78,7 @@ class _ObserverSettingsViewState extends State<ObserverSettingsView> {
final c = svc.config;
setState(() => _saving = true);
var ok = true;
var invalidInterval = false;
Future<void> setIfChanged(bool changed, String key, String value) async {
if (changed) ok = await svc.setFlat(key, value) && ok;
@ -101,11 +102,17 @@ class _ObserverSettingsViewState extends State<ObserverSettingsView> {
'mqtt.iata',
_iata.text,
);
await setIfChanged(
c == null || _statusInterval.text != '${c.mqtt.statusInterval}',
'mqtt.status_interval',
_statusInterval.text,
);
// Validate before sending: the firmware range is 10-3600. Don't put a
// predictably invalid value on the wire (#78 MINOR-B).
if (c == null || _statusInterval.text != '${c.mqtt.statusInterval}') {
final n = int.tryParse(_statusInterval.text);
if (n != null && n >= 10 && n <= 3600) {
ok = await svc.setFlat('mqtt.status_interval', '$n') && ok;
} else {
invalidInterval = true;
ok = false;
}
}
await setIfChanged(
c == null || _displayAlwaysOn != c.display.alwaysOn,
'display.always_on',
@ -125,7 +132,9 @@ class _ObserverSettingsViewState extends State<ObserverSettingsView> {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
ok
invalidInterval
? 'Status interval must be 103600 seconds'
: ok
? 'Observer settings saved'
: (svc.lastError ??
'Some changes failed — re-read from the device'),
@ -190,6 +199,7 @@ class _ObserverSettingsViewState extends State<ObserverSettingsView> {
),
const SizedBox(height: 12),
TextField(
key: const Key('observer_interval'),
controller: _statusInterval,
keyboardType: TextInputType.number,
inputFormatters: [FilteringTextInputFormatter.digitsOnly],

@ -182,8 +182,19 @@ class ObserverConfigService extends ChangeNotifier {
rotation: int.tryParse(rotation ?? '') ?? 0,
),
);
_stale = false;
_lastError = null;
// A null from any getFlat is a failed read (GET returns the value, null
// on ERR/timeout). Don't present defaults as a complete snapshot or wipe
// the error a failed field already surfaced (SAFELANE error-visibility).
final allRead =
ssid != null &&
wifiEnabled != null &&
wifiStatus != null &&
iata != null &&
statusInterval != null &&
alwaysOn != null &&
rotation != null;
_stale = !allRead;
if (allRead) _lastError = null;
notifyListeners();
} catch (e) {
_stale = true;

@ -135,4 +135,40 @@ void main() {
expect(find.textContaining('last read'), findsOneWidget);
expect(find.text('GET wifi.ssid failed'), findsOneWidget);
});
testWidgets('an out-of-range status interval is not sent (#78 MINOR-B)', (
tester,
) async {
final fake = _FakeSvc(_cfg());
await _pump(tester, fake);
await tester.enterText(find.byKey(const Key('observer_interval')), '5');
await tester.tap(find.widgetWithText(FilledButton, 'Save'));
await tester.pumpAndSettle(); // _save runs; the SnackBar is still showing
expect(
fake.sets.where((e) => e.key == 'mqtt.status_interval'),
isEmpty,
reason: 'a value below the firmware range must not be put on the wire',
);
expect(find.textContaining('Status interval must be'), findsOneWidget);
// drain the SnackBar's auto-dismiss timer for a clean test end
await tester.pump(const Duration(seconds: 5));
await tester.pumpAndSettle();
});
testWidgets('an in-range status interval is sent (#78 MINOR-B)', (
tester,
) async {
final fake = _FakeSvc(_cfg());
await _pump(tester, fake);
await tester.enterText(find.byKey(const Key('observer_interval')), '120');
await _save(tester);
final iv = fake.sets.where((e) => e.key == 'mqtt.status_interval').toList();
expect(iv, hasLength(1));
expect(iv.single.value, '120');
});
}

@ -48,6 +48,73 @@ class _FakeConnector extends MeshCoreConnector {
void close() => _frames.close();
}
/// Auto-responds to each request so refresh()'s round-trips complete without
/// manual interleaving. [failKeys] answer ERR; others answer VALUE; the broker
/// dump is an empty pool (START -> END).
class _AutoConnector extends MeshCoreConnector {
_AutoConnector({this.failKeys = const {}});
final Set<String> failKeys;
final StreamController<Uint8List> _frames =
StreamController<Uint8List>.broadcast();
@override
Stream<Uint8List> get receivedFrames => _frames.stream;
@override
int? get firmwareVerCode => 14;
@override
int? get offbandCaps => 0x01;
@override
Future<void> sendFrame(
Uint8List data, {
String? channelSendQueueId,
bool expectsGenericAck = false,
}) async {
final op = data[1];
if (op == ObserverConfigClient.opGet) {
final key = utf8.decode(data.sublist(2, data.length - 1));
final frame = failKeys.contains(key)
? _resp(ObserverConfigClient.rErr, 'ERROR not available')
: _resp(ObserverConfigClient.rValue, '$key = ${_valueFor(key)}');
Future.microtask(() => _frames.add(frame));
} else if (op == ObserverConfigClient.opBrokers) {
Future.microtask(() {
_frames.add(
Uint8List.fromList([
ObserverConfigClient.respConfig,
ObserverConfigClient.rBrokersStart,
0,
]),
);
_frames.add(
Uint8List.fromList([
ObserverConfigClient.respConfig,
ObserverConfigClient.rBrokersEnd,
]),
);
});
}
}
String _valueFor(String key) => switch (key) {
'wifi.enabled' => '1',
'display.always_on' => '0',
'mqtt.status_interval' => '60',
'display.rotation' => '0',
'wifi.status' => 'StaConnected',
_ => 'x',
};
Uint8List _resp(int sub, String text) => Uint8List.fromList([
ObserverConfigClient.respConfig,
sub,
...utf8.encode(text),
0,
]);
void closeStream() => _frames.close();
}
Uint8List _respText(int sub, String text) => Uint8List.fromList([
ObserverConfigClient.respConfig,
sub,
@ -125,4 +192,34 @@ void main() {
reason: 'secret value must never appear in an error',
);
});
// ---- MINOR-A (#78): a partial read failure must stay visible ----
test(
'refresh keeps the error visible + stale on a partial read failure',
() async {
final auto = _AutoConnector(failKeys: {'wifi.ssid'});
final s = ObserverConfigService(auto);
await s.refresh();
expect(
s.stale,
isTrue,
reason: 'a failed field must mark the snapshot stale',
);
expect(
s.lastError,
isNotNull,
reason: 'a failed GET error must not be wiped by partial success',
);
auto.closeStream();
},
);
test('refresh clears the error + is not stale on a full read', () async {
final auto = _AutoConnector();
final s = ObserverConfigService(auto);
await s.refresh();
expect(s.stale, isFalse);
expect(s.lastError, isNull);
auto.closeStream();
});
}

Loading…
Cancel
Save

Powered by TurnKey Linux.