fix(#79): degrade gracefully when the broker dump fails (G2 finding)

The G2 BLE log showed the config protocol works (GETs return values; the broker
dump starts) but the firmware's broker dump stalls without BROKERS_END, so
getBrokers() times out. The client was treating that as a TOTAL failure --
discarding the 7 flat settings it read cleanly and blanking the pane.

Now the broker pool loads independently: refresh() builds + shows the flat
config even when getBrokers fails, flags brokersUnavailable, and the pane shows
the broker section as "unavailable" instead of going stale. Flat settings stay
usable. TDD red->green; full observer suite (39) green.

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

@ -236,7 +236,13 @@ class _ObserverSettingsViewState extends State<ObserverSettingsView> {
), ),
const Divider(height: 32), const Divider(height: 32),
_sectionTitle('MQTT brokers'), _sectionTitle('MQTT brokers'),
..._brokerTiles(svc.config?.brokers ?? const []), if (svc.brokersUnavailable)
_banner(
Icons.cloud_off,
'Broker pool unavailable — the device did not finish sending it.',
)
else
..._brokerTiles(svc.config?.brokers ?? const []),
], ],
); );
} }

@ -37,6 +37,11 @@ class ObserverConfigService extends ChangeNotifier {
String? _lastError; String? _lastError;
String? get lastError => _lastError; String? get lastError => _lastError;
/// True when the broker-pool dump failed but the flat settings read OK the
/// UI shows the broker section as unavailable instead of blanking the pane.
bool _brokersUnavailable = false;
bool get brokersUnavailable => _brokersUnavailable;
/// Whether the connected device supports the config command the version /// Whether the connected device supports the config command the version
/// gate AND the capability bit, read live from the connector's device-info /// gate AND the capability bit, read live from the connector's device-info
/// parse. The settings UI watches the connector, so the Observer category /// parse. The settings UI watches the connector, so the Observer category
@ -164,28 +169,27 @@ class ObserverConfigService extends ChangeNotifier {
final rotation = await getFlat('display.rotation'); final rotation = await getFlat('display.rotation');
final brokers = await getBrokers(); final brokers = await getBrokers();
if (brokers == null) { // The broker pool loads independently of the flat settings: a broker-dump
_stale = true; // failure (e.g. the firmware never sends BROKERS_END) must NOT blank
notifyListeners(); // settings that read cleanly. Surface it as broker-only "unavailable".
return;
}
_config = ObserverConfig( _config = ObserverConfig(
wifi: _parseWifi(ssid, wifiEnabled, wifiStatus), wifi: _parseWifi(ssid, wifiEnabled, wifiStatus),
mqtt: MqttGlobalConfig( mqtt: MqttGlobalConfig(
iata: iata ?? '', iata: iata ?? '',
statusInterval: int.tryParse(statusInterval ?? '') ?? 60, statusInterval: int.tryParse(statusInterval ?? '') ?? 60,
), ),
brokers: brokers, brokers: brokers ?? const [],
display: DisplayConfig( display: DisplayConfig(
alwaysOn: alwaysOn == '1', alwaysOn: alwaysOn == '1',
rotation: int.tryParse(rotation ?? '') ?? 0, rotation: int.tryParse(rotation ?? '') ?? 0,
), ),
); );
// A null from any getFlat is a failed read (GET returns the value, null _brokersUnavailable = brokers == null;
// on ERR/timeout). Don't present defaults as a complete snapshot or wipe
// the error a failed field already surfaced (SAFELANE error-visibility). // A null from any flat getFlat is a failed read (GET returns the value,
final allRead = // null on ERR/timeout). Stale reflects the FLAT read only; a broker miss
// is shown in its own section, not as a stale snapshot (SAFELANE §6).
final allFlatRead =
ssid != null && ssid != null &&
wifiEnabled != null && wifiEnabled != null &&
wifiStatus != null && wifiStatus != null &&
@ -193,8 +197,8 @@ class ObserverConfigService extends ChangeNotifier {
statusInterval != null && statusInterval != null &&
alwaysOn != null && alwaysOn != null &&
rotation != null; rotation != null;
_stale = !allRead; _stale = !allFlatRead;
if (allRead) _lastError = null; if (allFlatRead) _lastError = null;
notifyListeners(); notifyListeners();
} catch (e) { } catch (e) {
_stale = true; _stale = true;

@ -17,14 +17,21 @@ import 'package:meshcore_open/services/observer_config_service.dart';
class _DummyConn extends MeshCoreConnector {} class _DummyConn extends MeshCoreConnector {}
class _FakeSvc extends ObserverConfigService { class _FakeSvc extends ObserverConfigService {
_FakeSvc(this._cfg, {this.staleFlag = false, this.errorText}) _FakeSvc(
: super(_DummyConn()); this._cfg, {
this.staleFlag = false,
this.errorText,
this.brokersDown = false,
}) : super(_DummyConn());
final ObserverConfig _cfg; final ObserverConfig _cfg;
final bool staleFlag; final bool staleFlag;
final String? errorText; final String? errorText;
final bool brokersDown;
final List<MapEntry<String, String>> sets = []; final List<MapEntry<String, String>> sets = [];
@override
bool get brokersUnavailable => brokersDown;
@override @override
bool get supported => true; bool get supported => true;
@override @override
@ -171,4 +178,16 @@ void main() {
expect(iv, hasLength(1)); expect(iv, hasLength(1));
expect(iv.single.value, '120'); expect(iv.single.value, '120');
}); });
testWidgets('broker pool shows unavailable when the dump failed (#79)', (
tester,
) async {
final fake = _FakeSvc(_cfg(), brokersDown: true);
await _pump(tester, fake);
// The broker section is at the bottom of the scroll view; scroll it in.
await tester.drag(find.byType(ListView), const Offset(0, -2000));
await tester.pumpAndSettle();
expect(find.textContaining('Broker pool unavailable'), findsOneWidget);
});
} }

