diff --git a/lib/helpers/config_profile_diff.dart b/lib/helpers/config_profile_diff.dart index 8df667e..cfa850d 100644 --- a/lib/helpers/config_profile_diff.dart +++ b/lib/helpers/config_profile_diff.dart @@ -100,25 +100,7 @@ ProfileDiff buildProfileDiff( ), ); } - // enabled change (add/change), never danger/secret - if (b.enabled != null) { - final current = cur[ConfigKeys.brokerEnabled]; - final newVal = b.enabled! ? '1' : '0'; - if (current != newVal) { - rows.add( - DiffRow( - label: 'broker ${b.slot}.${ConfigKeys.brokerEnabled}', - oldValue: current, - newValue: newVal, - kind: (current == null || current.isEmpty) - ? DiffKind.add - : DiffKind.change, - danger: false, - secret: false, - ), - ); - } - } + // Broker enabled is not a profile field (#456) — never diffed/applied. } return ProfileDiff(rows); diff --git a/lib/helpers/config_profile_parser.dart b/lib/helpers/config_profile_parser.dart index 8ce7fda..12b6d10 100644 --- a/lib/helpers/config_profile_parser.dart +++ b/lib/helpers/config_profile_parser.dart @@ -114,7 +114,6 @@ List _parseBrokers(dynamic node) { final map = _asMap(node[i], ctx); _rejectUnknownKeys(map, const { 'slot', - 'enabled', 'url', 'port', 'transport', @@ -151,7 +150,6 @@ List _parseBrokers(dynamic node) { brokers.add( BrokerConfig( slot: slot, - enabled: _optBool(map, 'enabled', ctx), url: _optString(map, 'url', ctx), port: port, transport: _optEnum( diff --git a/lib/helpers/config_profile_writes.dart b/lib/helpers/config_profile_writes.dart index 176cedd..e589fae 100644 --- a/lib/helpers/config_profile_writes.dart +++ b/lib/helpers/config_profile_writes.dart @@ -6,8 +6,8 @@ import '../models/config_profile.dart'; /// - **Skip null or empty**, a profile only touches keys it actually sets; a /// blank never clobbers a configured value. Clearing is a separate explicit op. /// - **Skip `jwt_token`**, it's live-minted by firmware at connect, never config. -/// - **`enabled` is written last** (the executor's activation guard), so it is -/// returned separately from [BrokerWrites.fields]. +/// - **Skip broker `enabled`** (#456), enabling a broker is the operator's +/// runtime decision, not profile config; apply preserves the current state. /// - **Danger fields** (owner/identity/credentials) are flagged so the preview /// (#406) can gate them: broker `username`/`password`/`jwt_owner`/`jwt_email`, /// and global `wifi.pwd`. @@ -31,19 +31,17 @@ class FlatWrite { final bool danger; } -/// The writes for one broker slot. [fields] excludes `enabled` (written last by -/// the executor) and `jwt_token` (never written). [enabled] is null when the -/// profile doesn't set it, so the executor preserves the device's current state. +/// The writes for one broker slot. [fields] excludes `jwt_token` (never written) +/// and the broker `enabled` flag (#456: not a profile field — apply preserves +/// the device's current enabled state). class BrokerWrites { const BrokerWrites({ required this.slot, required this.fields, - required this.enabled, required this.dangerFields, }); final int slot; final Map fields; - final bool? enabled; final Set dangerFields; } @@ -65,8 +63,7 @@ class ProfileWrites { /// Partition writes into (safe, danger) so the preview's two buttons each apply /// their own set: the normal Apply writes safe changes; the red gate writes the -/// credential/identity ones. A broker with both is split across both, its -/// `enabled` toggle rides with the safe half only. +/// credential/identity ones. A broker with both is split across both. ({ProfileWrites safe, ProfileWrites danger}) splitProfileWrites( ProfileWrites w, ) { @@ -84,14 +81,9 @@ class ProfileWrites { for (final e in b.fields.entries) if (b.dangerFields.contains(e.key)) e.key: e.value, }; - if (safeFields.isNotEmpty || b.enabled != null) { + if (safeFields.isNotEmpty) { safeBrokers.add( - BrokerWrites( - slot: b.slot, - fields: safeFields, - enabled: b.enabled, - dangerFields: const {}, - ), + BrokerWrites(slot: b.slot, fields: safeFields, dangerFields: const {}), ); } if (dangerFields.isNotEmpty) { @@ -99,7 +91,6 @@ class ProfileWrites { BrokerWrites( slot: b.slot, fields: dangerFields, - enabled: null, // never toggle activation from the credential pass dangerFields: dangerFields.keys.toSet(), ), ); @@ -171,13 +162,12 @@ ProfileWrites enumerateProfileWrites(ConfigProfile p) { put(ConfigKeys.brokerTopicPrefix, b.topicPrefix); put(ConfigKeys.brokerIataOverride, b.iataOverride); - if (fields.isEmpty && b.enabled == null) continue; // nothing to write + if (fields.isEmpty) continue; // nothing to write brokers.add( BrokerWrites( slot: b.slot, fields: fields, - enabled: b.enabled, dangerFields: fields.keys.where(kDangerBrokerFields.contains).toSet(), ), ); diff --git a/lib/models/config_profile.dart b/lib/models/config_profile.dart index 51ee47e..3706d1a 100644 --- a/lib/models/config_profile.dart +++ b/lib/models/config_profile.dart @@ -74,7 +74,6 @@ class WifiConfig { class BrokerConfig { const BrokerConfig({ required this.slot, - this.enabled, this.url, this.port, this.transport, @@ -92,8 +91,12 @@ class BrokerConfig { }); /// 0-based slot index, `0 <= slot < kMaxBrokerSlots`. + /// + /// Broker `enabled` is deliberately NOT a profile field (#456): enabling a + /// broker is the operator's runtime decision (firmware ships slots disabled, + /// opt-in per #262). A profile configures the connection; apply preserves the + /// device's current enabled state. final int slot; - final bool? enabled; final String? url; final int? port; final MqttTransport? transport; diff --git a/lib/services/observer_apply_service.dart b/lib/services/observer_apply_service.dart index 74c8ff3..bd0034f 100644 --- a/lib/services/observer_apply_service.dart +++ b/lib/services/observer_apply_service.dart @@ -38,16 +38,15 @@ class ObserverApplyService { } for (final b in writes.brokers) { - // Current state: wasLive for the safe-save dance, and to preserve `enabled` - // when the profile doesn't set it. + // Profiles never set broker enabled (#456) — preserve the device's current + // state: read wasLive for the safe-save dance and re-enable to the same. final current = await _svc.getBroker(b.slot); final wasLive = current?.enabled ?? false; - final enable = b.enabled ?? wasLive; final res = await _svc.saveBroker( b.slot, fields: b.fields, - enable: enable, + enable: wasLive, wasLive: wasLive, ); items.add( diff --git a/test/helpers/config_profile_diff_test.dart b/test/helpers/config_profile_diff_test.dart index 07aba57..e53168c 100644 --- a/test/helpers/config_profile_diff_test.dart +++ b/test/helpers/config_profile_diff_test.dart @@ -77,26 +77,34 @@ void main() { }); }); - test('broker enabled change is a plain (non-danger) row', () { - final w = _writes( - const ConfigProfile( - schemaVersion: 2, - mqtt: MqttSection( - brokers: [BrokerConfig(slot: 2, enabled: true, url: 'h')], + test( + 'broker field change is a plain (non-danger) row; unchanged dropped', + () { + final w = _writes( + const ConfigProfile( + schemaVersion: 2, + mqtt: MqttSection( + brokers: [ + BrokerConfig(slot: 2, url: 'new-host', topicPrefix: 'mc'), + ], + ), ), - ), - ); - final d = buildProfileDiff( - w, - currentFlat: const {}, - currentBroker: { - 2: {ConfigKeys.brokerEnabled: '0', ConfigKeys.brokerUrl: 'h'}, - }, - ); - // url unchanged (h==h) dropped; enabled 0->1 present, not danger - final labels = d.rows.map((r) => r.label).toList(); - expect(labels, ['broker 2.${ConfigKeys.brokerEnabled}']); - expect(d.rows.single.danger, isFalse); - }); + ); + final d = buildProfileDiff( + w, + currentFlat: const {}, + currentBroker: { + 2: { + ConfigKeys.brokerUrl: 'old-host', + ConfigKeys.brokerTopicPrefix: 'mc', // unchanged -> dropped + }, + }, + ); + final labels = d.rows.map((r) => r.label).toList(); + expect(labels, ['broker 2.${ConfigKeys.brokerUrl}']); + expect(d.rows.single.kind, DiffKind.change); + expect(d.rows.single.danger, isFalse); + }, + ); }); } diff --git a/test/helpers/config_profile_parser_test.dart b/test/helpers/config_profile_parser_test.dart index b8df6a9..36c5dfc 100644 --- a/test/helpers/config_profile_parser_test.dart +++ b/test/helpers/config_profile_parser_test.dart @@ -17,7 +17,6 @@ mqtt: status_interval: 60 brokers: - slot: 0 - enabled: true url: mqtt.example.org port: 8883 transport: tls @@ -26,7 +25,6 @@ mqtt: password: p topic_prefix: meshcore - slot: 2 - enabled: false transport: wss auth_type: jwt jwt_refresh: 3600 @@ -46,7 +44,6 @@ mqtt: expect(b0.authType, MqttAuthType.basic); final b2 = p.mqtt!.brokers.firstWhere((b) => b.slot == 2); - expect(b2.enabled, false); expect(b2.transport, MqttTransport.wss); expect(b2.authType, MqttAuthType.jwt); expect(b2.jwtRefresh, 3600); @@ -119,6 +116,15 @@ mqtt: ); }); + test('rejects broker "enabled" (#456 — not a profile field)', () { + expect( + () => parseConfigProfile( + 'schema_version: 2\nmqtt:\n brokers:\n - slot: 0\n enabled: true\n', + ), + throwsA(isA()), + ); + }); + test('rejects an out-of-range broker slot', () { expect( () => parseConfigProfile( diff --git a/test/helpers/config_profile_writes_test.dart b/test/helpers/config_profile_writes_test.dart index 8753450..8d593e0 100644 --- a/test/helpers/config_profile_writes_test.dart +++ b/test/helpers/config_profile_writes_test.dart @@ -49,18 +49,16 @@ void main() { expect(b.fields[ConfigKeys.brokerUrl], 'h'); }); - test('enabled kept out of fields (executor writes it last)', () { + test('broker enabled is never emitted (#456 — not a profile field)', () { final w = enumerateProfileWrites( const ConfigProfile( schemaVersion: 2, - mqtt: MqttSection( - brokers: [BrokerConfig(slot: 1, url: 'h', enabled: true)], - ), + mqtt: MqttSection(brokers: [BrokerConfig(slot: 1, url: 'h')]), ), ); final b = w.brokers.single; expect(b.fields.containsKey(ConfigKeys.brokerEnabled), isFalse); - expect(b.enabled, true); + expect(b.fields[ConfigKeys.brokerUrl], 'h'); }); test('formats enums as wire strings and ints as text', () { @@ -135,20 +133,14 @@ void main() { }); group('splitProfileWrites', () { - test('splits a mixed broker; enabled rides with safe half only', () { + test('splits a mixed broker into safe + danger halves', () { final w = enumerateProfileWrites( const ConfigProfile( schemaVersion: 2, wifi: WifiConfig(ssid: 'net', password: 'pw', enabled: true), mqtt: MqttSection( brokers: [ - BrokerConfig( - slot: 0, - url: 'h', - username: 'u', - password: 'p', - enabled: true, - ), + BrokerConfig(slot: 0, url: 'h', username: 'u', password: 'p'), ], ), ), @@ -160,7 +152,6 @@ void main() { expect(safeFlatKeys, contains(ConfigKeys.wifiEnabled)); expect(safeFlatKeys.contains(ConfigKeys.wifiPassword), isFalse); expect(s.safe.brokers.single.fields.keys, contains(ConfigKeys.brokerUrl)); - expect(s.safe.brokers.single.enabled, true); final dangerFlatKeys = s.danger.flats.map((f) => f.key).toSet(); expect(dangerFlatKeys, {ConfigKeys.wifiPassword}); @@ -169,7 +160,6 @@ void main() { ConfigKeys.brokerUsername, ConfigKeys.brokerPassword, }); - expect(db.enabled, isNull); }); }); }