fix(#80): broker enable/disable feedback + don't validate a disable

On-hardware test feedback:
- Long-press Enable/Disable was silent on success AND failure. It now confirms
  on success and, on failure, surfaces the device's reason (lastError) instead
  of nothing.
- The client now refuses to quick-Enable a broker that can't work (incomplete
  required fields, e.g. JWT without audience/owner) and says why -- so it never
  reports "enabled" on a broker that won't run. (Firmware should reject too.)
- The editor blocked a DISABLE on blank/invalid fields. Validation now gates
  ENABLING only -- disabling a slot with partial values is fine; it won't be
  active.

Validation rules shared via BrokerConfig.enableError (editor pre-save + list
quick-Enable). Full suite green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/64-observer-config
Strycher 4 weeks ago
parent bc8b285ed4
commit 0d9dc79b91

@ -144,6 +144,19 @@ class BrokerConfig {
/// A slot is occupied iff it has a URL (firmware `cfg.url[0] != '\0'`).
bool get isPopulated => url.isNotEmpty;
/// Why this broker can't be safely enabled, or null if complete. Shared by
/// the editor's pre-save check and the list's quick Enable so the client
/// never activates a broker that can't work (#80).
String? get enableError {
if (url.isEmpty) return 'URL is required';
if (port < 1 || port > 65535) return 'Port must be between 1 and 65535';
if (authType == BrokerAuthType.jwt &&
(jwtAudience.isEmpty || jwtOwner.isEmpty)) {
return 'JWT auth needs an audience and an owner';
}
return null;
}
/// Build from one slot's decoded `key=value` lines (the OCFG_BROKER_KV bodies).
/// Unknown keys are ignored; missing keys keep the defaults.
factory BrokerConfig.fromWireFields(int slot, Map<String, String> kv) {

@ -112,20 +112,16 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
bool get _dirty =>
_changedFields().isNotEmpty || _enabled != _baseline.enabled;
/// Reject predictably-bad values before they reach the wire (#80). Returns an
/// error message, or null when the form is valid.
String? _validate() {
if (_url.text.trim().isEmpty) return 'URL is required';
final port = int.tryParse(_port.text.trim());
if (port == null || port < 1 || port > 65535) {
return 'Port must be between 1 and 65535';
}
if (_authType == BrokerAuthType.jwt &&
(_jwtAudience.text.trim().isEmpty || _jwtOwner.text.trim().isEmpty)) {
return 'JWT auth requires an audience and an owner';
}
return null;
}
/// Pre-enable validation, shared with the list's quick Enable via
/// [BrokerConfig.enableError]. Builds a tentative config from the form.
String? _validate() => BrokerConfig(
slot: _baseline.slot,
url: _url.text.trim(),
port: int.tryParse(_port.text.trim()) ?? -1,
authType: _authType,
jwtAudience: _jwtAudience.text.trim(),
jwtOwner: _jwtOwner.text.trim(),
).enableError;
void _snack(String msg, {bool isError = false}) {
ScaffoldMessenger.of(context).showSnackBar(
@ -137,10 +133,14 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
}
Future<void> _save() async {
final err = _validate();
if (err != null) {
_snack(err, isError: true);
return;
// Validation only gates ENABLING disabling a slot with blank/partial
// fields is fine; it won't be active.
if (_enabled) {
final err = _validate();
if (err != null) {
_snack(err, isError: true);
return;
}
}
final svc = context.read<ObserverConfigService>();
final messenger = ScaffoldMessenger.of(context);

@ -57,9 +57,43 @@ class _MqttBrokersScreenState extends State<MqttBrokersScreen> {
}
Future<void> _quickToggle(BrokerConfig b, bool enable) async {
final messenger = ScaffoldMessenger.of(context);
final errorColor = Theme.of(context).colorScheme.error;
// The client refuses to enable a broker that can't work, and says why
// (firmware should reject it too, but don't claim success on the wire).
if (enable) {
final err = b.enableError;
if (err != null) {
messenger.showSnackBar(
SnackBar(
content: Text("Can't enable broker ${b.slot}: $err"),
backgroundColor: errorColor,
),
);
return;
}
}
final svc = context.read<ObserverConfigService>();
await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0');
if (mounted) await _reload();
final ok = await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0');
if (!mounted) return;
if (ok) {
messenger.showSnackBar(
SnackBar(
content: Text('Broker ${b.slot} ${enable ? 'enabled' : 'disabled'}'),
),
);
await _reload();
} else {
messenger.showSnackBar(
SnackBar(
content: Text(
svc.lastError ??
'Failed to ${enable ? 'enable' : 'disable'} broker ${b.slot}',
),
backgroundColor: errorColor,
),
);
}
}
Future<void> _clear(BrokerConfig b) async {

@ -115,7 +115,7 @@ void main() {
await _open(
tester,
fake,
const BrokerConfig(slot: 1, url: 'h', port: 1883),
const BrokerConfig(slot: 1, url: 'h', port: 1883, enabled: true),
);
await tester.enterText(find.byKey(const Key('broker_port')), '99999');
@ -172,4 +172,57 @@ void main() {
expect(find.text('fromdevice'), findsOneWidget);
expect(find.text('8883'), findsOneWidget);
});
testWidgets('disabling a slot saves even with incomplete fields', (
tester,
) async {
final fake = _FakeSvc();
await _open(
tester,
fake,
const BrokerConfig(
slot: 2,
url: 'h',
port: 1883,
enabled: true,
authType: BrokerAuthType.jwt, // blank audience/owner
),
);
await tester.tap(find.byType(SwitchListTile)); // toggle enabled OFF
await tester.pumpAndSettle();
await tester.tap(find.byTooltip('Save'));
await tester.pumpAndSettle();
expect(
fake.saveCalls,
hasLength(1),
reason: 'disabling must not be blocked by field validation',
);
expect(fake.saveCalls.single.enable, isFalse);
await _drainSnack(tester);
});
testWidgets('enabling with incomplete JWT is blocked', (tester) async {
final fake = _FakeSvc();
await _open(
tester,
fake,
const BrokerConfig(
slot: 2,
url: 'h',
port: 1883,
authType: BrokerAuthType.jwt, // disabled, blank audience/owner
),
);
await tester.tap(find.byType(SwitchListTile)); // toggle enabled ON
await tester.pumpAndSettle();
await tester.tap(find.byTooltip('Save'));
await tester.pumpAndSettle();
expect(fake.saveCalls, isEmpty);
expect(find.textContaining('needs an audience'), findsOneWidget);
await _drainSnack(tester);
});
}

@ -18,7 +18,11 @@ class _FakeSvc extends ObserverConfigService {
final List<BrokerConfig> _brokers;
final List<String> setCalls = [];
final List<int> clearCalls = [];
bool toggleOk = true;
String? errorText;
@override
String? get lastError => errorText;
@override
ObserverConfig? get config => ObserverConfig(brokers: _brokers);
@override
@ -26,7 +30,7 @@ class _FakeSvc extends ObserverConfigService {
@override
Future<bool> setBrokerField(int slot, String field, String value) async {
setCalls.add('$slot.$field=$value');
return true;
return toggleOk;
}
@override
@ -97,4 +101,46 @@ void main() {
await tester.pumpAndSettle();
expect(fake.clearCalls, contains(2));
});
testWidgets('quick Enable refuses an incomplete broker with a reason', (
tester,
) async {
final fake = _FakeSvc(const [
BrokerConfig(slot: 2, url: 'a', port: 1883, authType: BrokerAuthType.jwt),
]);
await _pump(tester, fake);
await tester.longPress(find.text('[2] a'));
await tester.pumpAndSettle();
await tester.tap(find.text('Enable'));
await tester.pumpAndSettle();
expect(
fake.setCalls,
isEmpty,
reason: 'an incomplete broker must not be enabled',
);
expect(find.textContaining("Can't enable"), findsOneWidget);
await tester.pump(const Duration(seconds: 5));
});
testWidgets('a failed quick toggle surfaces the device reason', (
tester,
) async {
final fake =
_FakeSvc(const [
BrokerConfig(slot: 2, url: 'a', port: 1883, enabled: true),
])
..toggleOk = false
..errorText = 'ERROR broker busy';
await _pump(tester, fake);
await tester.longPress(find.text('[2] a'));
await tester.pumpAndSettle();
await tester.tap(find.text('Disable'));
await tester.pumpAndSettle();
expect(find.textContaining('ERROR broker busy'), findsOneWidget);
await tester.pump(const Duration(seconds: 5));
});
}

Loading…
Cancel
Save

Powered by TurnKey Linux.