@ -52,8 +52,9 @@ class _FakeConnector extends MeshCoreConnector {
/// manual interleaving. [failKeys] answer ERR; others answer VALUE; the broker /// manual interleaving. [failKeys] answer ERR; others answer VALUE; the broker
/// dump is an empty pool (START -> END). /// dump is an empty pool (START -> END).
class _AutoConnector extends MeshCoreConnector { class _AutoConnector extends MeshCoreConnector {
_AutoConnector({this.failKeys = const {}}); _AutoConnector({this.failKeys = const {}, this.failBrokers = false});
final Set<String> failKeys; final Set<String> failKeys;
final bool failBrokers;
final StreamController<Uint8List> _frames = final StreamController<Uint8List> _frames =
StreamController<Uint8List>.broadcast(); StreamController<Uint8List>.broadcast();
@ -78,6 +79,7 @@ class _AutoConnector extends MeshCoreConnector {
: _resp(ObserverConfigClient.rValue, '$key = ${_valueFor(key)}'); : _resp(ObserverConfigClient.rValue, '$key = ${_valueFor(key)}');
Future.microtask(() => _frames.add(frame)); Future.microtask(() => _frames.add(frame));
} else if (op == ObserverConfigClient.opBrokers) { } else if (op == ObserverConfigClient.opBrokers) {
if (failBrokers) return; // no response -> getBrokers times out
Future.microtask(() { Future.microtask(() {
_frames.add( _frames.add(
Uint8List.fromList([ Uint8List.fromList([
@ -222,4 +224,28 @@ void main() {
expect(s.lastError, isNull); expect(s.lastError, isNull);
auto.closeStream(); auto.closeStream();
}); });
// ---- #79: a broker-dump failure must not blank the flat settings ----
test(
'broker-dump failure keeps flat settings + flags brokers unavailable',
() async {
final auto = _AutoConnector(failBrokers: true);
final s = ObserverConfigService(auto);
s.timeout = const Duration(milliseconds: 50);
await s.refresh();
expect(
s.config,
isNotNull,
reason: 'settings that read cleanly must still show',
);
expect(s.config!.wifi.ssid, 'x');
expect(s.brokersUnavailable, isTrue);
expect(
s.stale,
isFalse,
reason: 'a broker-dump miss is not a stale snapshot',
);
auto.closeStream();
},
);
} }

Loading…
Cancel
Save

Powered by TurnKey Linux.