refactor(device): extract raw-command session interpretation into a collaborator (part of #344)

[tylerkron]

Problem

DaqifiStreamingDevice is one of the two god-classes #344 exists to decompose. The block this PR takes on is the #379 session tracking — the code that keeps Core's view of a streaming session in step when the session is driven through the raw Send path instead of the typed API (which is exactly what the example CLI does).

It sat inline as three private methods and three private constants, and it mixed two different kinds of thing:

• the decision — is this command text a start, a stop, or an ADC enable mask, and is its argument usable?
• the effects — re-anchor the session, flip IsStreaming, assign IChannel.IsEnabled under the device's channels lock.

Only the second kind actually needs a device. The first is pure text-and-arithmetic, but because it lived on the device it could only be tested by constructing a scripted device and driving Send through it.

Fix

Device/Internal/SessionCommandInterpreter now makes the decision: a command string plus the device's sampling ceiling go in, a typed SessionCommandEffect comes out (None / StopStreaming / StartStreaming(frequency) / UnusableStreamingStart(rejectedRate) / SetAdcEnableMask(mask)). DaqifiStreamingDevice.TrackSessionCommand becomes a switch that applies the effect.

UnusableStreamingStart is its own kind rather than folded into None so the device can still trace the rejection with the text that caused it — the diagnostic that existed before is preserved, not dropped.

The single-read property is preserved deliberately. MaxSamplingRate is a mutable public property, so the ceiling is read once by the device and handed to the interpreter, and the validated value comes back to be assigned to the backing field rather than through the validating StreamingFrequency setter. Validating against one read and assigning through a setter that takes another is what would let a concurrent capabilities update throw out of a Send whose command has already reached the device — the existing TrackingARate_NeverThrowsOutOfSend_WhileCapabilitiesChangeUnderneath test drives that window and stays green.

No public API change and no behavior change. DaqifiStreamingDevice.cs drops from 1385 to 1310 lines.

Tests

• 31 new SessionCommandInterpreterTests, covering the decision directly: blank and unrelated commands, case-insensitive matching (SCPI short/long forms differ only in case), surrounding whitespace, the stop-vs-start prefix ambiguity (both start SYSTem:St, so the check order is load-bearing), every unusable-rate shape, an inclusive ceiling, the Math.Max(1, ...) fallback for an uninitialized MaxSamplingRate, mask parsing including uint.MaxValue and the unparseable cases, and a guard that the constants still match what ScpiMessageProducer actually emits.
• The existing DeviceReconnectTests coverage of the effects is unchanged and green — it is what pins that the device still applies the decision correctly.
• FULL suite green on net9.0 and net10.0: 2623 passed, 2 skipped, 0 warnings (plus 23 in Daqifi.Mcp.Tests).

Bench validation

Run on the real Nyquist over USB (fw 3.7.2). The example CLI drives ENAble:VOLTage:DC, SYSTem:StartStreamData and SYSTem:StopStreamData through raw Send, so a bench stream exercises this interpreter end to end rather than only in unit tests:

• mask 3 at 200 Hz → two analog values per frame; mask 1 at 100 Hz → one. The raw enable mask is still being applied to the channel set.
• 396 samples over 5 s, timestamps strictly monotonic with uniform deltas from the very first frame — the session re-anchored correctly, which is the thing that breaks if a raw start stops being recognized.
• --min-samples 300 gate passed, exit 0. Clean start and stop, no discarded frames or decode failures reported.

Non-destructive throughout: no SD, firmware, reboot or power-cycle operations.

Part of #344.

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Extract raw SCPI session-command interpretation into SessionCommandInterpreter

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Extract raw Send-path SCPI session command parsing into an internal interpreter.
• Keep DaqifiStreamingDevice responsible for applying session/channel state effects.
• Add unit tests covering parsing, validation, and ADC enable mask handling.

Diagram

graph TD
  A["DaqifiDevice.Send"] --> B["DaqifiStreamingDevice.TrackSessionCommand"] --> C["SessionCommandInterpreter.Interpret"] --> D["SessionCommandEffect"] --> E["Apply effect"]
  E --> F["Session state"]
  E --> G["Channel enable state"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Derive command strings directly from ScpiMessageProducer

• ➕ Single source of truth for SCPI command text (no duplication).
• ➕ Reduces risk of drift without needing a dedicated guard test.
• ➖ May introduce undesirable coupling/dependency direction between internal device logic and message producer layer.
• ➖ Harder to keep the interpreter as a pure, isolated component if it starts depending on producer APIs.

2. Introduce a small SCPI tokenizer/parser and interpret structured tokens

• ➕ More robust parsing (handles spacing/formatting variations consistently).
• ➕ Easier to extend to additional commands without fragile string prefix matching.
• ➖ More code and complexity than needed for a narrowly-scoped session tracker.
• ➖ Higher maintenance cost; likely overkill for recognizing a few commands.

Recommendation: The PR’s approach is a good balance: it keeps interpretation pure and testable while leaving device-side effects (locking, anchoring, state flips) in the device. Duplicating the SCPI command strings is acceptable here because the new tests explicitly guard against drift, and the separation preserves the single-read ceiling behavior that prevents Send from failing due to concurrent capability updates.

Files changed (3) +453 / -122

Refactor (2) +274 / -122

DaqifiStreamingDevice.csRefactor session tracking to apply typed effects from interpreter +47/-122

Refactor session tracking to apply typed effects from interpreter

• Replaces inline string parsing and argument validation with a call to SessionCommandInterpreter and a switch over the resulting SessionCommandEffectKind. Preserves the single-read MaxSamplingRate behavior by assigning the validated rate to the backing field, and keeps device-owned side effects (BeginStreamingSession, IsStreaming changes, channel updates) local.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

SessionCommandInterpreter.csAdd SessionCommandInterpreter and typed SessionCommandEffect model +227/-0

Add SessionCommandInterpreter and typed SessionCommandEffect model

• Adds a pure interpreter that recognizes the SCPI commands defining a streaming session (start/stop/ADC enable mask) and returns a typed effect. Implements rate validation with an inclusive ceiling and a floor of 1 for unusable max-rate values, and returns a distinct UnusableStreamingStart effect to allow caller tracing without mutating session state.

src/Daqifi.Core/Device/Internal/SessionCommandInterpreter.cs

Tests (1) +179 / -0

SessionCommandInterpreterTests.csAdd focused unit tests for session-command interpretation +179/-0

Add focused unit tests for session-command interpretation

• Introduces a new test suite that directly pins the command-to-effect decision logic for raw SCPI commands. Covers blank/unrelated commands, case-insensitive matching and whitespace, start/stop ambiguity, rate validation (including inclusive ceiling and invalid ceilings), and ADC enable mask parsing and failure modes.

src/Daqifi.Core.Tests/Device/Internal/SessionCommandInterpreterTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[tylerkron]

Ready for review.

Qodo came back clean (0 bugs, 0 rule violations, 0 requirement gaps, no inline threads). CI build is green on the current head 06062a7, which includes a merge of main after #436 and #434 landed — that merge was conflict-free, and the full suite was re-run on the merged result: 2653 passed / 2 skipped on both net9.0 and net10.0, 0 warnings.

Worth noting alongside #436: that PR extracted the device-administration block from the same file, and this one extracts the session-command interpretation. They touch disjoint regions, so the two composed without conflict. With both in, DaqifiStreamingDevice.cs is down to 1196 lines (from 1385 before #436).

Not merging — leaving this for your review.