Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>
{
"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<string>
{
"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<string>
{
"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<string>
{
"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()
{
Expand Down
8 changes: 4 additions & 4 deletions src/Daqifi.Core/Device/DaqifiStreamingDevice.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <summary>
Expand Down
53 changes: 53 additions & 0 deletions src/Daqifi.Core/Device/ScpiResponseClassifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using System;

#nullable enable

namespace Daqifi.Core.Device
{
/// <summary>
/// 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.
/// </summary>
internal static class ScpiResponseClassifier
{
/// <summary>
/// Returns true if the line is a non-result error/status line that
/// response parsers should drop. Matches both SCPI error responses
/// (canonical <c>**ERROR</c> marker, bare <c>ERROR</c> token followed
/// by a SCPI delimiter <c>:</c> / space / tab / end-of-line) and
/// firmware status text (<c>Error !! ...</c> with space, or the
/// no-space <c>Error!!</c> form). A double-<c>!</c> is required when
/// no other delimiter is present so legitimate filenames like
/// <c>error!log.bin</c> aren't dropped — single <c>!</c> alone is
/// ambiguous between error-status and filename. Trims both ends so
/// a bare <c>"ERROR\r"</c> from CRLF line endings still classifies.
/// Plain filenames whose basename starts with <c>error</c> /
/// <c>Errors</c> pass through unmatched (closes #190).
/// </summary>
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] == '!';
}
}
}
8 changes: 5 additions & 3 deletions src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ public static IReadOnlyList<SdCardFileInfo> ParseFileList(IEnumerable<string> 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;
}
Expand Down
Loading