From ff1176f75f1958fa5b5696105ed8164f454e7156 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sat, 18 Jul 2026 20:50:02 -0600 Subject: [PATCH 1/3] refactor(device): consolidate SCPI error-code extraction into ScpiResponseClassifier (part of #345) DaqifiStreamingDevice.TryParseScpiErrorCode independently re-derived the **ERROR/ERROR prefix + delimiter-trim logic that ScpiResponseClassifier already owns for line *detection*, risking drift in the accepted delimiter set. The extraction now lives next to the matchers as ScpiResponseClassifier.TryExtractErrorCode, with a single-sourced TokenDelimiters set; the device's two call sites delegate to it and the private duplicate is removed. Behavior-preserving. Addresses acceptance-criterion 3 of #345 (the last of the three batch items). - 12 new TryExtractErrorCode tests (all delimiter variants, no-comma, positive code, whitespace/CRLF; non-numeric/non-error -> false + code 0). Full suite 1647 pass / 0 fail (net9+net10). Co-Authored-By: Claude Opus 4.8 --- .../Device/ScpiResponseClassifierTests.cs | 26 +++++++++++ .../Device/DaqifiStreamingDevice.cs | 39 +--------------- .../Device/ScpiResponseClassifier.cs | 44 +++++++++++++++++++ 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs b/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs index d86f9aaf..8558c4a2 100644 --- a/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs +++ b/src/Daqifi.Core.Tests/Device/ScpiResponseClassifierTests.cs @@ -29,5 +29,31 @@ public void IsScpiErrorLine_DoesNotMatchNonScpiText(string line) { Assert.False(ScpiResponseClassifier.IsScpiErrorLine(line)); } + + [Theory] + [InlineData("**ERROR: -200, \"Execution error\"", -200)] + [InlineData("**ERROR -200, \"Execution error\"", -200)] + [InlineData("**ERROR\t-200, \"Execution error\"", -200)] + [InlineData("ERROR: -113, \"Undefined header\"", -113)] + [InlineData("ERROR -113", -113)] // no trailing comma + [InlineData(" ERROR: -200, \"x\" \r\n", -200)] // leading/trailing whitespace + CRLF + [InlineData("ERROR: 42, \"positive\"", 42)] // positive code + public void TryExtractErrorCode_ExtractsCode_AcrossDelimiterVariants(string line, int expected) + { + Assert.True(ScpiResponseClassifier.TryExtractErrorCode(line, out var code)); + Assert.Equal(expected, code); + } + + [Theory] + [InlineData("Error !! No SD Card Detected")] // ERROR token but non-numeric follow + [InlineData("error_log.bin")] // filename + [InlineData("Errors.txt")] // filename + [InlineData("OK")] // not an error line + [InlineData("")] + public void TryExtractErrorCode_ReturnsFalseAndZero_ForNonNumericOrNonError(string line) + { + Assert.False(ScpiResponseClassifier.TryExtractErrorCode(line, out var code)); + Assert.Equal(0, code); + } } } diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 24187f20..2e4c4874 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -1347,7 +1347,7 @@ public async Task GetSdCardStorageAsync(CancellationToken can // query at all — typically because it predates the version that introduced it — so // it gets the typed feature-gating exception instead of a generic operation error. if (lastScpiError != null - && TryParseScpiErrorCode(lastScpiError, out var scpiErrorCode) + && ScpiResponseClassifier.TryExtractErrorCode(lastScpiError, out var scpiErrorCode) && scpiErrorCode == ScpiErrorCodeUndefinedHeader) { throw new FeatureNotSupportedException( @@ -1846,41 +1846,6 @@ private static bool IsScpiErrorLine(string line) return ScpiResponseClassifier.IsScpiErrorLine(line); } - /// - /// Parses the numeric error code out of a SCPI error line matched by - /// — e.g. **ERROR: -113, "Undefined header", - /// ERROR: -113,"Undefined header", or a space/tab-delimited variant like - /// **ERROR -113, "Undefined header". The delimiter between the ERROR/ - /// **ERROR token and the code may be :, space, or tab — matching the - /// delimiters accepts — and the code is the text - /// up to the following comma (if any). - /// - private static bool TryParseScpiErrorCode(string line, out int code) - { - code = 0; - var trimmed = line.TrimStart(); - - string afterToken; - if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) - { - afterToken = trimmed[7..]; - } - else if (trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) - { - afterToken = trimmed[5..]; - } - else - { - return false; - } - - afterToken = afterToken.TrimStart(':', ' ', '\t'); - - var commaIndex = afterToken.IndexOf(','); - var codeSpan = (commaIndex >= 0 ? afterToken[..commaIndex] : afterToken).Trim(); - return int.TryParse(codeSpan, NumberStyles.Integer, CultureInfo.InvariantCulture, out code); - } - // 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 @@ -1998,7 +1963,7 @@ private static void ValidateSdCardFileName(string fileName) // so the caller's retry loop can react (kick LAN:APPLY) instead of just // waiting out a blind delay. var errorLine = lines.LastOrDefault(IsScpiErrorLine); - if (errorLine != null && TryParseScpiErrorCode(errorLine, out var errorCode) && errorCode == -200) + if (errorLine != null && ScpiResponseClassifier.TryExtractErrorCode(errorLine, out var errorCode) && errorCode == -200) { throw new LanNotInitializedException(errorLine.Trim()); } diff --git a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs index dd20ea6b..915ff658 100644 --- a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs +++ b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs @@ -1,4 +1,5 @@ using System; +using System.Globalization; #nullable enable @@ -50,6 +51,49 @@ internal static bool IsScpiErrorLine(string line) || MatchesStrictScpiErrorPrefix(trimmed, "ERROR"); } + /// + /// Extracts the numeric error code from a SCPI error line — e.g. -200 from + /// **ERROR: -200,"Execution error", ERROR -113,"Undefined header", or + /// **ERROR\t-113,.... The delimiter between the ERROR/**ERROR token and + /// the code may be :, space, or tab — the same set the line matchers above accept, kept + /// here so the accepted delimiters can't drift between detection and extraction. The code is + /// the text up to the following comma (if any). + /// + /// The candidate error line. + /// The parsed error code when the method returns true; otherwise 0. + /// true if a numeric error code was extracted; otherwise false. + internal static bool TryExtractErrorCode(string line, out int code) + { + code = 0; + var trimmed = line.TrimStart(); + + string afterToken; + if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + { + afterToken = trimmed[7..]; + } + else if (trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) + { + afterToken = trimmed[5..]; + } + else + { + return false; + } + + afterToken = afterToken.TrimStart(TokenDelimiters); + + var commaIndex = afterToken.IndexOf(','); + var codeSpan = (commaIndex >= 0 ? afterToken[..commaIndex] : afterToken).Trim(); + return int.TryParse(codeSpan, NumberStyles.Integer, CultureInfo.InvariantCulture, out code); + } + + /// + /// The delimiters accepted between the ERROR/**ERROR token and the error code. + /// Single-sourced here so and the line matchers stay in lockstep. + /// + private static readonly char[] TokenDelimiters = { ':', ' ', '\t' }; + private static bool MatchesErrorPrefix(string trimmed, string prefix) { if (trimmed.Length < prefix.Length From cc8fc91a85bb0d059fe09a7acdd3f75aeef58686 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sat, 18 Jul 2026 21:07:53 -0600 Subject: [PATCH 2/3] refactor(device): route error-line matchers through IsTokenDelimiter (single delimiter source) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Qodo review on #357: TokenDelimiters was documented as the single source of truth but only TryExtractErrorCode used it — the prefix matchers still hard-coded ':'/' '/'\t'. Both matchers now consult a shared IsTokenDelimiter(char) helper, so a future delimiter change can't drift detection from extraction. Behavior-preserving (in the strict matcher ':' is handled before the check, so !IsTokenDelimiter(next) reduces exactly to the prior "not space/tab" test). Co-Authored-By: Claude Opus 4.8 --- src/Daqifi.Core/Device/ScpiResponseClassifier.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs index 915ff658..4f69c395 100644 --- a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs +++ b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs @@ -90,10 +90,14 @@ internal static bool TryExtractErrorCode(string line, out int code) /// /// The delimiters accepted between the ERROR/**ERROR token and the error code. - /// Single-sourced here so and the line matchers stay in lockstep. + /// Single-sourced here and consulted via by both + /// and the line matchers, so detection and extraction can't drift. /// private static readonly char[] TokenDelimiters = { ':', ' ', '\t' }; + /// Returns true if is one of the accepted . + private static bool IsTokenDelimiter(char c) => Array.IndexOf(TokenDelimiters, c) >= 0; + private static bool MatchesErrorPrefix(string trimmed, string prefix) { if (trimmed.Length < prefix.Length @@ -102,7 +106,7 @@ private static bool MatchesErrorPrefix(string trimmed, string prefix) if (trimmed.Length == prefix.Length) return true; var next = trimmed[prefix.Length]; - if (next == ':' || next == ' ' || next == '\t') + if (IsTokenDelimiter(next)) return true; // Single '!' is ambiguous (could be a filename like "error!log.bin"). // Require '!!' so we still catch firmware "Error!!" status text but @@ -123,7 +127,8 @@ private static bool MatchesStrictScpiErrorPrefix(string trimmed, string prefix) var next = trimmed[prefix.Length]; if (next == ':') return true; - if (next != ' ' && next != '\t') + // ':' already returned above, so this rejects anything that isn't a space/tab delimiter. + if (!IsTokenDelimiter(next)) return false; // A space/tab delimiter alone is ambiguous — firmware status text like From 55f468e95bce68f70ca5babd230c245f112cea82 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Sat, 18 Jul 2026 21:27:05 -0600 Subject: [PATCH 3/3] docs(device): correct TokenDelimiters comment (extractor trims directly, matchers use IsTokenDelimiter) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Qodo review on #357: the comment claimed TryExtractErrorCode consults IsTokenDelimiter, but it trims with the TokenDelimiters array directly; only the matchers use IsTokenDelimiter. Reworded to describe each accurately — both still draw from the one array. Doc-only, no behavior change. Co-Authored-By: Claude Opus 4.8 --- src/Daqifi.Core/Device/ScpiResponseClassifier.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs index 4f69c395..1edfcd3b 100644 --- a/src/Daqifi.Core/Device/ScpiResponseClassifier.cs +++ b/src/Daqifi.Core/Device/ScpiResponseClassifier.cs @@ -90,8 +90,9 @@ internal static bool TryExtractErrorCode(string line, out int code) /// /// The delimiters accepted between the ERROR/**ERROR token and the error code. - /// Single-sourced here and consulted via by both - /// and the line matchers, so detection and extraction can't drift. + /// Single-sourced here: the line matchers test membership via , and + /// trims with this array directly — both draw from this one set, + /// so detection and extraction can't drift. /// private static readonly char[] TokenDelimiters = { ':', ' ', '\t' };