diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs index e59b00e..1910019 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs @@ -68,6 +68,134 @@ public async Task GetSdCardFilesAsync_ParsesResponseCorrectly() Assert.Null(files[1].CreatedDate); } + [Fact] + public async Task GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified() + { + // Regression for #190 covering BOTH bug locations: + // - IsNonResultLine in DaqifiStreamingDevice (the LIST? response classifier) + // - SdCardFileListParser.ParseFileList (the per-line parser; bare "ERROR" check) + // Pre-fix, both used a bare "ERROR" StartsWith check that + // false-positived on legit SD filenames. Tightened to require + // ERROR followed by ":"/" "/"!"/tab/end-of-line. + // + // Cover both path shapes the firmware may emit: + // - prefixed: "Daqifi/error_log.csv" + // - bare: "error_log.csv" (no Daqifi/ prefix) + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "Daqifi/error_log.csv", + "Daqifi/Errors_summary.bin", + "error_log.csv", + "Errors_summary.bin", + "ERROR_archive.bin", + "Daqifi/normal.bin", + }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + var names = files.Select(f => f.FileName).ToList(); + Assert.Contains("error_log.csv", names); + Assert.Contains("Errors_summary.bin", names); + Assert.Contains("ERROR_archive.bin", names); + Assert.Contains("normal.bin", names); + } + + [Fact] + public async Task GetSdCardFilesAsync_OnlyErrorPrefixedFilenames_AllSurvive() + { + // Edge case explicitly called out by Qodo on PR #195: a + // listing consisting SOLELY of error*-prefixed filenames + // (no normal.bin to act as a sanity anchor) must round-trip + // every entry. Pre-fix, the entire response would have parsed + // as zero files. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "error_log.csv", + "errors.bin", + "Erroneous_data.bin", + }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + Assert.Equal(3, files.Count); + var names = files.Select(f => f.FileName).ToList(); + Assert.Contains("error_log.csv", names); + Assert.Contains("errors.bin", names); + Assert.Contains("Erroneous_data.bin", names); + } + + [Theory] + [InlineData("**ERROR: -200, Execution error")] + [InlineData("**Error: bad")] + [InlineData("ERROR: -100, Bad command")] + [InlineData("Error !! Generic firmware status")] + [InlineData("Error!! No space firmware status")] + [InlineData("ERROR")] + [InlineData("error\tsomething")] + public async Task GetSdCardFilesAsync_RealErrorLinesStillSkipped(string errorLine) + { + // Confirm the tightening didn't go too far — real error + // lines still classify as non-result and don't end up + // misinterpreted as filenames. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "Daqifi/normal.bin", + errorLine, + }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + Assert.Single(files); + Assert.Equal("normal.bin", files[0].FileName); + } + + [Theory] + [InlineData("error!log.bin")] + [InlineData("Daqifi/error!log.bin")] + [InlineData("Erroneous!data.bin")] + public async Task GetSdCardFilesAsync_FilenamesWithSingleBangSurvive(string filename) + { + // Regression: a single '!' immediately after "error" is ambiguous + // (could be a filename like "error!log.bin"). The classifier must + // require '!!' to treat as an error/status line so legitimate + // filenames aren't dropped from listings. Filename validation + // already permits '!' in SD filenames. + var device = new TestableSdCardStreamingDevice("TestDevice"); + device.CannedTextResponse = new List + { + "Daqifi/normal.bin", + filename, + }; + device.Connect(); + + var files = await device.GetSdCardFilesAsync(); + + var names = files.Select(f => f.FileName).ToList(); + Assert.Equal(2, names.Count); + Assert.Contains("normal.bin", names); + // Mirror production normalization: strip the Daqifi/ prefix then keep + // the basename. Split on '/' explicitly (not Path.GetFileName) — the + // device protocol uses forward slashes, and Path.GetFileName treats + // '\\' as a separator on Windows but not on Linux/macOS, which would + // make this expectation OS-dependent if a future case used '\\'. + const string daqifiPrefix = "Daqifi/"; + var expected = filename.StartsWith(daqifiPrefix, StringComparison.OrdinalIgnoreCase) + ? filename.Substring(daqifiPrefix.Length) + : filename; + var lastSlash = expected.LastIndexOf('/'); + if (lastSlash >= 0) + { + expected = expected.Substring(lastSlash + 1); + } + Assert.Contains(expected, names); + } + [Fact] public async Task GetSdCardFilesAsync_RestoresLanInterface() { diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 72238d7..5c7a362 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -749,12 +749,12 @@ private static bool IsScpiErrorLine(string line) // Permissive: any line that looks like a device error or status message, // including firmware text such as "Error !! ...". Used to recognize that // the parser would yield no result, without polluting LastScpiError with - // non-SCPI text. + // non-SCPI text. Shared classifier so the SD-response rule (closes #190 + // — filenames starting with "error_" must NOT match) stays in lockstep + // across both call sites. private static bool IsNonResultLine(string line) { - var trimmed = line.TrimStart(); - return trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase) - || trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase); + return ScpiResponseClassifier.IsErrorResponseLine(line); } /// diff --git a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs new file mode 100644 index 0000000..af5d1f1 --- /dev/null +++ b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs @@ -0,0 +1,53 @@ +using System; + +#nullable enable + +namespace Daqifi.Core.Device +{ + /// + /// Shared classification for SCPI text response lines. Used by both the + /// general streaming-device response handling and SD card listing parsing + /// so the error/status line rule stays consistent across call sites. + /// + internal static class ScpiResponseClassifier + { + /// + /// Returns true if the line is a non-result error/status line that + /// response parsers should drop. Matches both SCPI error responses + /// (canonical **ERROR marker, bare ERROR token followed + /// by a SCPI delimiter : / space / tab / end-of-line) and + /// firmware status text (Error !! ... with space, or the + /// no-space Error!! form). A double-! is required when + /// no other delimiter is present so legitimate filenames like + /// error!log.bin aren't dropped — single ! alone is + /// ambiguous between error-status and filename. Trims both ends so + /// a bare "ERROR\r" from CRLF line endings still classifies. + /// Plain filenames whose basename starts with error / + /// Errors pass through unmatched (closes #190). + /// + internal static bool IsErrorResponseLine(string line) + { + var trimmed = line.Trim(); + return MatchesErrorPrefix(trimmed, "**ERROR") + || MatchesErrorPrefix(trimmed, "ERROR"); + } + + private static bool MatchesErrorPrefix(string trimmed, string prefix) + { + if (trimmed.Length < prefix.Length + || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + return false; + if (trimmed.Length == prefix.Length) + return true; + var next = trimmed[prefix.Length]; + if (next == ':' || next == ' ' || next == '\t') + return true; + // Single '!' is ambiguous (could be a filename like "error!log.bin"). + // Require '!!' so we still catch firmware "Error!!" status text but + // let plain filenames pass through. + return next == '!' + && trimmed.Length > prefix.Length + 1 + && trimmed[prefix.Length + 1] == '!'; + } + } +} diff --git a/src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs b/src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs index a9d5fea..06643e2 100644 --- a/src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs +++ b/src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs @@ -40,9 +40,11 @@ public static IReadOnlyList ParseFileList(IEnumerable li var path = line.Trim(); - // Skip SCPI error responses: "**ERROR: -200, ..." or "ERROR: -200, ..." - if (path.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase) || - path.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) + // Skip SCPI error responses ("**ERROR: -200, ...", "ERROR: -200, ...") + // and firmware status text ("Error !! ..."). Classification rule lives + // in ScpiResponseClassifier so it stays consistent with + // DaqifiStreamingDevice.IsNonResultLine (closes #190). + if (ScpiResponseClassifier.IsErrorResponseLine(path)) { continue; }