diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 993aba5..5f28d62 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -1586,6 +1586,7 @@ } }, "repeater_cliUnmatchedResponse": "Unrequested response from repeater", + "repeater_lateResponseDiscarded": "A late response arrived but was not applied, because you have unsaved changes.", "repeater_lateResponseReceived": "A late response arrived after the command timed out. See the CLI screen.", "repeater_cliQuickGetName": "Get Name", "repeater_cliQuickGetRadio": "Get Radio", diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index 0e25ba2..05661a2 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -5144,6 +5144,12 @@ abstract class AppLocalizations { /// **'Unrequested response from repeater'** String get repeater_cliUnmatchedResponse; + /// No description provided for @repeater_lateResponseDiscarded. + /// + /// In en, this message translates to: + /// **'A late response arrived but was not applied, because you have unsaved changes.'** + String get repeater_lateResponseDiscarded; + /// No description provided for @repeater_lateResponseReceived. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_bg.dart b/lib/l10n/app_localizations_bg.dart index c85488c..9a60600 100644 --- a/lib/l10n/app_localizations_bg.dart +++ b/lib/l10n/app_localizations_bg.dart @@ -2919,6 +2919,10 @@ class AppLocalizationsBg extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index 7424d8f..43a3de2 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -2922,6 +2922,10 @@ class AppLocalizationsDe extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index 74606aa..9426ce1 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -2863,6 +2863,10 @@ class AppLocalizationsEn extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_es.dart b/lib/l10n/app_localizations_es.dart index b1e9d30..c3127f0 100644 --- a/lib/l10n/app_localizations_es.dart +++ b/lib/l10n/app_localizations_es.dart @@ -2913,6 +2913,10 @@ class AppLocalizationsEs extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_fr.dart b/lib/l10n/app_localizations_fr.dart index 9ba508b..7d06f98 100644 --- a/lib/l10n/app_localizations_fr.dart +++ b/lib/l10n/app_localizations_fr.dart @@ -2935,6 +2935,10 @@ class AppLocalizationsFr extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_hu.dart b/lib/l10n/app_localizations_hu.dart index 190275c..3c72de8 100644 --- a/lib/l10n/app_localizations_hu.dart +++ b/lib/l10n/app_localizations_hu.dart @@ -2926,6 +2926,10 @@ class AppLocalizationsHu extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_it.dart b/lib/l10n/app_localizations_it.dart index a175f99..e8ba848 100644 --- a/lib/l10n/app_localizations_it.dart +++ b/lib/l10n/app_localizations_it.dart @@ -2919,6 +2919,10 @@ class AppLocalizationsIt extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_ja.dart b/lib/l10n/app_localizations_ja.dart index d31f6a3..6350be7 100644 --- a/lib/l10n/app_localizations_ja.dart +++ b/lib/l10n/app_localizations_ja.dart @@ -2799,6 +2799,10 @@ class AppLocalizationsJa extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_ko.dart b/lib/l10n/app_localizations_ko.dart index dbd50b5..3cb99bb 100644 --- a/lib/l10n/app_localizations_ko.dart +++ b/lib/l10n/app_localizations_ko.dart @@ -2797,6 +2797,10 @@ class AppLocalizationsKo extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_nl.dart b/lib/l10n/app_localizations_nl.dart index 686f34b..10e2bb6 100644 --- a/lib/l10n/app_localizations_nl.dart +++ b/lib/l10n/app_localizations_nl.dart @@ -2899,6 +2899,10 @@ class AppLocalizationsNl extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_pl.dart b/lib/l10n/app_localizations_pl.dart index 4e40642..b2a33a7 100644 --- a/lib/l10n/app_localizations_pl.dart +++ b/lib/l10n/app_localizations_pl.dart @@ -2927,6 +2927,10 @@ class AppLocalizationsPl extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_pt.dart b/lib/l10n/app_localizations_pt.dart index 422038f..8fbf97f 100644 --- a/lib/l10n/app_localizations_pt.dart +++ b/lib/l10n/app_localizations_pt.dart @@ -2911,6 +2911,10 @@ class AppLocalizationsPt extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_ru.dart b/lib/l10n/app_localizations_ru.dart index 03f237c..95040df 100644 --- a/lib/l10n/app_localizations_ru.dart +++ b/lib/l10n/app_localizations_ru.dart @@ -2917,6 +2917,10 @@ class AppLocalizationsRu extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_sk.dart b/lib/l10n/app_localizations_sk.dart index 55c6e02..d896b0b 100644 --- a/lib/l10n/app_localizations_sk.dart +++ b/lib/l10n/app_localizations_sk.dart @@ -2898,6 +2898,10 @@ class AppLocalizationsSk extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_sl.dart b/lib/l10n/app_localizations_sl.dart index 102f725..b1996fd 100644 --- a/lib/l10n/app_localizations_sl.dart +++ b/lib/l10n/app_localizations_sl.dart @@ -2897,6 +2897,10 @@ class AppLocalizationsSl extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_sv.dart b/lib/l10n/app_localizations_sv.dart index de72b5a..f118c64 100644 --- a/lib/l10n/app_localizations_sv.dart +++ b/lib/l10n/app_localizations_sv.dart @@ -2883,6 +2883,10 @@ class AppLocalizationsSv extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_uk.dart b/lib/l10n/app_localizations_uk.dart index 0953a80..2043089 100644 --- a/lib/l10n/app_localizations_uk.dart +++ b/lib/l10n/app_localizations_uk.dart @@ -2915,6 +2915,10 @@ class AppLocalizationsUk extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/l10n/app_localizations_zh.dart b/lib/l10n/app_localizations_zh.dart index 5882b65..610a991 100644 --- a/lib/l10n/app_localizations_zh.dart +++ b/lib/l10n/app_localizations_zh.dart @@ -2751,6 +2751,10 @@ class AppLocalizationsZh extends AppLocalizations { String get repeater_cliUnmatchedResponse => 'Unrequested response from repeater'; + @override + String get repeater_lateResponseDiscarded => + 'A late response arrived but was not applied, because you have unsaved changes.'; + @override String get repeater_lateResponseReceived => 'A late response arrived after the command timed out. See the CLI screen.'; diff --git a/lib/screens/repeater_settings_screen.dart b/lib/screens/repeater_settings_screen.dart index 1160daa..4e67510 100644 --- a/lib/screens/repeater_settings_screen.dart +++ b/lib/screens/repeater_settings_screen.dart @@ -194,12 +194,20 @@ class _RepeaterSettingsScreenState extends State { /// (#528). void _handleUnmatchedResponse(UnmatchedRepeaterResponse unmatched) { if (!mounted) return; - if (unmatched.command != null) { - _handleGetResponse(unmatched.command!, unmatched.response); - } + // A late reply is stale by definition. Applying it over edits the user + // made while waiting would silently discard their input, so it only lands + // when nothing is unsaved (Gemini review). + final applied = + !_hasChanges && + unmatched.command != null && + _handleGetResponse(unmatched.command!, unmatched.response); showDismissibleSnackBar( context, - content: Text(context.l10n.repeater_lateResponseReceived), + content: Text( + applied + ? context.l10n.repeater_lateResponseReceived + : context.l10n.repeater_lateResponseDiscarded, + ), ); } diff --git a/lib/services/repeater_command_service.dart b/lib/services/repeater_command_service.dart index c5faee7..e133d5e 100644 --- a/lib/services/repeater_command_service.dart +++ b/lib/services/repeater_command_service.dart @@ -166,6 +166,11 @@ class RepeaterCommandService { () { final completer = _pendingCommands[commandId]; if (completer != null && !completer.isCompleted) { + // Prune here as well as on receive. A run of commands that all + // time out with no traffic coming back would otherwise accumulate + // records until disposal, since handleResponse is the only other + // place that prunes and it never runs (Gemini review). + _pruneExpiredCommands(); // Remember what this prefix asked so a reply arriving after the // window can still reach the user with its question attached. _expiredCommands[prefix] = _ExpiredCommand( @@ -211,13 +216,7 @@ class RepeaterCommandService { // to matching by repeater. final matchedId = prefix != null ? _pendingByPrefix[prefix] : null; final commandId = - matchedId ?? - (prefix != null - ? '' - : _pendingCommands.keys.firstWhere( - (id) => id.startsWith(repeaterKey), - orElse: () => '', - )); + matchedId ?? (prefix != null ? '' : _solePendingFor(repeaterKey)); if (commandId.isNotEmpty) { final completer = _pendingCommands[commandId]; @@ -267,6 +266,28 @@ class RepeaterCommandService { ); } + /// The one command awaiting a reply from [repeaterKey], or `''` when that is + /// ambiguous. + /// + /// An unprefixed reply carries no correlation token, so position is all that + /// is left to match on. Firmware only echoes the prefix when the command is + /// longer than four characters including it (`simple_repeater/MyMesh.cpp` + /// `strlen(command) > 4 && command[2] == '|'`), so a very short command can + /// legitimately answer without one and the fallback has to stay. + /// + /// It is only sound with exactly one candidate though. With two or more, + /// "first" is map-iteration order, and choosing one would hand a caller + /// another command's output. Ambiguity is surfaced instead of guessed. + String _solePendingFor(String repeaterKey) { + String? only; + for (final id in _pendingCommands.keys) { + if (!id.startsWith(repeaterKey)) continue; + if (only != null) return ''; + only = id; + } + return only ?? ''; + } + Completer _registerPending(String commandId, String prefix) { final completer = Completer(); _pendingCommands[commandId] = completer; diff --git a/test/services/repeater_command_service_test.dart b/test/services/repeater_command_service_test.dart index 56039f4..c7913e3 100644 --- a/test/services/repeater_command_service_test.dart +++ b/test/services/repeater_command_service_test.dart @@ -221,4 +221,57 @@ void main() { expect(surfaced, isEmpty); }); }); + + group('unprefixed reply ambiguity (Gemini review)', () { + // #532 constrained PREFIXED replies. An unprefixed one still fell back to + // "first pending command for this repeater", which with two or more in + // flight is map-iteration order, so a caller could receive another + // command's output. + // + // The fallback cannot simply be deleted: firmware only echoes the prefix + // when the command exceeds four characters including it + // (simple_repeater/MyMesh.cpp, `strlen(command) > 4 && command[2] == '|'`), + // so a very short command legitimately replies without one. + + test( + 'with exactly one pending, an unprefixed reply still resolves it', + () async { + final only = service.registerPendingForTest( + repeater.publicKeyHex, + 'E1|', + ); + + service.handleResponse(repeater, 'bare payload'); + + expect(await only, 'bare payload'); + expect(surfaced, isEmpty); + }, + ); + + test('with two pending, an unprefixed reply resolves NEITHER', () async { + final first = service.registerPendingForTest( + repeater.publicKeyHex, + 'E1|', + ); + final second = service.registerPendingForTest( + repeater.publicKeyHex, + 'E2|', + ); + var firstDone = false, secondDone = false; + unawaited(first.then((_) => firstDone = true)); + unawaited(second.then((_) => secondDone = true)); + + service.handleResponse(repeater, 'ambiguous payload'); + await Future.delayed(Duration.zero); + + expect(firstDone, isFalse); + expect(secondDone, isFalse); + expect( + surfaced.single.response, + 'ambiguous payload', + reason: 'ambiguity must be surfaced, never guessed at', + ); + expect(surfaced.single.command, isNull); + }); + }); }