From c15bdce1c6d5d723b0aa8644bf6e45e45bc8a1c7 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 08:51:05 -0600 Subject: [PATCH 1/5] fix(sdcard): distinguish a timed-out SD LIST from an empty card (closes #396) GetSdCardFilesAsync returned an empty list both when the SD card was genuinely empty and when the device never answered, so an unreachable device holding data rendered downstream as "SD card OK, 0 files". The firmware emits no end-of-listing marker and writes nothing at all for an empty directory, so the two cases are byte-for-byte identical on the wire and cannot be told apart from the LIST reply alone. Core now appends a SYSTem:ERRor? query to the same text exchange and uses its reply as a terminator: the transport is ordered and the firmware does not process the next command until the listing has been handed to the output, so the reply proves both that the device is answering and that the listing ahead of it arrived in full. A missing terminator raises SdCardListIncompleteException instead of a plausible-looking empty list, which also catches the truncated listing that no downstream corroboration can detect. Co-Authored-By: Claude Opus 5 --- .../Device/ScpiResponseClassifierTests.cs | 29 +++ .../Device/SdCard/SdCardOperationsTests.cs | 239 +++++++++++++++++- .../Device/DaqifiStreamingDevice.cs | 148 ++++++++--- .../Device/ScpiResponseClassifier.cs | 61 +++++ .../Device/SdCard/ISdCardOperations.cs | 5 + .../SdCard/SdCardListIncompleteException.cs | 43 ++++ 6 files changed, 488 insertions(+), 37 deletions(-) create mode 100644 src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs diff --git a/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs b/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs index 8558c4a2..a15c1191 100644 --- a/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs +++ b/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs @@ -55,5 +55,34 @@ public void TryExtractErrorCode_ReturnsFalseAndZero_ForNonNumericOrNonError(stri Assert.False(ScpiResponseClassifier.TryExtractErrorCode(line, out var code)); Assert.Equal(0, code); } + + [Theory] + [InlineData("0,\"No error\"")] // clean queue — the common case + [InlineData("-200,\"Execution error\"")] + [InlineData("+0,\"No error\"")] // explicit positive sign + [InlineData("-420, \"Query UNTERMINATED\"")] // space after the comma + [InlineData(" 0,\"No error\" \r\n")] // leading/trailing whitespace + CRLF + [InlineData("0,\"\"")] // empty message + public void IsSystemErrorReplyLine_MatchesErrorQueueReplies(string line) + { + Assert.True(ScpiResponseClassifier.IsSystemErrorReplyLine(line)); + } + + [Theory] + [InlineData("Daqifi/log_20240115_103000.bin 1024")] // SD listing entry + [InlineData("Daqifi/log_20240115_103000.bin")] // listing entry with no size + [InlineData("0,\"No error\" 1024")] // reply shape but with a trailing size + [InlineData("**ERROR: -200, \"Execution error\"")] // ERROR-prefixed, not a query reply + [InlineData("Error !! No SD Card Detected")] + [InlineData("0 1024")] // no comma + [InlineData("0,No error")] // unquoted message + [InlineData("0,\"")] // unterminated quote + [InlineData("-,\"No error\"")] // sign with no digits + [InlineData("[Error:3]Failed to open directory")] + [InlineData("")] + public void IsSystemErrorReplyLine_DoesNotMatchListingOrOtherText(string line) + { + Assert.False(ScpiResponseClassifier.IsSystemErrorReplyLine(line)); + } } } diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs index 41d5a951..17fab924 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs @@ -871,8 +871,9 @@ public async Task GetSdCardFilesAsync_WithFilesAndInterleavedError_ReturnsFiles( [Fact] public async Task GetSdCardFilesAsync_WithEmptyDirectory_ReturnsEmptyList() { - // Arrange - device returns no lines (empty directory, no errors). This is - // the legitimate "0 files" case and must keep its existing behavior. + // Arrange - device returns no listing lines (empty directory, no errors) but does + // answer the terminator query. This is the legitimate "0 files" case and must keep + // its existing behavior: an empty list, no exception (#396). var device = new RetryableSdCardStreamingDevice("TestDevice"); device.ResponseSequence.Enqueue(new List()); device.Connect(); @@ -885,6 +886,150 @@ public async Task GetSdCardFilesAsync_WithEmptyDirectory_ReturnsEmptyList() Assert.Equal(1, device.ExecuteTextCommandCallCount); } + #region Timed-out vs. empty listing (issue #396) + + [Fact] + public async Task GetSdCardFilesAsync_SendsTerminatorQueryAfterListCommand() + { + // The terminator only proves the listing is complete if it is requested AFTER the + // listing itself, in the same exchange — the transport is ordered, so its reply + // cannot overtake listing lines the device had already written. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List { "Daqifi/log_20240115_103000.bin" }; + device.Connect(); + + await device.GetSdCardFilesAsync(); + + var sentCommands = device.SentMessages.Select(m => m.Data).ToList(); + var listIndex = sentCommands.IndexOf("SYSTem:STORage:SD:LIST?"); + var terminatorIndex = sentCommands.IndexOf("SYSTem:ERRor?"); + Assert.True(listIndex >= 0, "The file-list query was not sent."); + Assert.True(terminatorIndex > listIndex, "The terminator query must be sent after the file-list query."); + } + + [Fact] + public async Task GetSdCardFilesAsync_TerminatorReplyIsNotParsedAsAFile() + { + // The terminator is a protocol artifact, not directory content: it must never reach + // the file parser and show up as a phantom SD card file. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List { "Daqifi/log_20240115_103000.bin" }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + Assert.Single(files); + Assert.Equal("log_20240115_103000.bin", files[0].FileName); + } + + [Fact] + public async Task GetSdCardFilesAsync_WhenDeviceNeverAnswers_ThrowsInsteadOfReturningEmptyList() + { + // The bug in #396: a device that never answered produced the exact same empty list as + // a healthy empty card, so downstream rendered "SD card OK - 0 files" for an + // unreachable device holding data. Both attempts go unanswered here. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List { "Daqifi/log_20240115_103000.bin" }; + device.Connect(); + + // A first, healthy listing populates the cache... + Assert.Single(await device.GetSdCardFilesAsync()); + + // ...then the device goes silent. + device.CannedTextResponse = new List(); + device.UnterminatedAttempts = int.MaxValue; + + await Assert.ThrowsAsync( + () => device.GetSdCardFilesAsync()); + + // The cache must not be overwritten with a listing we never actually received. + Assert.Single(device.SdCardFiles); + Assert.Equal("log_20240115_103000.bin", device.SdCardFiles[0].FileName); + } + + [Fact] + public async Task GetSdCardFilesAsync_WhenListingIsTruncated_ThrowsRatherThanReturningAShortList() + { + // The case no downstream mitigation can reach: the device answered, but stopped + // part-way through the listing. Corroborating with a later query (as the Avalonia port + // does) cannot detect this — only the missing terminator can. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "Daqifi/log_20240115_103000.bin", + "Daqifi/log_20240115_1030", + }; + device.UnterminatedAttempts = int.MaxValue; + device.Connect(); + + var ex = await Assert.ThrowsAsync( + () => device.GetSdCardFilesAsync()); + + // The partial response is preserved for diagnostics rather than silently returned. + Assert.Equal(2, ex.RawDeviceResponse.Count); + } + + [Fact] + public async Task GetSdCardFilesAsync_WithStaleTerminatorAheadOfTheListing_StillReturnsTheFiles() + { + // A terminator reply from a previous, timed-out exchange can still be in the transport + // buffer and lead this response. Splitting at the FIRST match would discard the real + // listing behind it and report an empty card — the very failure #396 is about. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "-200,\"Execution error\"", // stale reply left over from an earlier exchange + "Daqifi/log_20240115_103000.bin", + "Daqifi/data.bin", + }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + // Both files survive, and the stale reply is not parsed as a phantom file. + Assert.Equal(2, files.Count); + Assert.Equal("log_20240115_103000.bin", files[0].FileName); + Assert.Equal("data.bin", files[1].FileName); + } + + [Fact] + public async Task GetSdCardFilesAsync_WhenTerminatorMissingOnFirstAttemptOnly_RetriesAndSucceeds() + { + // A one-off stall is retried on the same terms as a transient SCPI error, so a single + // dropped reply does not become a user-visible failure. + var device = new RetryableSdCardStreamingDevice("TestDevice"); + device.ResponseSequence.Enqueue(new List()); + device.ResponseSequence.Enqueue(new List { "Daqifi/log_20240115_103000.bin" }); + device.UnterminatedAttempts = 1; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + Assert.Single(files); + Assert.Equal("log_20240115_103000.bin", files[0].FileName); + Assert.Equal(2, device.ExecuteTextCommandCallCount); + } + + [Fact] + public async Task GetSdCardFilesAsync_WhenDeviceNeverAnswers_RestoresLanInterface() + { + // The throw must not skip the interface restore — the SD subsystem would be left + // enabled and the LAN disabled for every later command. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List(); + device.UnterminatedAttempts = int.MaxValue; + device.Connect(); + + await Assert.ThrowsAsync( + () => device.GetSdCardFilesAsync()); + + var sentCommands = device.SentMessages.Select(m => m.Data).ToList(); + Assert.Contains("SYSTem:STORage:SD:ENAble 0", sentCommands); // DisableStorageSd + Assert.Contains("SYSTem:COMMunicate:LAN:ENAbled 1", sentCommands); // EnableNetworkLan + } + + #endregion + [Fact] public async Task GetSdCardFilesAsync_LastScpiError_ContainsOnlyScpiFormattedLine() { @@ -1799,6 +1944,9 @@ private class RetryableSdCardStreamingDevice : DaqifiStreamingDevice public Queue> ResponseSequence { get; } = new(); public int ExecuteTextCommandCallCount { get; private set; } + /// + public int UnterminatedAttempts { get; set; } + public RetryableSdCardStreamingDevice(string name, IPAddress? ipAddress = null) : base(name, ipAddress) { @@ -1824,12 +1972,14 @@ protected override Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; setupAction(); ExecuteTextCommandCallCount++; var response = ResponseSequence.Count > 0 ? ResponseSequence.Dequeue() : new List(); - return Task.FromResult>(response); + return Task.FromResult(SdCardTestResponses.AnswerErrorQuery( + response, SentMessages, sentBefore, ExecuteTextCommandCallCount, UnterminatedAttempts)); } protected override async Task> ExecuteTextCommandAsync( @@ -1838,12 +1988,63 @@ protected override async Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; await setupActionAsync(cancellationToken).ConfigureAwait(false); ExecuteTextCommandCallCount++; var response = ResponseSequence.Count > 0 ? ResponseSequence.Dequeue() : new List(); - return response; + return SdCardTestResponses.AnswerErrorQuery( + response, SentMessages, sentBefore, ExecuteTextCommandCallCount, UnterminatedAttempts); + } + } + + /// + /// Shared device-behavior helper for the SD card fakes: models the way a live device + /// answers the SYSTem:ERRor? query that GetSdCardFilesAsync appends to the + /// listing exchange as an end-of-listing terminator (#396). + /// + private static class SdCardTestResponses + { + /// The reply a healthy device gives to SYSTem:ERRor? with a clean queue. + public const string NoErrorReply = "0,\"No error\""; + + /// + /// Appends the SYSTem:ERRor? reply to when the + /// exchange actually asked for it and this attempt is not one of the + /// leading attempts being simulated as + /// unanswered. + /// + public static IReadOnlyList AnswerErrorQuery( + IReadOnlyList response, + IReadOnlyList> sentMessages, + int sentBefore, + int attemptNumber, + int unterminatedAttempts) + { + if (attemptNumber <= unterminatedAttempts) + { + return response; + } + + var errorQuery = ScpiMessageProducer.GetSystemError.Data; + var askedForError = false; + for (var i = sentBefore; i < sentMessages.Count; i++) + { + if (sentMessages[i].Data == errorQuery) + { + askedForError = true; + break; + } + } + + if (!askedForError) + { + return response; + } + + var withTerminator = new List(response) { NoErrorReply }; + return withTerminator; } } @@ -1853,9 +2054,20 @@ protected override async Task> ExecuteTextCommandAsync( /// private class TestableSdCardStreamingDevice : DaqifiStreamingDevice { + private int _executeTextCommandCallCount; + public List> SentMessages { get; } = new(); public List CannedTextResponse { get; set; } = new(); + /// + /// Number of leading text exchanges to answer WITHOUT the SYSTem:ERRor? reply + /// that GetSdCardFilesAsync uses as its end-of-listing terminator (#396) — i.e. + /// how many attempts simulate a device that never answered, or stopped answering + /// part-way through the listing. Defaults to 0, so the fake behaves like a healthy + /// device and always terminates its listing. + /// + public int UnterminatedAttempts { get; set; } + /// /// Simulates a USB connection so SD card operations are allowed. /// @@ -1880,9 +2092,13 @@ protected override Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; + // Execute the setup action so we can capture the SCPI commands setupAction(); - return Task.FromResult>(CannedTextResponse); + _executeTextCommandCallCount++; + return Task.FromResult(SdCardTestResponses.AnswerErrorQuery( + CannedTextResponse, SentMessages, sentBefore, _executeTextCommandCallCount, UnterminatedAttempts)); } protected override async Task> ExecuteTextCommandAsync( @@ -1891,8 +2107,11 @@ protected override async Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; await setupActionAsync(cancellationToken).ConfigureAwait(false); - return CannedTextResponse; + _executeTextCommandCallCount++; + return SdCardTestResponses.AnswerErrorQuery( + CannedTextResponse, SentMessages, sentBefore, _executeTextCommandCallCount, UnterminatedAttempts); } } @@ -2143,8 +2362,10 @@ protected override Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; setupAction(); - return Task.FromResult>(CannedTextResponse); + return Task.FromResult(SdCardTestResponses.AnswerErrorQuery( + CannedTextResponse, SentMessages, sentBefore, attemptNumber: 1, unterminatedAttempts: 0)); } protected override async Task> ExecuteTextCommandAsync( @@ -2153,8 +2374,10 @@ protected override async Task> ExecuteTextCommandAsync( int completionTimeoutMs = 250, CancellationToken cancellationToken = default) { + var sentBefore = SentMessages.Count; await setupActionAsync(cancellationToken).ConfigureAwait(false); - return CannedTextResponse; + return SdCardTestResponses.AnswerErrorQuery( + CannedTextResponse, SentMessages, sentBefore, attemptNumber: 1, unterminatedAttempts: 0); } protected override async Task ExecuteRawCaptureAsync( diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index ebcd6dcf..8383f81b 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -1583,6 +1583,32 @@ private void EnsureSdFileTransferSupportedOnTransport() /// Thrown when no SD card is installed in the device. /// Thrown when the SD card filesystem cannot satisfy the request (corrupt card, unreadable directory). /// Thrown when the device returned an SCPI error that did not match a more specific condition. Empty directories return an empty list rather than throwing. + /// + /// Thrown when the listing did not arrive in full — the device never answered, or stopped + /// answering part-way through. Distinguishing this from a genuinely empty card is the whole + /// point of the terminator probe described in the remarks (closes #396). + /// + /// + /// + /// The firmware emits no end-of-listing marker, and for an empty directory it writes nothing + /// at all, so a lost or truncated reply is byte-for-byte indistinguishable from a healthy + /// empty card. Core closes that gap by appending a SYSTem:ERRor? query to the same + /// text exchange: the transport delivers in order and the firmware does not process the + /// next command until the listing has been handed to the output, so receiving the reply + /// proves both that the device is answering and that the listing ahead of it is complete. + /// Its absence means the response is incomplete, and the caller gets an exception instead of + /// a plausible-looking empty list. + /// + /// + /// The terminator's error code is used only as a liveness marker, never for classification: + /// the queue it pops can hold entries left by earlier commands, so attributing the code to + /// this listing would misreport stale failures. SD errors continue to be classified from the + /// listing lines themselves. Note the side effect this implies — each listing consumes one + /// entry from the device's SCPI error queue, so a + /// run afterwards will not see the entry + /// this listing generated. + /// + /// public async Task> GetSdCardFilesAsync(CancellationToken cancellationToken = default) { if (!IsConnected) @@ -1598,42 +1624,44 @@ public async Task> GetSdCardFilesAsync(Cancellatio Send(ScpiMessageProducer.StopStreaming); IsStreaming = false; - IReadOnlyList lines; + IReadOnlyList lines = Array.Empty(); + IReadOnlyList listing = Array.Empty(); + var isComplete = false; try { - lines = await ExecuteTextCommandAsync(async ct => + // Attempt 0 plus SD_LIST_MAX_RETRIES retries. A SCPI error here is often a transient + // timing issue, and an unterminated response can be a one-off stall, so both are + // retried once after an additional settle delay before being surfaced. + for (var attempt = 0; attempt <= SD_LIST_MAX_RETRIES; attempt++) { - PrepareSdInterface(); - - // Allow the device firmware to complete the SPI bus switch - // before querying the SD card. Without this delay, the device - // can return SCPI error -200 (Execution error). - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - - Send(ScpiMessageProducer.GetSdFileList); - }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); - - // If the response contains a SCPI error (transient timing issue), - // retry once after an additional settle delay. - if (ContainsScpiError(lines)) - { - for (var retry = 0; retry < SD_LIST_MAX_RETRIES; retry++) + if (attempt > 0) { cancellationToken.ThrowIfCancellationRequested(); await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); + } - lines = await ExecuteTextCommandAsync(async ct => - { - PrepareSdInterface(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - Send(ScpiMessageProducer.GetSdFileList); - }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); + lines = await ExecuteTextCommandAsync(async ct => + { + PrepareSdInterface(); - if (!ContainsScpiError(lines)) - { - break; - } + // Allow the device firmware to complete the SPI bus switch + // before querying the SD card. Without this delay, the device + // can return SCPI error -200 (Execution error). + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); + + Send(ScpiMessageProducer.GetSdFileList); + + // End-of-listing terminator — see this method's remarks. Sent inside the + // same text exchange so the ordering guarantee holds. + Send(ScpiMessageProducer.GetSystemError); + }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); + + isComplete = TrySplitAtSdListTerminator(lines, out listing); + + if (isComplete && !ContainsScpiError(listing)) + { + break; } } } @@ -1646,13 +1674,73 @@ public async Task> GetSdCardFilesAsync(Cancellatio } } - ThrowIfSdCardListError(lines); + if (!isComplete) + { + throw new SdCardListIncompleteException(lines); + } + + ThrowIfSdCardListError(listing); - var files = SdCardFileListParser.ParseFileList(lines); + var files = SdCardFileListParser.ParseFileList(listing); _sdCardFiles = files; return files; } + /// + /// Splits a raw SD listing response at the SYSTem:ERRor? terminator reply that + /// appends to the exchange. + /// + /// The raw response lines captured from the device. + /// + /// The lines that precede the terminator — the directory listing proper — when the method + /// returns true; otherwise the unmodified input. + /// + /// + /// true when the terminator was present, meaning the response is complete; + /// false when it never arrived, meaning the response is missing or truncated. + /// + private static bool TrySplitAtSdListTerminator( + IReadOnlyList lines, + out IReadOnlyList listingLines) + { + // Scan from the end. A terminator reply from a PREVIOUS, timed-out exchange can still + // be sitting in the transport buffer and lead this response; splitting at the first + // match would then discard the listing that follows it and report an empty card — + // exactly the failure this terminator exists to prevent. + var terminatorIndex = -1; + for (var i = lines.Count - 1; i >= 0; i--) + { + if (ScpiResponseClassifier.IsSystemErrorReplyLine(lines[i])) + { + terminatorIndex = i; + break; + } + } + + if (terminatorIndex < 0) + { + listingLines = lines; + return false; + } + + var listing = new List(terminatorIndex); + for (var j = 0; j < terminatorIndex; j++) + { + // Any other terminator-shaped line is a stale reply of the same kind, not + // directory content — no firmware listing entry can match that shape, since + // entries are always " ". + if (ScpiResponseClassifier.IsSystemErrorReplyLine(lines[j])) + { + continue; + } + + listing.Add(lines[j]); + } + + listingLines = listing; + return true; + } + /// /// Retrieves the free and total byte counts of the device's SD card. /// @@ -2331,6 +2419,8 @@ private static void ThrowIfSdCardListError(IReadOnlyList lines) } // No error lines and no content lines — empty directory. Caller continues. + // Safe to treat as empty rather than as a lost reply: GetSdCardFilesAsync only reaches + // this point once the device has answered the end-of-listing terminator (#396). } /// diff --git a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs index 1edfcd3b..edb503c6 100644 --- a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs +++ b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs @@ -51,6 +51,67 @@ internal static bool IsScpiErrorLine(string line) || MatchesStrictScpiErrorPrefix(trimmed, "ERROR"); } + /// + /// Returns true if the line is the reply to a SYSTem:ERRor? query — the IEEE 488.2 + /// error-queue format <code>,"<message>", e.g. 0,"No error" or + /// -200,"Execution error". Deliberately narrow: the code must be the first thing on + /// the line and the quoted message must run to the end of it, so a device-emitted SD listing + /// entry (always <path> <size>, space-separated, per the firmware's + /// "%s %u\r\n" format) can never match. Unlike this does + /// not require an ERROR token — the query reply carries the bare code. + /// + /// + /// Used as the end-of-response marker for the SD card directory listing (closes #396): the + /// firmware emits no terminator of its own and writes nothing at all for an empty directory, + /// so Core appends a SYSTem:ERRor? query to the same text exchange and treats its + /// reply as proof that everything the device had to say about the listing has arrived. + /// + internal static bool IsSystemErrorReplyLine(string line) + { + var trimmed = line.Trim(); + + var index = 0; + if (index < trimmed.Length && (trimmed[index] == '+' || trimmed[index] == '-')) + { + index++; + } + + var digitStart = index; + while (index < trimmed.Length && trimmed[index] >= '0' && trimmed[index] <= '9') + { + index++; + } + + if (index == digitStart) + { + return false; + } + + index = SkipSpaces(trimmed, index); + + if (index >= trimmed.Length || trimmed[index] != ',') + { + return false; + } + + index = SkipSpaces(trimmed, index + 1); + + // Require an opening quote and a distinct closing quote at end-of-line. + return index < trimmed.Length - 1 + && trimmed[index] == '"' + && trimmed[trimmed.Length - 1] == '"'; + } + + private static int SkipSpaces(string value, int index) + { + while (index < value.Length && (value[index] == ' ' || value[index] == '\t')) + { + index++; + } + + return index; + } + /// /// Extracts the numeric error code from a SCPI error line — e.g. -200 from /// **ERROR: -200,"Execution error", ERROR -113,"Undefined header", or diff --git a/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs b/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs index 55f97822..884a04a0 100644 --- a/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs +++ b/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs @@ -40,6 +40,11 @@ public interface ISdCardOperations /// Thrown when no SD card is installed in the device. /// Thrown when the SD card filesystem cannot satisfy the request (e.g. corrupt card, unreadable directory). /// Thrown when the device returned an SCPI error that did not match a more specific condition. An empty directory returns an empty list rather than throwing. + /// + /// Thrown when the device did not answer the listing query, or stopped answering part-way + /// through it. An empty result therefore always means an empty card — never an unreachable + /// device — so callers can render it as such (closes #396). + /// Task> GetSdCardFilesAsync(CancellationToken cancellationToken = default); /// diff --git a/src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs b/src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs new file mode 100644 index 00000000..4b22020e --- /dev/null +++ b/src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs @@ -0,0 +1,43 @@ +using System.Collections.Generic; + +#nullable enable + +namespace Daqifi.Core.Device.SdCard +{ + /// + /// Thrown when an SD card directory listing did not arrive in full: the device either never + /// answered the SYSTem:STORage:SD:LISt? query, or stopped answering part-way through it. + /// + /// + /// + /// The firmware emits no end-of-listing marker and writes nothing at all for an empty directory, + /// so "no bytes received" is byte-for-byte identical to a healthy empty card on the wire. Core + /// therefore appends a SYSTem:ERRor? query to the same text exchange and uses its reply as + /// a terminator: the transport is ordered, so a terminator reply proves both that the device is + /// answering and that everything it had to say about the listing arrived first. This exception + /// is raised when that terminator never came back — which previously surfaced as a healthy-looking + /// "empty SD card" (closes #396). + /// + /// + /// A caller seeing this should treat the listing as unknown, not empty. Typical causes are a + /// silently dropped link, a device that is wedged or powered down, or congestion severe enough + /// to push the reply past the response window. Retrying once the link is known good is + /// reasonable; rendering "0 files" is not. + /// + /// + public class SdCardListIncompleteException : SdCardOperationException + { + /// + /// Initializes a new instance of the class. + /// + /// The raw response lines captured from the device, if any. + public SdCardListIncompleteException(IReadOnlyList rawDeviceResponse) + : base( + "The SD card directory listing did not complete: the device did not finish " + + "answering the file-list query, so the listing may be missing entries or be " + + "absent entirely. This is not an empty SD card — check the connection and retry.", + rawDeviceResponse) + { + } + } +} From 918c32ce845e9fcc10fbd529ec7e09372f4bc54a Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 09:23:06 -0600 Subject: [PATCH 2/5] fix(sdcard): drop pre-command lines and widen the SD listing completion window Addresses both Qodo findings on PR #400. 1. Stale terminator false-completes. A late reply to an earlier command can land in the window between the text consumer binding and the setup action sending anything, and the SD listing would accept it as its end-of-listing terminator -- letting a silent device pass as a healthy empty card, the very bug #396 is about. ExecuteTextCommandCoreAsync now marks that boundary and excludes anything received before the exchange sent a thing. It cannot be a response to a command that had not yet gone out, so this is correct for every caller, not just the listing. 2. Terminator missed by the completion window. The listing exchange kept the 250ms default, so a terminator trailing the last listing line by more than that would read as missing and fail a listing that was about to complete. The exchange now uses a dedicated 1s window. Co-Authored-By: Claude Opus 5 --- .../Device/DaqifiDeviceStaleTextLineTests.cs | 203 ++++++++++++++++++ src/Daqifi.Core/Device/DaqifiDevice.cs | 33 ++- .../Device/DaqifiStreamingDevice.cs | 18 +- 3 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 src/Daqifi.Core.Tests/Device/DaqifiDeviceStaleTextLineTests.cs diff --git a/src/Daqifi.Core.Tests/Device/DaqifiDeviceStaleTextLineTests.cs b/src/Daqifi.Core.Tests/Device/DaqifiDeviceStaleTextLineTests.cs new file mode 100644 index 00000000..895fccc4 --- /dev/null +++ b/src/Daqifi.Core.Tests/Device/DaqifiDeviceStaleTextLineTests.cs @@ -0,0 +1,203 @@ +using Daqifi.Core.Communication.Transport; +using Daqifi.Core.Device; +using System.Text; + +namespace Daqifi.Core.Tests.Device; + +/// +/// Coverage for the stale-line boundary in the text exchange (raised while fixing #396). +/// +/// +/// A late reply to an EARLIER command can still be in flight when the next text exchange opens +/// its consumer, and would otherwise be returned as part of the new exchange's response. That is +/// wrong for every caller, but it is actively dangerous for one that infers device liveness from +/// response content: the SD listing accepts a SYSTem:ERRor? reply as proof that the device +/// answered and that the listing before it is complete. A stale line satisfying that check would +/// let a silent device pass as a healthy empty SD card — the exact bug #396 is about. +/// +public class DaqifiDeviceStaleTextLineTests +{ + [Fact] + public async Task ExecuteTextCommand_DropsLinesThatArrivedBeforeTheExchangeSentAnything() + { + // The stale line is released into the stream at the moment the exchange binds its text + // consumer — after the protobuf consumer has been stopped, and before the setup action + // has sent anything. That is exactly the window a late reply to an earlier command can + // land in. The device then stays silent, as one that has stopped answering would. + using var transport = new ReleaseOnStreamAccessMockTransport("0,\"No error\"\r\n"); + using var device = new StaleLineTestableDevice("Preloaded Device", transport); + + device.Connect(); + transport.ReleaseOnStreamAccess(2); // 2nd access inside the exchange = text-consumer bind + + var lines = await device.CallExecuteTextCommandAsync(() => { }); + + // The exchange sent nothing, so nothing in it can legitimately have been answered. + Assert.Empty(lines); + + device.Disconnect(); + } + + [Fact] + public async Task ExecuteTextCommand_KeepsLinesThatArriveAfterTheExchangeSentSomething() + { + // The complement, so the fix cannot be "drop everything": a reply that arrives once the + // setup action has sent its command must still be returned. + using var transport = new ReleaseOnStreamAccessMockTransport("0,\"No error\"\r\n"); + using var device = new StaleLineTestableDevice("Answering Device", transport); + + device.Connect(); + + var lines = await device.CallExecuteTextCommandAsync(() => transport.Release()); + + Assert.Contains(lines, l => l.Contains("No error")); + + device.Disconnect(); + } + + /// Exposes the protected text-exchange entry point. + private class StaleLineTestableDevice : DaqifiDevice + { + public StaleLineTestableDevice(string name, IStreamTransport transport) + : base(name, transport) + { + } + + public Task> CallExecuteTextCommandAsync(Action setupAction) + { + return ExecuteTextCommandAsync(setupAction, responseTimeoutMs: 500, completionTimeoutMs: 150); + } + } + + /// + /// Transport whose stream withholds one canned line until released, and which can arm that + /// release on the Nth access of its property. + /// + /// + /// Keying off the property access — rather than a delay — makes the timing deterministic: + /// the text exchange reads Stream once up front and again when it binds the temporary + /// text consumer, and that second access happens after the protobuf consumer has been stopped + /// (so it cannot swallow the line first) and before the setup action runs. + /// + private sealed class ReleaseOnStreamAccessMockTransport : IStreamTransport + { + private readonly WithheldLineStream _stream; + private int _streamAccessCount; + private int _releaseOnAccess = -1; + private bool _isConnected; + private bool _disposed; + + public ReleaseOnStreamAccessMockTransport(string line) + { + _stream = new WithheldLineStream(line); + } + + public Stream Stream + { + get + { + if (_disposed) throw new ObjectDisposedException(nameof(ReleaseOnStreamAccessMockTransport)); + + var access = Interlocked.Increment(ref _streamAccessCount); + if (_releaseOnAccess > 0 && access == _releaseOnAccess) + { + _stream.Release(); + } + + return _stream; + } + } + + public bool IsConnected => _isConnected && !_disposed; + + public string ConnectionInfo => _isConnected ? "Withheld: Connected" : "Withheld: Disconnected"; + + public event EventHandler? StatusChanged; + + /// Arms the release for the Nth subsequent access of . + public void ReleaseOnStreamAccess(int accessNumber) + { + Interlocked.Exchange(ref _streamAccessCount, 0); + _releaseOnAccess = accessNumber; + } + + /// Releases the withheld line immediately. + public void Release() => _stream.Release(); + + public Task ConnectAsync() => ConnectAsync(null); + + public Task ConnectAsync(ConnectionRetryOptions? retryOptions) + { + if (_disposed) throw new ObjectDisposedException(nameof(ReleaseOnStreamAccessMockTransport)); + _isConnected = true; + StatusChanged?.Invoke(this, new TransportStatusEventArgs(true, ConnectionInfo)); + return Task.CompletedTask; + } + + public Task DisconnectAsync() + { + _isConnected = false; + StatusChanged?.Invoke(this, new TransportStatusEventArgs(false, ConnectionInfo)); + return Task.CompletedTask; + } + + public void Connect() => ConnectAsync().Wait(); + + public void Disconnect() => DisconnectAsync().Wait(); + + public void Dispose() + { + if (_disposed) return; + _isConnected = false; + _disposed = true; + } + + private sealed class WithheldLineStream : Stream + { + private readonly byte[] _line; + private readonly object _gate = new(); + private bool _released; + private int _position; + + public WithheldLineStream(string line) => _line = Encoding.ASCII.GetBytes(line); + + public void Release() + { + lock (_gate) + { + _released = true; + } + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => true; + public override long Length => throw new NotSupportedException(); + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + + public override void Flush() { } + + public override int Read(byte[] buffer, int offset, int count) + { + lock (_gate) + { + if (_released && _position < _line.Length) + { + var toCopy = Math.Min(count, _line.Length - _position); + Array.Copy(_line, _position, buffer, offset, toCopy); + _position += toCopy; + return toCopy; + } + } + + // Idle link: nothing to hand over, and no busy-spin in the reader thread. + Thread.Sleep(10); + return 0; + } + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) { } + } + } +} diff --git a/src/Daqifi.Core/Device/DaqifiDevice.cs b/src/Daqifi.Core/Device/DaqifiDevice.cs index 29a3bf30..13be9768 100644 --- a/src/Daqifi.Core/Device/DaqifiDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiDevice.cs @@ -738,7 +738,11 @@ private void RestartMessageConsumerAfterSwap() /// The time in milliseconds to wait for the first text response after sending commands. /// The time in milliseconds of inactivity after the first response before considering the response complete. Defaults to 250ms. /// A cancellation token to observe while waiting for the task to complete. - /// A list of text lines received from the device. + /// + /// A list of text lines received from the device. Lines that were already in flight when the + /// exchange opened — late replies to earlier commands — are excluded: only what arrived once + /// had begun sending is returned. + /// /// Thrown when the device is not connected or has no transport. /// Thrown when the operation is canceled. protected virtual Task> ExecuteTextCommandAsync( @@ -863,6 +867,10 @@ private async Task> ExecuteTextCommandCoreAsync( var stream = _transport.Stream; int? originalReadTimeout = null; + // Number of lines that were already in flight when this exchange opened — see the + // note at the point it is captured, below. + var staleLineCount = 0; + try { if (stream.CanTimeout) @@ -911,6 +919,23 @@ private async Task> ExecuteTextCommandCoreAsync( SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Text consumer started at {ElapsedMs}ms", sw.ElapsedMilliseconds)); + // Mark the boundary between "already in flight" and "answers to this exchange". + // Anything captured before the setup action has sent anything is a late reply to + // an EARLIER command, or line noise — never a response to a command this exchange + // has yet to send. Those lines are dropped from the result below. + // + // Position matters as much as content: a caller that keys off response content — + // e.g. the SD listing's end-of-listing terminator (#396) — would otherwise accept + // a stale line as proof that the device answered a query it never even received, + // and report a complete listing for a device that has gone silent. + staleLineCount = collectedLines.Count; + if (staleLineCount > 0) + { + SafeLog(() => _logger.LogDebug( + "[ExecuteTextCommandAsync] Discarding {StaleLineCount} line(s) received before this exchange sent anything", + staleLineCount)); + } + // Execute the setup action (sends SCPI commands). ConfigureAwait(false) // matches the surrounding lock-protected awaits. await setupActionAsync(cancellationToken).ConfigureAwait(false); @@ -984,7 +1009,11 @@ private async Task> ExecuteTextCommandCoreAsync( SafeLog(() => _logger.LogDebug("[ExecuteTextCommandAsync] Total elapsed: {ElapsedMs}ms", sw.ElapsedMilliseconds)); } - return collectedLines; + // The text consumer is stopped by this point, so the list is no longer being + // appended to concurrently and can be re-projected safely. + return staleLineCount > 0 + ? collectedLines.Skip(staleLineCount).ToList() + : collectedLines; } finally { diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 8383f81b..fe32dfd5 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -48,6 +48,19 @@ public class DaqifiStreamingDevice : DaqifiDevice, IStreamingDevice, INetworkCon /// private const int SD_LIST_MAX_RETRIES = 1; + /// + /// Inactivity window that ends the SD listing text exchange, in milliseconds. + /// + /// + /// Deliberately longer than the 250ms default. The listing is only accepted once its + /// end-of-listing terminator has been seen (see ), and the + /// terminator can trail the last listing line by more than the default window — the firmware + /// walks the directory tree between chunks, and a congested WiFi link adds its own gaps. With + /// the default, a merely-slow terminator would read as a missing one and fail a listing that + /// was about to complete. + /// + private const int SD_LIST_COMPLETION_TIMEOUT_MS = 1000; + /// /// Maximum number of retry attempts for the USB stream-interface command sent during /// when the device returns a transient SCPI error @@ -1655,7 +1668,10 @@ public async Task> GetSdCardFilesAsync(Cancellatio // End-of-listing terminator — see this method's remarks. Sent inside the // same text exchange so the ordering guarantee holds. Send(ScpiMessageProducer.GetSystemError); - }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); + }, + responseTimeoutMs: 3000, + completionTimeoutMs: SD_LIST_COMPLETION_TIMEOUT_MS, + cancellationToken: cancellationToken); isComplete = TrySplitAtSdListTerminator(lines, out listing); From a82c3afea48de939603029dfabb8adb417f423c5 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 09:54:19 -0600 Subject: [PATCH 3/5] fix(sdcard): move the SD bus switch and settle delay outside the text exchange Addresses the round-2 Qodo finding: the stale-line boundary is taken before the setup action runs, so a gap INSIDE that action still left a window where a late reply to an earlier command could be mistaken for this listing's terminator. For the listing that gap was the 100ms SPI settle delay. Doing the switch and its settle before the exchange opens leaves the exchange with no internal gap at all -- its first act is now the LIST query itself, so the boundary collapses to thread scheduling rather than a timed wait. From the device's side the delay is unchanged, if anything longer, since the consumer swap now follows it. Bench: listing returns all 31 files across three consecutive runs; SD storage and a 10 Hz two-channel stream unaffected. Co-Authored-By: Claude Opus 5 --- .../Device/DaqifiStreamingDevice.cs | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index fe32dfd5..da82a0d8 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -1613,6 +1613,13 @@ private void EnsureSdFileTransferSupportedOnTransport() /// a plausible-looking empty list. /// /// + /// The terminator is only meaningful if it cannot be confused with a late reply to an + /// earlier command, so two things guard that boundary: the text exchange discards whatever + /// was already in flight when it opened, and this method does its SPI-bus switch and settle + /// delay before the exchange rather than inside it, leaving the exchange with no internal + /// gap for a stale reply to slip into. + /// + /// /// The terminator's error code is used only as a liveness marker, never for classification: /// the queue it pops can hold entries left by earlier commands, so attributing the code to /// this listing would misreport stale failures. SD errors continue to be classified from the @@ -1654,24 +1661,31 @@ public async Task> GetSdCardFilesAsync(Cancellatio await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); } - lines = await ExecuteTextCommandAsync(async ct => - { - PrepareSdInterface(); - - // Allow the device firmware to complete the SPI bus switch - // before querying the SD card. Without this delay, the device - // can return SCPI error -200 (Execution error). - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - - Send(ScpiMessageProducer.GetSdFileList); - - // End-of-listing terminator — see this method's remarks. Sent inside the - // same text exchange so the ordering guarantee holds. - Send(ScpiMessageProducer.GetSystemError); - }, - responseTimeoutMs: 3000, - completionTimeoutMs: SD_LIST_COMPLETION_TIMEOUT_MS, - cancellationToken: cancellationToken); + // Switch the shared SPI bus over to the SD card and let the firmware settle + // BEFORE opening the text exchange. Querying the card too soon after the switch + // makes the device answer -200 (Execution error), so the delay itself is not + // optional — but running it outside the exchange leaves the exchange with no + // internal gap at all, so its very first act is the LIST query. That matters for + // the terminator: the exchange discards anything received before its setup + // action sends, and a gap inside the action would widen that boundary into a + // window where a late reply to an earlier command could still be mistaken for + // this listing's terminator. The delay is unchanged from the device's point of + // view — if anything longer, since the consumer swap now follows it. + PrepareSdInterface(); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); + + lines = await ExecuteTextCommandAsync( + () => + { + Send(ScpiMessageProducer.GetSdFileList); + + // End-of-listing terminator — see this method's remarks. Sent inside + // the same text exchange so the ordering guarantee holds. + Send(ScpiMessageProducer.GetSystemError); + }, + responseTimeoutMs: 3000, + completionTimeoutMs: SD_LIST_COMPLETION_TIMEOUT_MS, + cancellationToken: cancellationToken); isComplete = TrySplitAtSdListTerminator(lines, out listing); From 2d7952fef22de0fb98ce3fcd914a3cceb16585a4 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 10:12:52 -0600 Subject: [PATCH 4/5] fix(sdcard): restore ConfigureAwait(false) on the SD listing settle delays The settle delay kept ConfigureAwait(false) while it lived inside the setup action lambda and lost it when moved out in a82c3af. Restored, and applied to the retry-gap delay in the same loop for consistency: neither wait has any reason to resume on a captured synchronization context, and doing so makes a sync-over-async caller more deadlock-prone. Co-Authored-By: Claude Opus 5 --- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index da82a0d8..2fd73499 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -1658,7 +1658,7 @@ public async Task> GetSdCardFilesAsync(Cancellatio { cancellationToken.ThrowIfCancellationRequested(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken).ConfigureAwait(false); } // Switch the shared SPI bus over to the SD card and let the firmware settle @@ -1672,7 +1672,7 @@ public async Task> GetSdCardFilesAsync(Cancellatio // this listing's terminator. The delay is unchanged from the device's point of // view — if anything longer, since the consumer swap now follows it. PrepareSdInterface(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken).ConfigureAwait(false); lines = await ExecuteTextCommandAsync( () => From 0c2f9c01a733845016d30534f315c452558fb58c Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 10:24:02 -0600 Subject: [PATCH 5/5] fix(sdcard): remove the same setup-action gap from DeleteSdCardFileAsync The delete path still had the exact pattern removed from the listing in a82c3af: PrepareSdInterface() plus an awaited settle delay INSIDE the setup action, at both the first-attempt and retry call sites. That gap is a window in which a late reply to an earlier command is captured as part of this response. The consequence here is milder than the listing's -- delete keys off ContainsScpiError, so a stale error line causes a spurious delete-and-relist retry rather than an incomplete listing read as complete -- but it is the same defect, so it gets the same fix. These were the last two async setup actions in the file; every call site is now a synchronous lambda with no internal gap. Co-Authored-By: Claude Opus 5 --- .../Device/DaqifiStreamingDevice.cs | 44 ++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 2fd73499..625386fa 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -2122,13 +2122,23 @@ public async Task DeleteSdCardFileAsync(string fileName, CancellationToken cance IReadOnlyList lines; try { - lines = await ExecuteTextCommandAsync(async ct => - { - PrepareSdInterface(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - Send(ScpiMessageProducer.DeleteSdFile(fileName)); - Send(ScpiMessageProducer.GetSdFileList); - }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); + // Switch the shared SPI bus to the SD card and settle BEFORE opening the text + // exchange, for the same reason as GetSdCardFilesAsync: a gap inside the setup + // action is a window in which a late reply to an earlier command can be captured + // as part of this response. Here that would mean a stale error line triggering a + // pointless delete-and-relist retry rather than a bad listing, but it is the same + // defect, so it gets the same treatment. + PrepareSdInterface(); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken).ConfigureAwait(false); + + lines = await ExecuteTextCommandAsync( + () => + { + Send(ScpiMessageProducer.DeleteSdFile(fileName)); + Send(ScpiMessageProducer.GetSdFileList); + }, + responseTimeoutMs: 3000, + cancellationToken: cancellationToken); if (ContainsScpiError(lines)) { @@ -2136,15 +2146,19 @@ public async Task DeleteSdCardFileAsync(string fileName, CancellationToken cance { cancellationToken.ThrowIfCancellationRequested(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken).ConfigureAwait(false); - lines = await ExecuteTextCommandAsync(async ct => - { - PrepareSdInterface(); - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - Send(ScpiMessageProducer.DeleteSdFile(fileName)); - Send(ScpiMessageProducer.GetSdFileList); - }, responseTimeoutMs: 3000, cancellationToken: cancellationToken); + PrepareSdInterface(); + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken).ConfigureAwait(false); + + lines = await ExecuteTextCommandAsync( + () => + { + Send(ScpiMessageProducer.DeleteSdFile(fileName)); + Send(ScpiMessageProducer.GetSdFileList); + }, + responseTimeoutMs: 3000, + cancellationToken: cancellationToken); if (!ContainsScpiError(lines)) {