diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 48427c75..acce36d6 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -211,6 +211,25 @@ enum IdentityTransfer { noReply, } +/// How a channel write (`CMD_SET_CHANNEL`) ended, as the radio reported it. +/// (#1026) +enum ChannelWrite { + /// The radio answered OK. + ok, + + /// The radio refused because a different channel holds the slot + /// (`ERR_CODE_BAD_STATE`, OffbandMesh/meshcore-firmware#1864). That channel + /// is still there, intact. + occupied, + + /// The radio refused for another reason, e.g. a slot index it does not have. + rejected, + + /// Not connected, or no reply before the timeout. The write may or may not + /// have landed. + noReply, +} + /// Result of [MeshCoreConnector.exportPrivateKey]. [identity] is /// non-null only when [outcome] is [IdentityTransfer.ok]. (#578) class IdentityExportResult { @@ -7114,32 +7133,17 @@ class MeshCoreConnector extends ChangeNotifier { } } - Future setChannel(int index, String name, Uint8List psk) async { - if (!isConnected) return; - - // Persisted channel history is keyed by the channel's PSK identity (#194), - // so reassigning this slot to a different channel cannot surface the old - // occupant's messages THERE. Two slot-keyed remnants still can, and both - // are handled when the slot changes occupant (different PSK): - // - legacy slot-index keys (pre-#194) would be adopted onto the new - // occupant by the store's one-time migration; purge them (#193/#1063). - // - the in-memory conversation map is slot-keyed; drop this slot's entry. - // The old occupant's PSK-keyed history is deliberately left intact: if - // that channel is ever re-added (any slot), its history comes back - // (#1063). A rename (same PSK) keeps everything. - final existing = _findChannelByIndex(index); - final sameChannel = existing != null && listEquals(existing.psk, psk); + /// How long a channel write waits for the radio's OK or ERR. (#1026) + @visibleForTesting + Duration channelWriteReplyTimeout = const Duration(seconds: 5); - await sendFrame(buildSetChannelFrame(index, name, psk)); + int _channelWritesInFlight = 0; - if (!sameChannel) { - await _channelMessageStore.clearLegacySlotHistory(index); - _channelMessages.remove(index); - } - - // Refresh channels after setting - await getChannels(force: true); - } + /// True while the channel list cannot be trusted to show which slots are + /// free: a sync is re-reading it, or a write has not been answered yet. A + /// slot chosen against it could already be taken. (#1021, #1026) + bool get isChannelTableBusy => + _isSyncingChannels || _channelWritesInFlight > 0; /// Writes the client's cached channel list back onto the radio (#1060). /// @@ -7165,47 +7169,222 @@ class MeshCoreConnector extends ChangeNotifier { tag: 'ChannelSync', ); - var sent = 0; - for (final channel in toRestore) { - if (!isConnected) break; - await sendFrame( - buildSetChannelFrame(channel.index, channel.name, channel.psk), + // Counted as a write in flight for its whole length, so an add made while + // it runs is refused instead of choosing a slot this is about to fill + // (#1021, #1026). + _channelWritesInFlight++; + try { + var sent = 0; + for (final channel in toRestore) { + if (!isConnected) break; + await sendFrame( + buildSetChannelFrame(channel.index, channel.name, channel.psk), + ); + sent++; + // Space the writes. On BLE [sendFrame] prefers write-without-response, + // which returns once the frame reaches the OS and not once the radio + // has taken it, so an unpaced loop can overrun the radio and lose + // channels with no error anywhere. The USB branch of [sendFrame] + // already spaces its own frames for exactly this reason; BLE has no + // such pause. + if (sent < toRestore.length) { + await Future.delayed(_channelRestoreFrameGap); + } + } + + // The radio now holds these again, so a fresh sync should confirm them + // and the erased-slot block clears itself on the next pass. + // + // This deliberately does NOT report how many the radio confirmed: + // [getChannels] returns once the first request is away, not once the + // sync has finished, so counting confirmations here would read a list + // the radio has not answered for yet and report 0 every time. The + // authoritative feedback is the erased-slot banner, which is re-derived + // on every sync and therefore stays up if some writes did not land. + await getChannels(force: true); + return sent; + } finally { + _channelWritesInFlight--; + } + } + + /// Writes [name] and [psk] into slot [index] and reports what the radio said. + /// + /// [refresh] re-reads the channel list afterwards. A caller writing several + /// channels passes false and refreshes once at the end, because a write sent + /// during a sync cannot tell its reply from the sync's. + Future setChannel( + int index, + String name, + Uint8List psk, { + bool refresh = true, + }) async { + if (!isConnected) return ChannelWrite.noReply; + return _setChannel( + index, + name, + psk, + occupantPsk: _findChannelByIndex(index)?.psk, + refresh: refresh, + ); + } + + /// Writes an edit of [channel]. A rename keeps the key and is a plain write. + /// A new key is clear-then-add: firmware with + /// OffbandMesh/meshcore-firmware#1864 refuses a different channel over an + /// occupied slot, and that is what a new key in place is. + /// + /// A new key is a slot reassignment for history purposes, the same as + /// before the clear-then-add: the old key's PSK-keyed history is kept, so + /// it comes back if that key is ever re-added (#1063). + Future editChannel( + Channel channel, + String name, + Uint8List psk, + ) async { + if (!isConnected) return ChannelWrite.noReply; + if (listEquals(channel.psk, psk)) { + return _setChannel( + channel.index, + name, + psk, + occupantPsk: channel.psk, + refresh: true, ); - sent++; - // Space the writes. On BLE [sendFrame] prefers write-without-response, - // which returns once the frame reaches the OS and not once the radio has - // taken it, so an unpaced loop can overrun the radio and lose channels - // with no error anywhere. The USB branch of [sendFrame] already spaces - // its own frames for exactly this reason; BLE has no such pause. - if (sent < toRestore.length) { - await Future.delayed(_channelRestoreFrameGap); + } + _channelWritesInFlight++; + try { + final cleared = await _writeChannelSlot(channel.index, '', Uint8List(16)); + if (cleared != ChannelWrite.ok) { + await getChannels(force: true); + return cleared; } + await _retireSlotOccupant(channel.index); + return await _setChannel( + channel.index, + name, + psk, + occupantPsk: null, + refresh: true, + ); + } finally { + _channelWritesInFlight--; } + } - // The radio now holds these again, so a fresh sync should confirm them and - // the erased-slot block clears itself on the next pass. - // - // This deliberately does NOT report how many the radio confirmed: - // [getChannels] returns once the first request is away, not once the sync - // has finished, so counting confirmations here would read a list the radio - // has not answered for yet and report 0 every time. The authoritative - // feedback is the erased-slot banner, which is re-derived on every sync and - // therefore stays up if some writes did not land. - await getChannels(force: true); - return sent; + Future deleteChannel(int index, {bool refresh = true}) async { + if (!isConnected) return ChannelWrite.noReply; + _channelWritesInFlight++; + try { + // A slot is deleted by writing an empty name and a zero PSK. Its history + // goes outright (the true-delete path, #1063), once the radio has said + // OK: a refused delete leaves the channel, and it keeps its history. + final result = await _writeChannelSlot(index, '', Uint8List(16)); + if (result == ChannelWrite.ok) { + await _channelMessageStore.clearChannelMessages(index); + _channelMessages.remove(index); + } + if (refresh) await getChannels(force: true); + return result; + } finally { + _channelWritesInFlight--; + } } - Future deleteChannel(int index) async { - if (!isConnected) return; + Future _setChannel( + int index, + String name, + Uint8List psk, { + required Uint8List? occupantPsk, + required bool refresh, + }) async { + _channelWritesInFlight++; + try { + // A rename (same PSK) keeps everything; a different PSK retires the + // slot's previous occupant (see [_retireSlotOccupant]). + final sameChannel = occupantPsk != null && listEquals(occupantPsk, psk); + + final result = await _writeChannelSlot(index, name, psk); + + // Only after the radio says OK. A refused write leaves the slot's + // channel in place, and its slot-keyed state has to stay with it. (#1026) + if (result == ChannelWrite.ok && !sameChannel) { + await _retireSlotOccupant(index); + } + + // Re-read on every outcome: after a refusal the list we chose the slot + // from was wrong, and after no reply nobody knows what the slot holds. + if (refresh) await getChannels(force: true); + return result; + } finally { + _channelWritesInFlight--; + } + } + + /// Sends one `CMD_SET_CHANNEL` and waits for the radio's OK or ERR, using + /// the generic-ack queue the channel-message sends already use. + /// + /// That queue matches replies to commands by order, which holds because the + /// firmware answers commands one at a time in the order they arrive. + Future _writeChannelSlot( + int index, + String name, + Uint8List psk, + ) async { + if (!isConnected) return ChannelWrite.noReply; + final reply = Completer(); + final pending = _PendingCommandAck( + commandCode: cmdSetChannel, + reply: reply, + ); + // Queued before the frame goes out, so a fast reply cannot arrive first. + _pendingGenericAckQueue.add(pending); + try { + await sendFrame(buildSetChannelFrame(index, name, psk)); + } catch (e) { + _pendingGenericAckQueue.remove(pending); + _appDebugLogService?.error( + 'Channel write to slot $index was not sent: $e', + tag: 'ChannelWrite', + ); + return ChannelWrite.noReply; + } + final int? errCode; + try { + errCode = await reply.future.timeout(channelWriteReplyTimeout); + } on TimeoutException { + // Dropped so a late or lost reply cannot be matched to the next command. + _pendingGenericAckQueue.remove(pending); + _appDebugLogService?.warn( + 'Radio did not answer the channel write to slot $index', + tag: 'ChannelWrite', + ); + return ChannelWrite.noReply; + } + if (errCode == null) return ChannelWrite.ok; + _appDebugLogService?.warn( + 'Radio refused the channel write to slot $index (error $errCode)', + tag: 'ChannelWrite', + ); + return errCode == errCodeBadState + ? ChannelWrite.occupied + : ChannelWrite.rejected; + } - // Delete by setting empty name and zero PSK - await sendFrame(buildSetChannelFrame(index, '', Uint8List(16))); - // Clear stored messages for this channel - await _channelMessageStore.clearChannelMessages(index); - // Clear in-memory messages for this channel + /// Drops the slot-keyed remnants of a slot's previous occupant once the slot + /// has been given to a different channel. + /// + /// Persisted channel history is keyed by the channel's PSK identity (#194), + /// so reassigning this slot to a different channel cannot surface the old + /// occupant's messages THERE. Two slot-keyed remnants still can: + /// - legacy slot-index keys (pre-#194) would be adopted onto the new + /// occupant by the store's one-time migration; purge them (#193/#1063). + /// - the in-memory conversation map is slot-keyed; drop this slot's entry. + /// The old occupant's PSK-keyed history is deliberately left intact: if + /// that channel is ever re-added (any slot), its history comes back (#1063). + Future _retireSlotOccupant(int index) async { + await _channelMessageStore.clearLegacySlotHistory(index); _channelMessages.remove(index); - // Refresh channels after deleting - await getChannels(force: true); } // --- Offband GPS query (CMD_OFFBAND_GPS / RESP_CODE_OFFBAND_GPS = 0xC1) (#135) @@ -8421,6 +8600,7 @@ class MeshCoreConnector extends ChangeNotifier { } final failedAck = _pendingGenericAckQueue.removeAt(0); + failedAck.answer(errCode); if (failedAck.commandCode != cmdSendChannelTxtMsg || failedAck.channelSendQueueId == null) { return; @@ -10379,6 +10559,7 @@ class MeshCoreConnector extends ChangeNotifier { } final pendingAck = _pendingGenericAckQueue.removeAt(0); + pendingAck.answer(null); if (pendingAck.commandCode != cmdSendChannelTxtMsg || pendingAck.channelSendQueueId == null) { return; @@ -11957,7 +12138,20 @@ class _PendingCommandAck { final int commandCode; final String? channelSendQueueId; - _PendingCommandAck({required this.commandCode, this.channelSendQueueId}); + /// Completed with null for OK or with the error code for ERR, for a caller + /// that waits on the reply. (#1026) + final Completer? reply; + + _PendingCommandAck({ + required this.commandCode, + this.channelSendQueueId, + this.reply, + }); + + void answer(int? errCode) { + final waiter = reply; + if (waiter != null && !waiter.isCompleted) waiter.complete(errCode); + } } /// A `0xC5` request waiting for its reply. diff --git a/lib/connector/meshcore_protocol.dart b/lib/connector/meshcore_protocol.dart index f27a9aa9..937dd9a8 100644 --- a/lib/connector/meshcore_protocol.dart +++ b/lib/connector/meshcore_protocol.dart @@ -502,6 +502,11 @@ const int errCodeUnsupportedCmd = 1; /// not hold. (#762) const int errCodeNotFound = 2; +/// `ERR_CODE_BAD_STATE`: e.g. `CMD_SET_CHANNEL` putting a different channel +/// over an occupied slot, which firmware with OffbandMesh/meshcore-firmware#1864 +/// refuses. (#1026) +const int errCodeBadState = 4; + Uint8List buildOffbandBlockAddFrame(Uint8List pubKey) => Uint8List.fromList([cmdOffbandBlock, offbandBlockAdd, ...pubKey]); Uint8List buildOffbandBlockRemoveFrame(Uint8List pubKey) => diff --git a/lib/helpers/channel_slot_decision.dart b/lib/helpers/channel_slot_decision.dart index 5bca5ab2..ee481a15 100644 --- a/lib/helpers/channel_slot_decision.dart +++ b/lib/helpers/channel_slot_decision.dart @@ -38,9 +38,10 @@ class ChannelSlotDecision { /// handed slot 0 as a consolation, which is the old `return 0` bug and is /// where Public normally lives. /// -/// Callers pass the connector's `isSyncingChannels`, not `hasLoadedChannels`: -/// the latter stays false after a *failed* sync while the cached list is -/// restored into use, so gating on it would refuse forever on a flaky link. +/// Callers pass the connector's `isChannelTableBusy` (a sync, or a write the +/// radio has not answered yet), not `hasLoadedChannels`: the latter stays false +/// after a *failed* sync while the cached list is restored into use, so gating +/// on it would refuse forever on a flaky link. /// /// An absent index genuinely means a free slot, because the connector never /// adds an empty channel to its list. diff --git a/lib/helpers/channel_slot_ui.dart b/lib/helpers/channel_slot_ui.dart index c8a7f5c0..95a6e702 100644 --- a/lib/helpers/channel_slot_ui.dart +++ b/lib/helpers/channel_slot_ui.dart @@ -29,10 +29,11 @@ int? allocateChannelSlotOrExplain( final decision = decideChannelSlot( channels: connector.channels, maxChannels: connector.maxChannels, - // isSyncingChannels, not hasLoadedChannels: the latter stays false after a - // FAILED sync while the cached list is restored, so gating on it would make - // adding a channel impossible on a flaky link. - isSyncing: connector.isSyncingChannels, + // Not hasLoadedChannels: that stays false after a FAILED sync while the + // cached list is restored, so gating on it would make adding a channel + // impossible on a flaky link. Busy also covers a write the radio has not + // answered yet (#1026): until it does, the list does not show its slot. + isSyncing: connector.isChannelTableBusy, ); switch (decision.refusal) { case ChannelSlotRefusal.syncing: @@ -57,3 +58,36 @@ int? allocateChannelSlotOrExplain( void showChannelBlockedMessage(BuildContext context, String message) { showDismissibleSnackBar(context, content: Text(message), persist: true); } + +/// Explains a channel write the radio did not confirm, and returns true if it +/// did. A caller shows its own success message only when this returns false, +/// so nothing claims success before the radio has said OK (#1026). +/// +/// A success toast shown after this would replace the explanation (a newer +/// toast replaces the visible one), so callers must not show one. +bool explainUnconfirmedChannelWrite(BuildContext context, ChannelWrite result) { + final l10n = context.l10n; + final message = switch (result) { + ChannelWrite.ok => null, + ChannelWrite.occupied => l10n.channels_writeOccupied, + ChannelWrite.rejected => l10n.channels_writeRejected, + ChannelWrite.noReply => l10n.channels_writeNoReply, + }; + if (message == null) return false; + showChannelBlockedMessage(context, message); + return true; +} + +/// For an edit or delete: refuses while the channel list is being re-read or +/// a write is unanswered, and says so. Returns true if it refused. +/// +/// Replies are matched to commands by order, so a write sent during a sync +/// could take one of the sync's replies as its own. +bool refuseWhileChannelTableBusy( + BuildContext context, + MeshCoreConnector connector, +) { + if (!connector.isChannelTableBusy) return false; + showChannelBlockedMessage(context, context.l10n.channels_slotBusySyncing); + return true; +} diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index a1126c8e..6897ba03 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -1030,7 +1030,7 @@ } }, "channels_publicChannelAdded": "Public channel added", - "channels_slotBusySyncing": "Still reading channels from the radio. Try adding again in a moment.", + "channels_slotBusySyncing": "The radio is still updating its channel list. Try again in a moment.", "channels_slotsAllFull": "All {count} channel slots are in use. Delete a channel first.", "@channels_slotsAllFull": { "placeholders": { @@ -1040,6 +1040,18 @@ } }, "channels_publicAlreadyAdded": "The Public channel is already on this radio.", + "channels_writeOccupied": "The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.", + "@channels_writeOccupied": { + "description": "The radio refused a channel write because a different channel holds the slot (ERR_CODE_BAD_STATE, firmware #1864). Persistent (#1026)." + }, + "channels_writeRejected": "The radio refused the change, so the channel is as it was.", + "@channels_writeRejected": { + "description": "The radio answered a channel write with an error other than slot-occupied. Persistent (#1026)." + }, + "channels_writeNoReply": "The radio did not confirm the change. Check the channel list before trying again.", + "@channels_writeNoReply": { + "description": "A channel write got no reply before the timeout, or the radio is not connected; it may or may not have landed. Persistent (#1026)." + }, "channels_sortBy": "Sort by", "channels_sortManual": "Manual", "channels_sortAZ": "A-Z", diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index 7c326819..4d1430a2 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -3223,7 +3223,7 @@ abstract class AppLocalizations { /// No description provided for @channels_slotBusySyncing. /// /// In en, this message translates to: - /// **'Still reading channels from the radio. Try adding again in a moment.'** + /// **'The radio is still updating its channel list. Try again in a moment.'** String get channels_slotBusySyncing; /// No description provided for @channels_slotsAllFull. @@ -3238,6 +3238,24 @@ abstract class AppLocalizations { /// **'The Public channel is already on this radio.'** String get channels_publicAlreadyAdded; + /// The radio refused a channel write because a different channel holds the slot (ERR_CODE_BAD_STATE, firmware #1864). Persistent (#1026). + /// + /// In en, this message translates to: + /// **'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'** + String get channels_writeOccupied; + + /// The radio answered a channel write with an error other than slot-occupied. Persistent (#1026). + /// + /// In en, this message translates to: + /// **'The radio refused the change, so the channel is as it was.'** + String get channels_writeRejected; + + /// A channel write got no reply before the timeout, or the radio is not connected; it may or may not have landed. Persistent (#1026). + /// + /// In en, this message translates to: + /// **'The radio did not confirm the change. Check the channel list before trying again.'** + String get channels_writeNoReply; + /// No description provided for @channels_sortBy. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_bg.dart b/lib/l10n/app_localizations_bg.dart index 150fc508..b329bbe7 100644 --- a/lib/l10n/app_localizations_bg.dart +++ b/lib/l10n/app_localizations_bg.dart @@ -1828,7 +1828,7 @@ class AppLocalizationsBg extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1839,6 +1839,18 @@ class AppLocalizationsBg extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Сортирай по'; diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index 3e135b4a..6fef73bf 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -1823,7 +1823,7 @@ class AppLocalizationsDe extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1834,6 +1834,18 @@ class AppLocalizationsDe extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sortiere nach'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index 7e9f11fa..8527a688 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -1801,7 +1801,7 @@ class AppLocalizationsEn extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1812,6 +1812,18 @@ class AppLocalizationsEn extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sort by'; diff --git a/lib/l10n/app_localizations_es.dart b/lib/l10n/app_localizations_es.dart index a5235000..4c559480 100644 --- a/lib/l10n/app_localizations_es.dart +++ b/lib/l10n/app_localizations_es.dart @@ -1824,7 +1824,7 @@ class AppLocalizationsEs extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1835,6 +1835,18 @@ class AppLocalizationsEs extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Ordenar por'; diff --git a/lib/l10n/app_localizations_fr.dart b/lib/l10n/app_localizations_fr.dart index b97536a3..a4ecf771 100644 --- a/lib/l10n/app_localizations_fr.dart +++ b/lib/l10n/app_localizations_fr.dart @@ -1829,7 +1829,7 @@ class AppLocalizationsFr extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1840,6 +1840,18 @@ class AppLocalizationsFr extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Trier par'; diff --git a/lib/l10n/app_localizations_hu.dart b/lib/l10n/app_localizations_hu.dart index feb311cb..63b00a3d 100644 --- a/lib/l10n/app_localizations_hu.dart +++ b/lib/l10n/app_localizations_hu.dart @@ -1829,7 +1829,7 @@ class AppLocalizationsHu extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1840,6 +1840,18 @@ class AppLocalizationsHu extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Szűrés'; diff --git a/lib/l10n/app_localizations_it.dart b/lib/l10n/app_localizations_it.dart index 63dfc9b3..04902b49 100644 --- a/lib/l10n/app_localizations_it.dart +++ b/lib/l10n/app_localizations_it.dart @@ -1826,7 +1826,7 @@ class AppLocalizationsIt extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1837,6 +1837,18 @@ class AppLocalizationsIt extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Ordina per'; diff --git a/lib/l10n/app_localizations_ja.dart b/lib/l10n/app_localizations_ja.dart index 108d7516..633e7dcb 100644 --- a/lib/l10n/app_localizations_ja.dart +++ b/lib/l10n/app_localizations_ja.dart @@ -1767,7 +1767,7 @@ class AppLocalizationsJa extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1778,6 +1778,18 @@ class AppLocalizationsJa extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => '並び替え'; diff --git a/lib/l10n/app_localizations_ko.dart b/lib/l10n/app_localizations_ko.dart index 954a697b..410fb880 100644 --- a/lib/l10n/app_localizations_ko.dart +++ b/lib/l10n/app_localizations_ko.dart @@ -1762,7 +1762,7 @@ class AppLocalizationsKo extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1773,6 +1773,18 @@ class AppLocalizationsKo extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => '정렬 기준 선택'; diff --git a/lib/l10n/app_localizations_nl.dart b/lib/l10n/app_localizations_nl.dart index cddd566b..32493e33 100644 --- a/lib/l10n/app_localizations_nl.dart +++ b/lib/l10n/app_localizations_nl.dart @@ -1814,7 +1814,7 @@ class AppLocalizationsNl extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1825,6 +1825,18 @@ class AppLocalizationsNl extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sorteren door'; diff --git a/lib/l10n/app_localizations_pl.dart b/lib/l10n/app_localizations_pl.dart index 75756c0d..e264da75 100644 --- a/lib/l10n/app_localizations_pl.dart +++ b/lib/l10n/app_localizations_pl.dart @@ -1835,7 +1835,7 @@ class AppLocalizationsPl extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1846,6 +1846,18 @@ class AppLocalizationsPl extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sortuj po'; diff --git a/lib/l10n/app_localizations_pt.dart b/lib/l10n/app_localizations_pt.dart index a2372d1f..2a47cf89 100644 --- a/lib/l10n/app_localizations_pt.dart +++ b/lib/l10n/app_localizations_pt.dart @@ -1823,7 +1823,7 @@ class AppLocalizationsPt extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1834,6 +1834,18 @@ class AppLocalizationsPt extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Ordenar por'; diff --git a/lib/l10n/app_localizations_ru.dart b/lib/l10n/app_localizations_ru.dart index e7f17869..3f1e2ded 100644 --- a/lib/l10n/app_localizations_ru.dart +++ b/lib/l10n/app_localizations_ru.dart @@ -1824,7 +1824,7 @@ class AppLocalizationsRu extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1835,6 +1835,18 @@ class AppLocalizationsRu extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Сортировка'; diff --git a/lib/l10n/app_localizations_sk.dart b/lib/l10n/app_localizations_sk.dart index 4f10fd9f..91344fd1 100644 --- a/lib/l10n/app_localizations_sk.dart +++ b/lib/l10n/app_localizations_sk.dart @@ -1815,7 +1815,7 @@ class AppLocalizationsSk extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1826,6 +1826,18 @@ class AppLocalizationsSk extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Triediť podľa'; diff --git a/lib/l10n/app_localizations_sl.dart b/lib/l10n/app_localizations_sl.dart index b58fd784..544f83ef 100644 --- a/lib/l10n/app_localizations_sl.dart +++ b/lib/l10n/app_localizations_sl.dart @@ -1812,7 +1812,7 @@ class AppLocalizationsSl extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1823,6 +1823,18 @@ class AppLocalizationsSl extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sortiraj po'; diff --git a/lib/l10n/app_localizations_sv.dart b/lib/l10n/app_localizations_sv.dart index fd18a6f2..e1590d5b 100644 --- a/lib/l10n/app_localizations_sv.dart +++ b/lib/l10n/app_localizations_sv.dart @@ -1804,7 +1804,7 @@ class AppLocalizationsSv extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1815,6 +1815,18 @@ class AppLocalizationsSv extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Sortera efter'; diff --git a/lib/l10n/app_localizations_uk.dart b/lib/l10n/app_localizations_uk.dart index 42aebaf0..c453804d 100644 --- a/lib/l10n/app_localizations_uk.dart +++ b/lib/l10n/app_localizations_uk.dart @@ -1819,7 +1819,7 @@ class AppLocalizationsUk extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1830,6 +1830,18 @@ class AppLocalizationsUk extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => 'Сортувати за'; diff --git a/lib/l10n/app_localizations_zh.dart b/lib/l10n/app_localizations_zh.dart index eae23af1..974dba0a 100644 --- a/lib/l10n/app_localizations_zh.dart +++ b/lib/l10n/app_localizations_zh.dart @@ -1748,7 +1748,7 @@ class AppLocalizationsZh extends AppLocalizations { @override String get channels_slotBusySyncing => - 'Still reading channels from the radio. Try adding again in a moment.'; + 'The radio is still updating its channel list. Try again in a moment.'; @override String channels_slotsAllFull(int count) { @@ -1759,6 +1759,18 @@ class AppLocalizationsZh extends AppLocalizations { String get channels_publicAlreadyAdded => 'The Public channel is already on this radio.'; + @override + String get channels_writeOccupied => + 'The radio refused: another channel already uses that slot, and it was left unchanged. Try again now that the list has been re-read.'; + + @override + String get channels_writeRejected => + 'The radio refused the change, so the channel is as it was.'; + + @override + String get channels_writeNoReply => + 'The radio did not confirm the change. Check the channel list before trying again.'; + @override String get channels_sortBy => '排序方式'; diff --git a/lib/screens/channels_screen.dart b/lib/screens/channels_screen.dart index 052e186f..484f1983 100644 --- a/lib/screens/channels_screen.dart +++ b/lib/screens/channels_screen.dart @@ -934,18 +934,31 @@ class _ChannelsScreenState extends State psk[i] = random.nextInt(256); } Navigator.pop(dialogContext); - await connector.setChannel(slot, name, psk); - await channelMessageStore.clearChannelMessages( + final result = await connector.setChannel( slot, + name, + psk, ); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text( - context.l10n.channels_channelAdded(name), - ), - ); + // Only on OK: a refused write leaves another + // channel in this slot, and this would wipe + // that channel's history (#1026). + if (result == ChannelWrite.ok) { + await channelMessageStore + .clearChannelMessages(slot); } + if (!context.mounted) return; + if (explainUnconfirmedChannelWrite( + context, + result, + )) { + return; + } + showDismissibleSnackBar( + context, + content: Text( + context.l10n.channels_channelAdded(name), + ), + ); }, child: Text(dialogContext.l10n.common_create), ), @@ -992,7 +1005,7 @@ class _ChannelsScreenState extends State children: [ Expanded( child: FilledButton( - onPressed: () { + onPressed: () async { final name = nameController.text.trim(); final pskHex = pskController.text.trim(); if (name.isEmpty) { @@ -1026,15 +1039,24 @@ class _ChannelsScreenState extends State ); if (slot == null) return; Navigator.pop(dialogContext); - connector.setChannel(slot, name, psk); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text( - context.l10n.channels_channelAdded(name), - ), - ); + final result = await connector.setChannel( + slot, + name, + psk, + ); + if (!context.mounted) return; + if (explainUnconfirmedChannelWrite( + context, + result, + )) { + return; } + showDismissibleSnackBar( + context, + content: Text( + context.l10n.channels_channelAdded(name), + ), + ); }, child: Text(dialogContext.l10n.common_add), ), @@ -1055,7 +1077,7 @@ class _ChannelsScreenState extends State children: [ Expanded( child: FilledButton( - onPressed: () { + onPressed: () async { final slot = _allocateChannelSlot( dialogContext, connector, @@ -1065,15 +1087,24 @@ class _ChannelsScreenState extends State Channel.publicChannelPsk, ); Navigator.pop(dialogContext); - connector.setChannel(slot, 'Public', psk); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text( - context.l10n.channels_publicChannelAdded, - ), - ); + final result = await connector.setChannel( + slot, + 'Public', + psk, + ); + if (!context.mounted) return; + if (explainUnconfirmedChannelWrite( + context, + result, + )) { + return; } + showDismissibleSnackBar( + context, + content: Text( + context.l10n.channels_publicChannelAdded, + ), + ); }, child: Text(dialogContext.l10n.common_add), ), @@ -1255,17 +1286,26 @@ class _ChannelsScreenState extends State if (dialogContext.mounted) { Navigator.pop(dialogContext); } - connector.setChannel(slot, channelName, psk); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text( - context.l10n.channels_channelAdded( - channelName, - ), - ), - ); + final result = await connector.setChannel( + slot, + channelName, + psk, + ); + if (!context.mounted) return; + if (explainUnconfirmedChannelWrite( + context, + result, + )) { + return; } + showDismissibleSnackBar( + context, + content: Text( + context.l10n.channels_channelAdded( + channelName, + ), + ), + ); }, child: Text(dialogContext.l10n.common_add), ), @@ -1380,24 +1420,19 @@ class _ChannelsScreenState extends State // write and never loses the community. Uses the // screen's context, not the dialog's: this runs // after an await and the dialog may be gone. + int? slot; + var channelRefused = false; if (addPublicChannel && context.mounted) { - final slot = _allocateChannelSlot( + slot = _allocateChannelSlot( context, connector, ); - if (slot != null) { - final psk = community - .deriveCommunityPublicPsk(); - final channelName = - '${community.name} Public'; - connector.setChannel( - slot, - channelName, - psk, - ); - } + channelRefused = slot == null; } + // Closed before the write, which now waits for + // the radio: an open dialog would let a second + // tap create the community twice. if (dialogContext.mounted) { Navigator.pop(dialogContext); } @@ -1405,13 +1440,34 @@ class _ChannelsScreenState extends State // Refresh communities list _loadCommunities(); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text( - context.l10n.community_created(name), - ), + if (slot != null) { + final result = await connector.setChannel( + slot, + '${community.name} Public', + community.deriveCommunityPublicPsk(), ); + if (context.mounted) { + channelRefused = + explainUnconfirmedChannelWrite( + context, + result, + ); + } + } + + if (context.mounted) { + // A refusal's message is showing and + // persists; this toast would replace it + // (#1026). The QR below still shows: the + // community was created either way. + if (!channelRefused) { + showDismissibleSnackBar( + context, + content: Text( + context.l10n.community_created(name), + ), + ); + } // Show QR code dialog await QrCodeShareDialog.show( @@ -1711,6 +1767,9 @@ class _ChannelsScreenState extends State ); return; } + if (refuseWhileChannelTableBusy(dialogContext, connector)) { + return; + } final Uint8List psk; if (channelType == ChannelType.hashtag) { @@ -1732,7 +1791,16 @@ class _ChannelsScreenState extends State Navigator.pop(dialogContext); try { - await connector.setChannel(channel.index, name, psk); + final result = await connector.editChannel( + channel, + name, + psk, + ); + if (result != ChannelWrite.ok) { + if (!context.mounted) return; + explainUnconfirmedChannelWrite(context, result); + return; + } await connector.setChannelCyr2LatEnabled( channel.index, cyr2latEnabled, @@ -1834,13 +1902,21 @@ class _ChannelsScreenState extends State ), TextButton( onPressed: () async { + if (refuseWhileChannelTableBusy(dialogContext, connector)) { + return; + } Navigator.pop(dialogContext); try { - await connector.deleteChannel(channel.index); + final result = await connector.deleteChannel(channel.index); - await channelMessageStore.clearChannelMessages(channel.index); + // Only on OK: if the radio kept the channel, it keeps its + // history too (#1026). + if (result == ChannelWrite.ok) { + await channelMessageStore.clearChannelMessages(channel.index); + } if (!context.mounted) return; + if (explainUnconfirmedChannelWrite(context, result)) return; showDismissibleSnackBar( context, @@ -1872,7 +1948,10 @@ class _ChannelsScreenState extends State ); } - void _addPublicChannel(BuildContext context, MeshCoreConnector connector) { + Future _addPublicChannel( + BuildContext context, + MeshCoreConnector connector, + ) async { // This used to write slot 0 unconditionally, with no occupancy check, so // it overwrote whatever was there (#1021). Public is identified by its PSK // (`Channel.isPublicChannel`), not by its index, so it has no claim on @@ -1887,7 +1966,9 @@ class _ChannelsScreenState extends State final slot = _allocateChannelSlot(context, connector); if (slot == null) return; final psk = Channel.parsePskHex(Channel.publicChannelPsk); - connector.setChannel(slot, 'Public', psk); + final result = await connector.setChannel(slot, 'Public', psk); + if (!context.mounted) return; + if (explainUnconfirmedChannelWrite(context, result)) return; showDismissibleSnackBar( context, content: Text(context.l10n.channels_publicChannelAdded), @@ -2108,23 +2189,42 @@ class _ChannelsScreenState extends State ), TextButton( onPressed: () async { + if (communityChannels.isNotEmpty && + refuseWhileChannelTableBusy(dialogContext, connector)) { + return; + } Navigator.pop(dialogContext); - // Delete all community channels from the device + // Delete all community channels from the device. Each delete is + // answered before the next goes out, and the list is re-read + // once at the end: a re-read between deletes would put the next + // one in the middle of a sync, where its reply cannot be told + // from the sync's (#1026). + ChannelWrite? failure; for (final channel in communityChannels) { - await connector.deleteChannel(channel.index); + final result = await connector.deleteChannel( + channel.index, + refresh: false, + ); + if (result != ChannelWrite.ok) failure ??= result; + } + if (communityChannels.isNotEmpty) { + await connector.getChannels(force: true); } // Remove community from store await _communityStore.removeCommunity(community.id); _loadCommunities(); - if (context.mounted) { - showDismissibleSnackBar( - context, - content: Text(context.l10n.community_deleted(community.name)), - ); + if (!context.mounted) return; + if (failure != null && + explainUnconfirmedChannelWrite(context, failure)) { + return; } + showDismissibleSnackBar( + context, + content: Text(context.l10n.community_deleted(community.name)), + ); }, child: Text( dialogContext.l10n.community_delete, diff --git a/lib/screens/community_qr_scanner_screen.dart b/lib/screens/community_qr_scanner_screen.dart index f29711c4..bce2865e 100644 --- a/lib/screens/community_qr_scanner_screen.dart +++ b/lib/screens/community_qr_scanner_screen.dart @@ -231,8 +231,9 @@ class _CommunityQrScannerScreenState extends State { // // This used to skip the channel silently when the radio was full and then // show a green "joined" toast, which told the user everything had worked - // when the channel had not been added (#1021). A refusal now explains - // itself, and only a real success shows the green confirmation. + // when the channel had not been added (#1021). A refusal, or a write the + // radio did not confirm (#1026), now explains itself, and only a confirmed + // write shows the green confirmation. var channelRefused = false; if (addPublicChannel && context.mounted) { final connector = context.read(); @@ -242,7 +243,10 @@ class _CommunityQrScannerScreenState extends State { } else { final psk = community.deriveCommunityPublicPsk(); final channelName = '${community.name} Public'; - connector.setChannel(slot, channelName, psk); + final result = await connector.setChannel(slot, channelName, psk); + if (context.mounted) { + channelRefused = explainUnconfirmedChannelWrite(context, result); + } } } @@ -288,7 +292,7 @@ class _CommunityQrScannerScreenState extends State { // No slot is chosen here. It used to be, and that is the same bug the // channels screen had: the index was picked before the confirmation // dialog below and written after it, so a channel sync during the - // confirmation (setChannel itself starts one) could leave it pointing at + // confirmation (a channel write starts one) could leave it pointing at // an occupied slot. The slot is now chosen after the user confirms. if (!context.mounted) return; final confirmed = await showDialog( @@ -313,7 +317,14 @@ class _CommunityQrScannerScreenState extends State { // Refused: the persistent message explains why. Stay on this screen so // the user can retry, rather than popping away from the explanation. if (slot == null) return; - connector.setChannel(slot, channel.name, channel.psk); + final result = await connector.setChannel( + slot, + channel.name, + channel.psk, + ); + if (!context.mounted) return; + // Not confirmed: stay on this screen with the explanation showing. + if (explainUnconfirmedChannelWrite(context, result)) return; showDismissibleSnackBar( context, content: Text(context.l10n.channels_qrAdded(channel.name)), diff --git a/lib/services/stock_config_import_service.dart b/lib/services/stock_config_import_service.dart index 7c068042..af991ff5 100644 --- a/lib/services/stock_config_import_service.dart +++ b/lib/services/stock_config_import_service.dart @@ -74,8 +74,9 @@ class StockConfigImportResult { /// Sections whose commands were **sent** to the device without error. /// - /// This is not device confirmation. Apart from the identity import, which - /// waits for a real OK or ERR, the firmware's generic OK for these commands + /// This is not device confirmation. Apart from the identity import and the + /// channel writes, which wait for a real OK or ERR, the firmware's generic OK + /// for these commands /// is not currently awaited, so a frame lost or refused after it left our /// side would still land here. Wiring per-command confirmation is tracked /// separately; until then this set means "sent", and the UI says so rather @@ -92,7 +93,9 @@ class StockConfigImportResult { /// send count and not a confirmed count. final int contactsWritten; - /// Channels whose write command was sent. Same caveat as [contactsWritten]. + /// Channels the radio confirmed writing. Unlike [contactsWritten] this is a + /// confirmed count: a channel write waits for the radio's OK, and one it + /// refused or never answered is in [skippedChannels] instead (#1026). final int channelsAdded; bool get isComplete => failed.isEmpty && skippedChannels.isEmpty; @@ -278,17 +281,38 @@ class StockConfigImportService { existing: _connector.channels, incoming: incoming, maxChannels: _connector.maxChannels, - isSyncing: _connector.isSyncingChannels, + isSyncing: _connector.isChannelTableBusy, ); + // Each write is answered before the next goes out, and the list is + // re-read once at the end. A re-read after every write would put the next + // write in the middle of a sync, where its reply cannot be told from the + // sync's (#1026). + var added = 0; for (final assignment in plan.assignments) { - await _connector.setChannel( + final result = await _connector.setChannel( assignment.slot, assignment.channel.name, assignment.channel.secret, + refresh: false, ); + if (result == ChannelWrite.ok) { + added++; + } else { + skipped.add( + SkippedChannel( + assignment.channel.name, + result == ChannelWrite.noReply + ? StockConfigImportIssue.noReply + : StockConfigImportIssue.rejected, + ), + ); + } + } + if (plan.assignments.isNotEmpty) { + await _connector.getChannels(force: true); } skipped.addAll(plan.skipped); - return plan.assignments.length; + return added; } /// Upserts every contact. The firmware's add-or-update command keys on the diff --git a/test/connector/channel_write_reply_test.dart b/test/connector/channel_write_reply_test.dart new file mode 100644 index 00000000..c132f64d --- /dev/null +++ b/test/connector/channel_write_reply_test.dart @@ -0,0 +1,209 @@ +// #1026 (epic #1021): a channel write waits for the radio's OK or ERR, and a +// slot's message history is cleared only once the radio has said OK. +// +// Firmware with OffbandMesh/meshcore-firmware#1864 refuses a different channel +// over an occupied slot with ERR_CODE_BAD_STATE and keeps that channel intact. +// Clearing its history on the strength of having sent the frame would lose it. + +import 'dart:typed_data'; + +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/connector/meshcore_connector.dart'; +import 'package:meshcore_open/connector/meshcore_protocol.dart'; +import 'package:meshcore_open/models/channel.dart'; +import 'package:meshcore_open/storage/drift/blob_store.dart'; +import 'package:meshcore_open/storage/drift/offband_database.dart'; +import 'package:meshcore_open/storage/prefs_manager.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +Uint8List psk(int fill) => Uint8List.fromList(List.filled(16, fill)); + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + late OffbandDatabase db; + late MeshCoreConnector connector; + late List sent; + + List setFrames() => + sent.where((f) => f.isNotEmpty && f[0] == cmdSetChannel).toList(); + + void ok() => connector.handleFrameForTest([respCodeOk]); + void err(int code) => connector.handleFrameForTest([respCodeErr, code]); + + /// The second write of a clear-then-add goes out only after the first has + /// been answered and its history cleared, which takes real async time. + Future untilSetFrames(int count) async { + for (var i = 0; i < 2000 && setFrames().length < count; i++) { + await Future.delayed(const Duration(milliseconds: 1)); + } + expect(setFrames(), hasLength(count)); + } + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + PrefsManager.reset(); + await PrefsManager.initialize(); + db = OffbandDatabase(NativeDatabase.memory()); + BlobStore.overrideForTest(BlobStore(db)); + connector = MeshCoreConnector(); + connector.setConnectedForTest(); + sent = []; + connector.sendFrameOverrideForTest = sent.add; + connector.channelMessagesForTest[3] = []; + }); + + tearDown(() async { + await db.close(); + BlobStore.clearTestOverride(); + }); + + group('setChannel', () { + test('OK clears the slot\'s old history and reports ok', () async { + final write = connector.setChannel( + 3, + 'Weather', + psk(0x22), + refresh: false, + ); + ok(); + + expect(await write, ChannelWrite.ok); + expect(connector.channelMessagesForTest.containsKey(3), isFalse); + }); + + test('BAD_STATE is occupied, and the history stays', () async { + final write = connector.setChannel( + 3, + 'Weather', + psk(0x22), + refresh: false, + ); + err(errCodeBadState); + + expect(await write, ChannelWrite.occupied); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + + test('any other ERR is rejected, and the history stays', () async { + final write = connector.setChannel( + 3, + 'Weather', + psk(0x22), + refresh: false, + ); + err(errCodeNotFound); + + expect(await write, ChannelWrite.rejected); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + + test('no reply is noReply, and the history stays', () async { + connector.channelWriteReplyTimeout = const Duration(milliseconds: 20); + final write = connector.setChannel( + 3, + 'Weather', + psk(0x22), + refresh: false, + ); + + expect(await write, ChannelWrite.noReply); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + + test('a timed-out write cannot take the next write\'s reply', () async { + connector.channelWriteReplyTimeout = const Duration(milliseconds: 20); + expect( + await connector.setChannel(3, 'A', psk(0x22), refresh: false), + ChannelWrite.noReply, + ); + + connector.channelWriteReplyTimeout = const Duration(seconds: 5); + final second = connector.setChannel(4, 'B', psk(0x33), refresh: false); + err(errCodeBadState); + + expect(await second, ChannelWrite.occupied); + }); + + test('the table is busy until the radio answers', () async { + final write = connector.setChannel( + 3, + 'Weather', + psk(0x22), + refresh: false, + ); + expect(connector.isChannelTableBusy, isTrue); + + ok(); + await write; + expect(connector.isChannelTableBusy, isFalse); + }); + + test('disconnected sends nothing and reports noReply', () async { + final fresh = MeshCoreConnector(); + fresh.sendFrameOverrideForTest = sent.add; + + expect( + await fresh.setChannel(3, 'Weather', psk(0x22)), + ChannelWrite.noReply, + ); + expect(sent, isEmpty); + }); + }); + + group('deleteChannel', () { + test('OK clears the history', () async { + final delete = connector.deleteChannel(3, refresh: false); + ok(); + + expect(await delete, ChannelWrite.ok); + expect(connector.channelMessagesForTest.containsKey(3), isFalse); + }); + + test('a refusal keeps the history', () async { + final delete = connector.deleteChannel(3, refresh: false); + err(errCodeNotFound); + + expect(await delete, ChannelWrite.rejected); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + }); + + group('editChannel', () { + final current = Channel(index: 3, name: 'Weather', psk: psk(0x22)); + + test('a rename is one write and keeps the history', () async { + final edit = connector.editChannel(current, 'Wx', psk(0x22)); + ok(); + + expect(await edit, ChannelWrite.ok); + expect(setFrames(), hasLength(1)); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + + test('a new key is clear-then-add, so #1864 firmware accepts it', () async { + final edit = connector.editChannel(current, 'Weather', psk(0x99)); + await untilSetFrames(1); + // The clear: an empty name. + expect(setFrames()[0][2], 0); + ok(); + + await untilSetFrames(2); + expect(setFrames()[1].sublist(2 + 32, 2 + 32 + 16), psk(0x99)); + ok(); + + expect(await edit, ChannelWrite.ok); + expect(connector.channelMessagesForTest.containsKey(3), isFalse); + }); + + test('a refused clear writes nothing more and keeps the history', () async { + final edit = connector.editChannel(current, 'Weather', psk(0x99)); + await untilSetFrames(1); + err(errCodeNotFound); + + expect(await edit, ChannelWrite.rejected); + expect(setFrames(), hasLength(1)); + expect(connector.channelMessagesForTest.containsKey(3), isTrue); + }); + }); +} diff --git a/untranslated.json b/untranslated.json index 26e30fc0..787fbfb6 100644 --- a/untranslated.json +++ b/untranslated.json @@ -155,6 +155,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -526,6 +529,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -897,6 +903,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -1268,6 +1277,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -1639,6 +1651,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -2010,6 +2025,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -2381,6 +2399,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -2752,6 +2773,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -3123,6 +3147,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -3494,6 +3521,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -3865,6 +3895,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -4236,6 +4269,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -4607,6 +4643,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -4978,6 +5017,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -5349,6 +5391,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -5720,6 +5765,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause", @@ -6091,6 +6139,9 @@ "channels_slotBusySyncing", "channels_slotsAllFull", "channels_publicAlreadyAdded", + "channels_writeOccupied", + "channels_writeRejected", + "channels_writeNoReply", "composerBudgetBlockedTitle", "composerBudgetLimitedTitle", "composerBudgetBluetoothCause",