diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 8da0d72..6e0de1c 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -1566,6 +1566,19 @@ } } }, + "repeater_cliLateResponse": "Late response to \"{command}\" (arrived {seconds}s after timeout)", + "@repeater_cliLateResponse": { + "placeholders": { + "command": { + "type": "String" + }, + "seconds": { + "type": "String" + } + } + }, + "repeater_cliUnmatchedResponse": "Unrequested response from repeater", + "repeater_lateResponseReceived": "A late response arrived after the command timed out. See the CLI screen.", "repeater_cliQuickGetName": "Get Name", "repeater_cliQuickGetRadio": "Get Radio", "repeater_cliQuickGetTx": "Get TX", diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index 517fcca..ff7b326 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -5126,6 +5126,24 @@ abstract class AppLocalizations { /// **'Error: {error}'** String repeater_cliCommandError(String error); + /// No description provided for @repeater_cliLateResponse. + /// + /// In en, this message translates to: + /// **'Late response to \"{command}\" (arrived {seconds}s after timeout)'** + String repeater_cliLateResponse(String command, String seconds); + + /// No description provided for @repeater_cliUnmatchedResponse. + /// + /// In en, this message translates to: + /// **'Unrequested response from repeater'** + String get repeater_cliUnmatchedResponse; + + /// No description provided for @repeater_lateResponseReceived. + /// + /// In en, this message translates to: + /// **'A late response arrived after the command timed out. See the CLI screen.'** + String get repeater_lateResponseReceived; + /// No description provided for @repeater_cliQuickGetName. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_bg.dart b/lib/l10n/app_localizations_bg.dart index b80f289..bc3b38a 100644 --- a/lib/l10n/app_localizations_bg.dart +++ b/lib/l10n/app_localizations_bg.dart @@ -2905,6 +2905,19 @@ class AppLocalizationsBg extends AppLocalizations { return 'Грешка: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Получи име'; diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index 221a7cc..4796cc0 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -2908,6 +2908,19 @@ class AppLocalizationsDe extends AppLocalizations { return 'Fehler: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Name erhalten'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index b483034..a2f753c 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -2849,6 +2849,19 @@ class AppLocalizationsEn extends AppLocalizations { return 'Error: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Get Name'; diff --git a/lib/l10n/app_localizations_es.dart b/lib/l10n/app_localizations_es.dart index 8b09b37..ada7780 100644 --- a/lib/l10n/app_localizations_es.dart +++ b/lib/l10n/app_localizations_es.dart @@ -2899,6 +2899,19 @@ class AppLocalizationsEs extends AppLocalizations { return 'Error: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Obtener Nombre'; diff --git a/lib/l10n/app_localizations_fr.dart b/lib/l10n/app_localizations_fr.dart index 5545ce0..48cc33d 100644 --- a/lib/l10n/app_localizations_fr.dart +++ b/lib/l10n/app_localizations_fr.dart @@ -2921,6 +2921,19 @@ class AppLocalizationsFr extends AppLocalizations { return 'Erreur : $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Obtenir le nom'; diff --git a/lib/l10n/app_localizations_hu.dart b/lib/l10n/app_localizations_hu.dart index d3d4fff..c03f5cc 100644 --- a/lib/l10n/app_localizations_hu.dart +++ b/lib/l10n/app_localizations_hu.dart @@ -2912,6 +2912,19 @@ class AppLocalizationsHu extends AppLocalizations { return 'Hiba: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Kapcsold össze a nevet'; diff --git a/lib/l10n/app_localizations_it.dart b/lib/l10n/app_localizations_it.dart index 7ce690b..74f1361 100644 --- a/lib/l10n/app_localizations_it.dart +++ b/lib/l10n/app_localizations_it.dart @@ -2905,6 +2905,19 @@ class AppLocalizationsIt extends AppLocalizations { return 'Errore: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Ottieni Nome'; diff --git a/lib/l10n/app_localizations_ja.dart b/lib/l10n/app_localizations_ja.dart index e1f2f0b..736587e 100644 --- a/lib/l10n/app_localizations_ja.dart +++ b/lib/l10n/app_localizations_ja.dart @@ -2785,6 +2785,19 @@ class AppLocalizationsJa extends AppLocalizations { return 'エラー：$error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => '名前を取得する'; diff --git a/lib/l10n/app_localizations_ko.dart b/lib/l10n/app_localizations_ko.dart index 8efc350..0892237 100644 --- a/lib/l10n/app_localizations_ko.dart +++ b/lib/l10n/app_localizations_ko.dart @@ -2783,6 +2783,19 @@ class AppLocalizationsKo extends AppLocalizations { return '오류: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => '이름을 알려주세요'; diff --git a/lib/l10n/app_localizations_nl.dart b/lib/l10n/app_localizations_nl.dart index 20f2ef5..3e9e5bc 100644 --- a/lib/l10n/app_localizations_nl.dart +++ b/lib/l10n/app_localizations_nl.dart @@ -2885,6 +2885,19 @@ class AppLocalizationsNl extends AppLocalizations { return 'Fout: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Naam opvragen'; diff --git a/lib/l10n/app_localizations_pl.dart b/lib/l10n/app_localizations_pl.dart index 3587475..1eb1a6a 100644 --- a/lib/l10n/app_localizations_pl.dart +++ b/lib/l10n/app_localizations_pl.dart @@ -2913,6 +2913,19 @@ class AppLocalizationsPl extends AppLocalizations { return 'Błąd: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Pobierz nazwę'; diff --git a/lib/l10n/app_localizations_pt.dart b/lib/l10n/app_localizations_pt.dart index 6585573..ae4403e 100644 --- a/lib/l10n/app_localizations_pt.dart +++ b/lib/l10n/app_localizations_pt.dart @@ -2897,6 +2897,19 @@ class AppLocalizationsPt extends AppLocalizations { return 'Erro: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Obter Nome'; diff --git a/lib/l10n/app_localizations_ru.dart b/lib/l10n/app_localizations_ru.dart index fcdeea8..269db9b 100644 --- a/lib/l10n/app_localizations_ru.dart +++ b/lib/l10n/app_localizations_ru.dart @@ -2903,6 +2903,19 @@ class AppLocalizationsRu extends AppLocalizations { return 'Ошибка: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Получить имя'; diff --git a/lib/l10n/app_localizations_sk.dart b/lib/l10n/app_localizations_sk.dart index ce420bd..2acb38d 100644 --- a/lib/l10n/app_localizations_sk.dart +++ b/lib/l10n/app_localizations_sk.dart @@ -2884,6 +2884,19 @@ class AppLocalizationsSk extends AppLocalizations { return 'Chyba: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Zísť meno'; diff --git a/lib/l10n/app_localizations_sl.dart b/lib/l10n/app_localizations_sl.dart index 849e118..26e1fa6 100644 --- a/lib/l10n/app_localizations_sl.dart +++ b/lib/l10n/app_localizations_sl.dart @@ -2883,6 +2883,19 @@ class AppLocalizationsSl extends AppLocalizations { return 'Napaka: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Pridobi ime'; diff --git a/lib/l10n/app_localizations_sv.dart b/lib/l10n/app_localizations_sv.dart index d033982..4111819 100644 --- a/lib/l10n/app_localizations_sv.dart +++ b/lib/l10n/app_localizations_sv.dart @@ -2869,6 +2869,19 @@ class AppLocalizationsSv extends AppLocalizations { return 'Fel: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Hämta namn'; diff --git a/lib/l10n/app_localizations_uk.dart b/lib/l10n/app_localizations_uk.dart index 2e44f5f..997fe73 100644 --- a/lib/l10n/app_localizations_uk.dart +++ b/lib/l10n/app_localizations_uk.dart @@ -2901,6 +2901,19 @@ class AppLocalizationsUk extends AppLocalizations { return 'Помилка: $error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => 'Отримати ім\'я'; diff --git a/lib/l10n/app_localizations_zh.dart b/lib/l10n/app_localizations_zh.dart index 578575c..f4eadf1 100644 --- a/lib/l10n/app_localizations_zh.dart +++ b/lib/l10n/app_localizations_zh.dart @@ -2737,6 +2737,19 @@ class AppLocalizationsZh extends AppLocalizations { return '错误：$error'; } + @override + String repeater_cliLateResponse(String command, String seconds) { + return 'Late response to \"$command\" (arrived ${seconds}s after timeout)'; + } + + @override + String get repeater_cliUnmatchedResponse => + 'Unrequested response from repeater'; + + @override + String get repeater_lateResponseReceived => + 'A late response arrived after the command timed out. See the CLI screen.'; + @override String get repeater_cliQuickGetName => '获取名称'; diff --git a/lib/screens/repeater_cli_screen.dart b/lib/screens/repeater_cli_screen.dart index 10e325a..82875ac 100644 --- a/lib/screens/repeater_cli_screen.dart +++ b/lib/screens/repeater_cli_screen.dart @@ -52,9 +52,31 @@ class _RepeaterCliScreenState extends State { super.initState(); final connector = Provider.of(context, listen: false); _commandService = RepeaterCommandService(connector); + _commandService!.onUnmatchedResponse = _handleUnmatchedResponse; _setupMessageListener(); } + /// A reply the command future never received, because its window had already + /// closed. It is still the repeater's real answer, so it goes into the + /// history instead of being dropped (#528). + void _handleUnmatchedResponse(UnmatchedRepeaterResponse unmatched) { + if (!mounted) return; + setState(() { + _commandHistory.add({ + 'type': 'late', + 'text': unmatched.response, + 'command': unmatched.command ?? '', + 'seconds': unmatched.sinceTimeout == null + ? '' + : (unmatched.sinceTimeout!.inMilliseconds / 1000).toStringAsFixed( + 1, + ), + 'timestamp': DateTime.now().toString(), + }); + }); + _scrollToBottom(); + } + @override void dispose() { _frameSubscription?.cancel(); @@ -103,11 +125,10 @@ class _RepeaterCliScreenState extends State { if (parsed == null) return; if (!_matchesRepeaterPrefix(parsed.senderPrefix)) return; - // Notify command service of response (for retry handling) + // The service routes this either to the waiting command's future, which + // _sendCommand appends to history, or to onUnmatchedResponse when the + // window has already closed. Both paths reach the transcript (#528). _commandService?.handleResponse(widget.repeater, parsed.text); - - // Note: The command service will handle the response via the Future - // We don't need to add it to history here anymore as _sendCommand will do it } bool _matchesRepeaterPrefix(Uint8List prefix) { @@ -184,7 +205,19 @@ class _RepeaterCliScreenState extends State { _historyIndex = -1; _commandFocusNode.requestFocus(); - // Auto-scroll to bottom + _scrollToBottom(); + } + + String _lateResponseLabel(Map entry) { + final command = entry['command'] ?? ''; + final seconds = entry['seconds'] ?? ''; + if (command.isEmpty || seconds.isEmpty) { + return context.l10n.repeater_cliUnmatchedResponse; + } + return context.l10n.repeater_cliLateResponse(command, seconds); + } + + void _scrollToBottom() { Future.delayed(const Duration(milliseconds: 100), () { if (_scrollController.hasClients) { _scrollController.animateTo( @@ -450,6 +483,25 @@ class _RepeaterCliScreenState extends State { itemBuilder: (context, index) { final entry = _commandHistory[index]; final isCommand = entry['type'] == 'command'; + final isLate = entry['type'] == 'late'; + final scheme = Theme.of(context).colorScheme; + + final Color badgeColor; + final Color badgeIconColor; + final IconData badgeIcon; + if (isCommand) { + badgeColor = scheme.primaryContainer; + badgeIconColor = scheme.onPrimaryContainer; + badgeIcon = Icons.chevron_right; + } else if (isLate) { + badgeColor = scheme.tertiaryContainer; + badgeIconColor = scheme.onTertiaryContainer; + badgeIcon = Icons.history; + } else { + badgeColor = scheme.secondaryContainer; + badgeIconColor = scheme.onSecondaryContainer; + badgeIcon = Icons.arrow_back; + } return Padding( padding: const EdgeInsets.only(bottom: 12), @@ -459,32 +511,34 @@ class _RepeaterCliScreenState extends State { Container( padding: const EdgeInsets.all(6), decoration: BoxDecoration( - color: isCommand - ? Theme.of(context).colorScheme.primaryContainer - : Theme.of(context).colorScheme.secondaryContainer, + color: badgeColor, borderRadius: BorderRadius.circular(4), ), - child: Icon( - isCommand ? Icons.chevron_right : Icons.arrow_back, - size: 16, - color: isCommand - ? Theme.of(context).colorScheme.onPrimaryContainer - : Theme.of(context).colorScheme.onSecondaryContainer, - ), + child: Icon(badgeIcon, size: 16, color: badgeIconColor), ), const SizedBox(width: 12), Expanded( child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ + if (isLate) + Padding( + padding: const EdgeInsets.only(bottom: 2), + child: Text( + _lateResponseLabel(entry), + style: TextStyle( + fontSize: 11, + fontStyle: FontStyle.italic, + color: scheme.tertiary, + ), + ), + ), SelectableText( entry['text']!, style: TextStyle( fontFamily: 'monospace', fontSize: 13, - color: isCommand - ? Theme.of(context).colorScheme.primary - : Theme.of(context).colorScheme.onSurface, + color: isCommand ? scheme.primary : scheme.onSurface, ), ), ], diff --git a/lib/screens/repeater_settings_screen.dart b/lib/screens/repeater_settings_screen.dart index 8e5c892..1160daa 100644 --- a/lib/screens/repeater_settings_screen.dart +++ b/lib/screens/repeater_settings_screen.dart @@ -183,10 +183,26 @@ class _RepeaterSettingsScreenState extends State { super.initState(); final connector = Provider.of(context, listen: false); _commandService = RepeaterCommandService(connector); + _commandService!.onUnmatchedResponse = _handleUnmatchedResponse; _setupMessageListener(); _loadSettings(); } + /// A repeater reply whose command had already timed out. This screen has no + /// transcript to append it to, so it is applied like any other response and + /// the user is told it arrived late rather than the reply being dropped + /// (#528). + void _handleUnmatchedResponse(UnmatchedRepeaterResponse unmatched) { + if (!mounted) return; + if (unmatched.command != null) { + _handleGetResponse(unmatched.command!, unmatched.response); + } + showDismissibleSnackBar( + context, + content: Text(context.l10n.repeater_lateResponseReceived), + ); + } + @override void dispose() { _frameSubscription?.cancel(); diff --git a/lib/services/repeater_command_service.dart b/lib/services/repeater_command_service.dart index 61d22c8..be074f8 100644 --- a/lib/services/repeater_command_service.dart +++ b/lib/services/repeater_command_service.dart @@ -1,8 +1,40 @@ import 'dart:async'; +import 'package:flutter/foundation.dart'; import '../models/contact.dart'; import '../models/path_selection.dart'; import '../connector/meshcore_connector.dart'; import '../connector/meshcore_protocol.dart'; +import '../utils/app_logger.dart'; + +/// A repeater reply that arrived with no command waiting to receive it. +/// +/// [command] carries the original request when the reply could be traced back +/// to one that already timed out, and is null when the reply cannot be +/// attributed to anything this service sent. +class UnmatchedRepeaterResponse { + final String repeaterKeyHex; + final String response; + final String? command; + final Duration? sinceTimeout; + + const UnmatchedRepeaterResponse({ + required this.repeaterKeyHex, + required this.response, + this.command, + this.sinceTimeout, + }); + + /// True when this answers a request that timed out rather than arriving + /// unsolicited. + bool get isLateReply => command != null; +} + +class _ExpiredCommand { + final String command; + final DateTime expiredAt; + + const _ExpiredCommand({required this.command, required this.expiredAt}); +} class RepeaterCommandService { final MeshCoreConnector _connector; @@ -10,10 +42,20 @@ class RepeaterCommandService { final Map _commandTimeouts = {}; final Map _commandPrefixes = {}; final Map _pendingByPrefix = {}; + final Map _expiredCommands = {}; int _prefixCounter = 0; static const int maxRetries = 5; + /// How long a timed-out command stays remembered so a reply arriving after + /// its window can still be presented with the request it answers. + static const Duration lateReplyRetention = Duration(minutes: 2); + + /// Invoked when a reply cannot be handed to a waiting command. Consumers + /// must surface this to the user: the reply is a real answer from the + /// repeater and dropping it silently loses it for good (#528). + void Function(UnmatchedRepeaterResponse)? onUnmatchedResponse; + RepeaterCommandService(this._connector); /// Send a CLI command to a repeater with automatic retries @@ -27,6 +69,7 @@ class RepeaterCommandService { }) async { final attemptCount = retries < 1 ? 1 : retries; final selection = await _connector.preparePathForContactSend(repeater); + final attemptPrefixes = []; for (int attempt = 0; attempt < attemptCount; attempt++) { onAttempt?.call(attempt + 1); @@ -36,7 +79,13 @@ class RepeaterCommandService { command, selection, attempt, + attemptPrefixes, ); + // The caller has its answer, so a straggler from an earlier attempt of + // this same command is noise rather than a lost response. + for (final prefix in attemptPrefixes) { + _expiredCommands.remove(prefix); + } onResponse?.call(response); return response; } catch (e) { @@ -52,6 +101,7 @@ class RepeaterCommandService { String command, PathSelection selection, int attempt, + List attemptPrefixes, ) async { final repeaterKey = repeater.publicKeyHex; final prefix = _nextPrefixToken(); @@ -60,6 +110,7 @@ class RepeaterCommandService { _pendingCommands[commandId] = completer; _commandPrefixes[commandId] = prefix; _pendingByPrefix[prefix] = commandId; + attemptPrefixes.add(prefix); try { final framedCommand = '$prefix$command'; @@ -93,6 +144,12 @@ class RepeaterCommandService { () { final completer = _pendingCommands[commandId]; if (completer != null && !completer.isCompleted) { + // Remember what this prefix asked so a reply arriving after the + // window can still reach the user with its question attached. + _expiredCommands[prefix] = _ExpiredCommand( + command: command, + expiredAt: DateTime.now(), + ); completer.completeError( 'Command timeout after $timeoutSeconds seconds', ); @@ -114,29 +171,89 @@ class RepeaterCommandService { /// Call this when a text message response is received from a repeater void handleResponse(Contact repeater, String responseText) { - // Find pending command for this repeater and complete it final repeaterKey = repeater.publicKeyHex; + _pruneExpiredCommands(); - String? commandId; + String? prefix; String responsePayload = responseText; if (responseText.length >= 3 && responseText[2] == '|') { - final prefix = responseText.substring(0, 3); - commandId = _pendingByPrefix[prefix]; + prefix = responseText.substring(0, 3); responsePayload = responseText.substring(3).trimLeft(); } - commandId ??= _pendingCommands.keys.firstWhere( - (id) => id.startsWith(repeaterKey), - orElse: () => '', - ); + final matchedId = prefix != null ? _pendingByPrefix[prefix] : null; + final commandId = + matchedId ?? + _pendingCommands.keys.firstWhere( + (id) => id.startsWith(repeaterKey), + orElse: () => '', + ); + + if (commandId.isNotEmpty) { + final completer = _pendingCommands[commandId]; + if (completer != null && !completer.isCompleted) { + completer.complete(responsePayload); + _cleanup(commandId); + return; + } + } + + // Nothing is waiting for this reply. It is still a real answer from the + // repeater, so it gets surfaced and logged rather than dropped (#528). + _surfaceUnmatchedResponse(repeaterKey, prefix, responsePayload); + } - if (commandId.isEmpty) return; + void _surfaceUnmatchedResponse( + String repeaterKey, + String? prefix, + String responsePayload, + ) { + final expired = prefix != null ? _expiredCommands.remove(prefix) : null; + final sinceTimeout = expired == null + ? null + : DateTime.now().difference(expired.expiredAt); - final completer = _pendingCommands[commandId]; - if (completer != null && !completer.isCompleted) { - completer.complete(responsePayload); - _cleanup(commandId); + if (expired != null) { + appLogger.warn( + 'Late reply to "${expired.command}" from $repeaterKey arrived ' + '${sinceTimeout!.inMilliseconds}ms after its window closed', + tag: 'RepeaterCommand', + ); + } else { + appLogger.warn( + 'Reply from $repeaterKey matched no pending or recently expired ' + 'command (prefix: ${prefix ?? 'none'})', + tag: 'RepeaterCommand', + ); } + + onUnmatchedResponse?.call( + UnmatchedRepeaterResponse( + repeaterKeyHex: repeaterKey, + response: responsePayload, + command: expired?.command, + sinceTimeout: sinceTimeout, + ), + ); + } + + /// Records a timed-out command so a reply arriving later can still be + /// attributed to it. Exposed because a test cannot drive a real + /// send-then-time-out cycle without a connected transport. + @visibleForTesting + void recordExpiredCommandForTest(String prefix, String command) { + _expiredCommands[prefix] = _ExpiredCommand( + command: command, + expiredAt: DateTime.now(), + ); + } + + void _pruneExpiredCommands() { + if (_expiredCommands.isEmpty) return; + final cutoff = DateTime.now().subtract(lateReplyRetention); + _expiredCommands.removeWhere( + (_, entry) => entry.expiredAt.isBefore(cutoff), + ); } void _cleanup(String commandId) { @@ -157,6 +274,8 @@ class RepeaterCommandService { _pendingCommands.clear(); _commandPrefixes.clear(); _pendingByPrefix.clear(); + _expiredCommands.clear(); + onUnmatchedResponse = null; } String _nextPrefixToken() { diff --git a/test/services/repeater_command_service_test.dart b/test/services/repeater_command_service_test.dart new file mode 100644 index 0000000..df75aa8 --- /dev/null +++ b/test/services/repeater_command_service_test.dart @@ -0,0 +1,117 @@ +// #528 (epic #473): a repeater CLI reply that arrives with no command waiting +// for it used to hit `if (commandId.isEmpty) return;` and vanish. No log, no +// UI, nothing. Since a command's window can close before the app has even +// finished fetching the message (MSG_WAITING then SYNC_NEXT_MESSAGE), that +// made "the command ran but the response was never reported" the normal +// outcome rather than an edge case. +// +// These tests pin the contract that no reply is ever dropped silently. + +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/connector/meshcore_connector.dart'; +import 'package:meshcore_open/models/contact.dart'; +import 'package:meshcore_open/services/repeater_command_service.dart'; +import 'package:meshcore_open/storage/prefs_manager.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +Contact _repeater() => Contact( + publicKey: Uint8List.fromList(List.generate(32, (i) => i)), + name: 'Test Repeater', + type: 2, + pathLength: 0, + path: Uint8List(0), + lastSeen: DateTime.fromMillisecondsSinceEpoch(0), +); + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late RepeaterCommandService service; + late Contact repeater; + late List surfaced; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + PrefsManager.reset(); + await PrefsManager.initialize(); + + service = RepeaterCommandService(MeshCoreConnector()); + repeater = _repeater(); + surfaced = []; + service.onUnmatchedResponse = surfaced.add; + }); + + tearDown(() => service.dispose()); + + test('a reply with no pending command is surfaced, not discarded', () { + service.handleResponse(repeater, 'Version: 1.2.3'); + + expect(surfaced, hasLength(1)); + expect(surfaced.single.response, 'Version: 1.2.3'); + expect(surfaced.single.repeaterKeyHex, repeater.publicKeyHex); + }); + + test('a reply for a command that already timed out names that command', () { + service.recordExpiredCommandForTest('A3|', 'ver'); + + service.handleResponse(repeater, 'A3|Version: 1.2.3'); + + expect(surfaced, hasLength(1)); + final reply = surfaced.single; + expect(reply.isLateReply, isTrue); + expect(reply.command, 'ver'); + expect(reply.response, 'Version: 1.2.3', reason: 'prefix must be stripped'); + expect(reply.sinceTimeout, isNotNull); + }); + + test('an unattributable reply is still surfaced, just without a command', () { + service.handleResponse(repeater, 'ZZ|unsolicited chatter'); + + expect(surfaced, hasLength(1)); + expect(surfaced.single.isLateReply, isFalse); + expect(surfaced.single.command, isNull); + expect(surfaced.single.response, 'unsolicited chatter'); + }); + + test('an expired command is only consumed once', () { + service.recordExpiredCommandForTest('A3|', 'ver'); + + service.handleResponse(repeater, 'A3|first'); + service.handleResponse(repeater, 'A3|second'); + + expect(surfaced, hasLength(2)); + expect(surfaced[0].command, 'ver'); + expect( + surfaced[1].command, + isNull, + reason: 'the record is consumed by the first reply that claims it', + ); + }); + + test('a reply is never swallowed when no callback is wired', () { + service.onUnmatchedResponse = null; + + // The contract is that this cannot throw and cannot hang. The logging path + // still runs; the absence of a listener must not resurrect the silent drop + // as an unhandled error. + expect( + () => service.handleResponse(repeater, 'orphan reply'), + returnsNormally, + ); + }); + + test('dispose clears the late-reply records', () { + service.recordExpiredCommandForTest('A3|', 'ver'); + service.dispose(); + + // Re-arm a listener on the disposed service and confirm the stale record is + // gone, so a reply after disposal cannot be mis-attributed to it. + final seen = []; + service.onUnmatchedResponse = seen.add; + service.handleResponse(repeater, 'A3|Version: 1.2.3'); + + expect(seen.single.command, isNull); + }); +}