refactor(device): extract the USB stream-interface routing decision into a collaborator (part of #344)

[tylerkron]

Part of #344. Another incremental slice of the DaqifiStreamingDevice decomposition — same shape as #439 and #441: host-free, no IDeviceOperationHost additions, no public surface change.

Problem

OnDeviceInitializingAsync carried the entire USB stream-routing policy inline, mixed in with the send itself:

• whether to route at all (skip on non-USB; skip on an observe-only session, per Second connection to a streaming device silently stops the first session's stream (InitializeAsync sends StopStreaming unconditionally) #385)
• how many times to retry a transient SCPI rejection, and how long to settle between attempts (fix: InitializeAsync should retry/type the transient SCPI-error-during-init exception #310)
• how to turn a rejection that persists across every attempt into a typed ScpiInitializationErrorException

None of that needs a device to decide, but all of it was only reachable through a connected device with a transport, so every test of the policy had to stand up a fake device end to end.

Fix

New Device/Internal/UsbStreamInterfaceInitializer. It takes the two connection facts the decision depends on (isUsbConnection, preserveActiveStream) plus a delegate that performs the send.

Decision moves out. Route-or-skip, retry budget, settle delay, error classification, and the typed throw.

Effect stays on the device. The hook still builds the ExecuteTextCommandAsync call, so the command goes out in text mode with the protobuf consumer stopped and any SCPI error response is captured rather than garbling the protobuf stream. The 500 ms response window stayed on the device too — it is a call-shape detail of the device's own helper, not part of the routing policy — but it is now a named constant instead of an inline literal.

The documented deliberate behavior of not observing the cancellation token on the skip path is preserved and is now pinned by a test rather than only by a comment.

DaqifiStreamingDevice.cs: 1196 → 1144 lines.

Conflict surface

Deliberately chosen to be disjoint from the two open slices. This PR's hunks are at L33-44 and L224-309; #440's are at L15-23 / L196-203 / L633-700 / L942, and #441's are at L515-629. No overlap, and no shared new files.

Testing

• +14 new UsbStreamInterfaceInitializerTests. Full suite green on net9 + net10: 2653 → 2667 passed / 2 skipped on each (+23 Daqifi.Mcp.Tests on net9; Mcp is net9-only). 0 warnings. The baseline was measured on a clean origin/main worktree, so the +14 is exact and nothing was lost.
• DaqifiDeviceInitializeTests left untouched on purpose — they are the evidence that the extraction changed nothing end to end.
• Mutation-checked rather than assumed. Setting MaxRetries to 0 and dropping preserveActiveStream from the gate made 7 of the 14 new tests fail, including the retry and Second connection to a streaming device silently stops the first session's stream (InitializeAsync sends StopStreaming unconditionally) #385 skip rules; restoring from a byte-identical backup was verified before committing. One test that had derived its expectation from MaxRetries + 1 (and so survived the mutation) now asserts a literal 2.

Bench (real Nq1, fw 3.7.2, USB, non-destructive)

Not just "it still streams" — the device was first put into the exact state this code exists to recover from. SYSTem:STReam:INTerface 1 was sent from the shell (accepted, 0,"No error"), routing the device's stream to WiFi. The example CLI was then built against this branch's Core and run over USB:

• 791 samples in 5 s across Analog0/Analog1, --min-samples 300 passed, exit 0.

With the routing broken, that run would have received nothing at all — so the extracted initializer is what put the data back on the serial port. 158 Hz effective vs 200 Hz requested is the known fw 3.7.2 clock mismatch, not a regression. The bench was explicitly returned to SYSTem:STReam:INTerface 0 afterwards and confirmed error-free.

---

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Refactor: extract USB stream routing policy into UsbStreamInterfaceInitializer

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Extract USB stream routing policy into internal UsbStreamInterfaceInitializer collaborator.
• Keep routing send mechanics and timeout in DaqifiStreamingDevice; public surface unchanged.
• Add unit tests covering skip gates, retries, cancellation semantics, and error classification.

Diagram

graph TD
  A["DaqifiStreamingDevice"] --> B["USB route policy"] --> C["ExecuteTextCommandAsync"] --> D["USB/Serial transport"]
  B --> E["SCPI response classifier"]
  T["Initializer tests"] --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a shared retry utility (or Polly) for the transient SCPI rejection

• ➕ Centralizes retry/backoff semantics across device initialization steps
• ➕ Makes retry budgets and delays easier to standardize/configure
• ➖ Adds extra abstraction (and possibly a dependency) for a very small policy
• ➖ Can obscure the important domain-specific behavior (skip path not observing cancellation)

2. Make routing policy an injected strategy/service instead of a static internal helper

• ➕ Easier to substitute policies in integration tests or future device variants
• ➕ Opens the door to configuration-driven policies (retry budget, delay)
• ➖ Increases wiring complexity and may pressure adding host/surface area
• ➖ Overkill if the policy is stable and only used in one place

Recommendation: Keep the current approach: an internal, host-free collaborator with a delegate for the send effect. It maximizes testability and separation of concerns without introducing new public surface area or DI/host plumbing, and it preserves the critical text-mode send behavior inside the device.

Files changed (3) +354 / -71

Refactor (2) +146 / -71

DaqifiStreamingDevice.csDelegate USB stream routing decision/retry logic to initializer +19/-71

Delegate USB stream routing decision/retry logic to initializer

• Removes inline USB stream-interface routing policy from OnDeviceInitializingAsync and delegates to UsbStreamInterfaceInitializer with an ExecuteTextCommandAsync-based send delegate. Extracts the response timeout literal into a named constant while keeping send mechanics in the device to preserve text-mode behavior.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

UsbStreamInterfaceInitializer.csIntroduce host-free USB stream-interface initializer policy +127/-0

Introduce host-free USB stream-interface initializer policy

• Adds an internal helper that decides whether to route the stream to USB and performs the retry/settle-delay policy on transient SCPI rejections. Throws ScpiInitializationErrorException with the last attempt’s response when errors persist across retries.

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

Tests (1) +208 / -0

UsbStreamInterfaceInitializerTests.csAdd unit tests for USB stream-interface routing policy +208/-0

Add unit tests for USB stream-interface routing policy

• Introduces a scripted sender harness to model per-attempt device responses. Covers the route-or-skip gate (USB + owning session), retry-on-transient-error behavior, last-error reporting, and cancellation semantics (including deliberately not observing cancellation on the skip path).

src/Daqifi.Core.Tests/Device/Internal/UsbStreamInterfaceInitializerTests.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. Not merging — leaving the merge to you.

State at head 8a99fd2:

• Qodo: clean at this exact commit. Bugs (0) / Rule violations (0) / Requirement gaps (0) — and that headline was cross-checked rather than taken at face value: the GraphQL reviewThreads query returns 0 threads total (not just 0 unresolved), and pulls/442/comments is empty, so there are no findings parked in collapsed inline threads. Qodo's only substantive commentary was its "alternative approaches" section, and it recommended keeping this design as-is.
• CI build green (dotnet test Daqifi.Core.sln, which covers net9 + net10).
• MERGEABLE, and not stale — origin/main (183dee9, the refactor(device): extract raw-command session interpretation into a collaborator (part of #344) #439 merge) is already an ancestor of this branch, so there is nothing to rebase and no conflict with the two sibling slices. mergeStateStatus: BLOCKED is just the code-owner approval gate, which is yours to give.

This is the third of three open #344 slices (#440, #441, #442), all now clean and awaiting your review. They were written to be conflict-disjoint from each other and can be merged in any order.