From ee01c1917526eeec8c511693179a87c314387bec Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Sat, 7 Nov 2026 05:53:52 -0500 Subject: [PATCH] test(#1117): pin the restore plan's rules directly Gemini review of #1117, findings verified against the code and firmware. Kept: - planChannelRestore was tested only through the connector. There are now 11 direct tests: Public to slot 0, nothing else there, a live channel never overwritten, a channel already on the radio left out, erased slots cleared first, no slot used twice, no key written twice, erased and empty cache entries never written, and no-room channels reported. - Removed a Public-first sort that guarded nothing: nothing but Public is ever placed in slot 0, so the order cannot matter (a mutation dropping it passed every test, which is how it showed). Justified, not changed: - "A second add right after Public displaces slot 0 uses a stale list": the re-read starts synchronously in the write's finally, before the write returns, so the table stays busy and the add gets the busy message. - "Public is refused on new firmware when slot 0 is occupied": unreachable there. Public can't be deleted, and boot puts a missing Public into the first empty slot. So Public missing with slot 0 occupied happens only on a full table, where nothing has room. On older firmware the fallback keeps slot 0's channel from being overwritten. - "Restoring Public over an erased slot 0 overwrites a recoverable record": erased flash holds nothing. A cached channel from slot 0 is re-homed elsewhere, not lost. Full suite 2045 pass, analyze clean. Co-Authored-By: Claude Opus 5.5 --- lib/helpers/channel_restore_plan.dart | 13 +-- test/helpers/channel_restore_plan_test.dart | 109 ++++++++++++++++++++ 2 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 test/helpers/channel_restore_plan_test.dart diff --git a/lib/helpers/channel_restore_plan.dart b/lib/helpers/channel_restore_plan.dart index 3cbb2b25..97bd2028 100644 --- a/lib/helpers/channel_restore_plan.dart +++ b/lib/helpers/channel_restore_plan.dart @@ -61,7 +61,8 @@ class ChannelRestorePlan { /// overwritten. /// * An erased slot is cleared first. /// -/// Public is placed first, so slot 0 is never used by anything else. +/// Order does not matter: nothing but Public is ever placed in slot 0, so +/// Public always finds it free unless a live channel holds it. ChannelRestorePlan planChannelRestore({ required List cached, required List live, @@ -70,13 +71,9 @@ ChannelRestorePlan planChannelRestore({ }) { final liveKeys = live.map((c) => c.pskHex).toSet(); final taken = live.map((c) => c.index).toSet(); - final candidates = - cached - .where((c) => !c.isUnusable && !liveKeys.contains(c.pskHex)) - .toList() - ..sort( - (a, b) => (b.isPublicChannel ? 1 : 0) - (a.isPublicChannel ? 1 : 0), - ); + final candidates = cached + .where((c) => !c.isUnusable && !liveKeys.contains(c.pskHex)) + .toList(); final steps = []; final noRoom = []; diff --git a/test/helpers/channel_restore_plan_test.dart b/test/helpers/channel_restore_plan_test.dart new file mode 100644 index 00000000..d6b4fbd7 --- /dev/null +++ b/test/helpers/channel_restore_plan_test.dart @@ -0,0 +1,109 @@ +// #1117: Restore to radio places by identity. The plan is pure, so its rules +// are pinned here directly; channel_erased_slot_test drives it through the +// connector against a radio that answers. + +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/helpers/channel_restore_plan.dart'; +import 'package:meshcore_open/models/channel.dart'; + +Channel _ch(int index, String name, int fill) => Channel( + index: index, + name: name, + psk: Uint8List.fromList(List.filled(16, fill)), +); + +final _public = Channel( + index: 3, + name: 'Public', + psk: Channel.parsePskHex(Channel.publicChannelPsk), +); + +ChannelRestorePlan _plan( + List cached, { + List live = const [], + Set erased = const {}, + int max = 8, +}) => planChannelRestore( + cached: cached, + live: live, + erasedSlots: erased, + maxChannels: max, +); + +Map _slots(ChannelRestorePlan p) => { + for (final s in p.steps) s.channel.name: s.slot, +}; + +void main() { + test('Public goes to slot 0, wherever the cache remembered it', () { + expect(_slots(_plan([_public])), {'Public': 0}); + }); + + test('Public is placed first, so nothing else can take slot 0', () { + final p = _plan([_ch(0, 'old zero', 1), _public]); + expect(_slots(p), {'Public': 0, 'old zero': 1}); + }); + + test('with a live channel in slot 0, Public takes a free slot instead', () { + // Older firmware only: nothing is written over the live channel. + final p = _plan([_public], live: [_ch(0, 'live', 9)]); + expect(_slots(p)['Public'], isNot(0)); + }); + + test('a channel keeps its remembered slot when that slot is free', () { + expect(_slots(_plan([_ch(4, 'four', 4)])), {'four': 4}); + }); + + test('a channel never overwrites a live one; it moves', () { + final p = _plan([_ch(1, 'cached', 1)], live: [_ch(1, 'live', 2)]); + expect(_slots(p)['cached'], 2); + }); + + test('a channel the radio still has, in any slot, is left out', () { + final p = _plan([_ch(1, 'same', 1)], live: [_ch(5, 'same', 1)]); + expect(p.steps, isEmpty); + expect(p.noRoom, isEmpty); + }); + + test('only erased slots are cleared first', () { + final p = _plan( + [_ch(1, 'erased slot', 1), _ch(2, 'empty slot', 2)], + erased: {1}, + ); + final clear = {for (final s in p.steps) s.channel.name: s.clearFirst}; + expect(clear, {'erased slot': true, 'empty slot': false}); + }); + + test('two remembered at one slot: one keeps it, one moves', () { + final p = _plan([_ch(1, 'first', 1), _ch(1, 'second', 2)]); + final slots = _slots(p); + expect(slots.values.toSet(), hasLength(2), reason: 'never one slot twice'); + expect(slots['first'], 1); + }); + + test('the same key cached twice is written once', () { + expect(_plan([_ch(1, 'a', 1), _ch(2, 'a again', 1)]).steps, hasLength(1)); + }); + + test('erased and empty cached entries are never written', () { + final erased = Channel( + index: 2, + name: 'x', + psk: Uint8List.fromList(List.filled(16, 0xFF)), + ); + final empty = Channel(index: 3, name: '', psk: Uint8List(16)); + expect(_plan([erased, empty]).steps, isEmpty); + }); + + test('with no room left, the rest are reported, not dropped', () { + final p = _plan( + [_ch(1, 'a', 1), _ch(2, 'b', 2)], + live: [_ch(1, 'l1', 8), _ch(2, 'l2', 9)], + max: 3, + ); + expect(p.steps, isEmpty, reason: 'slot 0 is Public\'s, 1 and 2 are live'); + expect(p.noRoom.map((c) => c.name), ['a', 'b']); + }); +}