feat(device): eliminate the DaqifiStreamingDevice downcast

[tylerkron]

Summary

DaqifiDeviceFactory's eight public Connect*/DiscoverAndConnectAsync methods were typed Task<DaqifiDevice> / DaqifiDevice, but the constructed instance was always a DaqifiStreamingDevice — every caller had to cast or pattern-match (if (device is IStreamingDevice streamingDevice)) to reach streaming, SD-card, network, or diagnostics operations. The README explained the downcast twice, docs/DEVICE_INTERFACES.md pattern-matched it in four sections, and the in-repo MCP consumer wrapped it in RequireStreaming/RequireSdCard helpers.

Worse, IStreamingDevice's own docs said a channel argument "must belong to this device's Channels collection," but neither IDevice nor IStreamingDevice exposed Channels, GetChannelsSnapshot(), or Metadata — a consumer holding only the interface couldn't obtain a channel to pass into the interface's own methods.

Changes

Following the issue's proposed "simplest path":

1. DaqifiDeviceFactory — narrowed every Connect*/DiscoverAndConnectAsync return type from DaqifiDevice to DaqifiStreamingDevice. Source-compatible: existing DaqifiDevice-typed call sites still compile via implicit upcast. Binary break — worth calling out in release notes.
2. IStreamingDevice — promoted Channels, GetChannelsSnapshot(), Metadata, and the ChannelsPopulated event onto the interface. DaqifiStreamingDevice already satisfies these via its DaqifiDevice base class, so no implementation changes were needed — this is purely a contract widening.
3. DaqifiDeviceRegistry — its internal DeviceConnector delegate stays typed over the base DaqifiDevice (its public Register(DaqifiDevice, ...) deliberately accepts any manually-constructed DaqifiDevice, not only ones the factory built — see the "Manual Device Connection (Advanced)" doc section), so the connector lambda needed a small async adjustment to bridge Task<DaqifiStreamingDevice> → Task<DaqifiDevice>.
4. MCP (DaqifiAgent.RequireStreaming) — for the same reason as SCPI Commands #3, this keeps its runtime IStreamingDevice check; I documented why in a comment rather than removing it, since the registry's contract doesn't statically guarantee every registered device is a DaqifiStreamingDevice even though in practice it always is. Fully eliminating it would mean retyping DaqifiDeviceRegistry itself, which is a real design change beyond this issue's proposed scope (and would break the registry's legitimate "register any manually-connected DaqifiDevice" use case) — flagging this as a candidate follow-up rather than doing it here.
5. Docs — README.md and docs/DEVICE_INTERFACES.md: removed the cast/pattern-match advice from every example that connects through the factory (digital output, PWM, network configuration, channel management, device diagnostics). The "Manual Device Connection (Advanced)" section, which constructs a plain DaqifiDevice directly rather than through the factory, is unchanged since it doesn't get the narrowed type.

Testing

• Reflection-based regression tests pinning the factory's return types and the new IStreamingDevice members (DaqifiDeviceFactoryTests.cs)
• Updated 6 test-only fake IStreamingDevice implementers across 3 files to satisfy the widened interface
• Full Daqifi.Core.Tests suite: 2867 passed
• Full Daqifi.Mcp.Tests suite: 36 passed

Fixes #333

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Return DaqifiStreamingDevice from factory and expose channels/metadata on IStreamingDevice

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Narrow DaqifiDeviceFactory Connect*/DiscoverAndConnectAsync to return DaqifiStreamingDevice (no
 downcasts).
• Widen IStreamingDevice to expose Channels/Metadata/GetChannelsSnapshot/ChannelsPopulated for
 channel-safe APIs.
• Update registry, MCP agent, tests, and docs to reflect the new contracts.

Diagram

graph TD
  C["Library consumer"] --> F["DaqifiDeviceFactory"] --> SD["DaqifiStreamingDevice"] --> IS["IStreamingDevice"]
  R["DaqifiDeviceRegistry"] --> F
  A["DaqifiAgent (MCP)"] --> R --> SD
  A --> IS

  subgraph Legend
    direction LR
    _app["Consumer"] ~~~ _svc["Core component"] ~~~ _int["Interface"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Return IStreamingDevice from factory

• ➕ Eliminates coupling to the concrete type in consumer code
• ➕ Communicates the intended usable surface directly
• ➕ Still avoids downcasts for streaming features
• ➖ Harder to access concrete-only features without additional interfaces
• ➖ Potentially obscures disposal/extended members if not on the interface
• ➖ May conflict with any existing non-streaming device variants

2. Keep returning DaqifiDevice and add helper APIs (extensions/RequireStreaming)

• ➕ Avoids binary-breaking signature changes
• ➕ Preserves a single base type for all device variants
• ➖ Does not fully remove downcasts/pattern-matching at call sites
• ➖ Docs and user code remain more complex
• ➖ Does not solve the ‘interface can’t get Channels/Metadata’ problem without widening interfaces anyway

3. Introduce parallel Connect*StreamingAsync overloads

• ➕ Non-breaking upgrade path for existing binaries
• ➕ Lets consumers opt-in explicitly to streaming device semantics
• ➖ API surface grows and duplicates methods
• ➖ Leaves ambiguity about which overload is recommended long-term
• ➖ Still requires documentation and migration guidance

Recommendation: The PR’s approach (return DaqifiStreamingDevice directly + widen IStreamingDevice) is the best fit for the stated goal: it removes the pervasive downcast and makes IStreamingDevice self-sufficient for its own channel-based methods. The main tradeoff is the binary-breaking signature change; if binary compatibility becomes a priority later, consider adding temporary overloads returning the old types as a migration bridge.

Files changed (10) +247 / -129

Enhancement (2) +58 / -26

DaqifiDeviceFactory.csNarrow Connect*/Discover return types to DaqifiStreamingDevice +26/-26

Narrow Connect*/Discover return types to DaqifiStreamingDevice

• Changes all public Connect* and DiscoverAndConnectAsync signatures to return DaqifiStreamingDevice (and Task<> variants) and updates internal helper methods accordingly.

src/Daqifi.Core/Device/DaqifiDeviceFactory.cs

IStreamingDevice.csExpose Channels/Metadata/snapshot/event on IStreamingDevice +32/-0

Expose Channels/Metadata/snapshot/event on IStreamingDevice

• Promotes Metadata, Channels, GetChannelsSnapshot(), and ChannelsPopulated onto IStreamingDevice so interface consumers can obtain valid channels and observe channel population events.

src/Daqifi.Core/Device/IStreamingDevice.cs

Refactor (1) +7 / -2

DaqifiDeviceRegistry.csBridge narrower factory Task return type in default connector +7/-2

Bridge narrower factory Task return type in default connector

• Replaces the method-group assignment with an async lambda so Task<DaqifiStreamingDevice> can be awaited and upcast to DaqifiDevice for the registry’s connector delegate.

src/Daqifi.Core/Device/DaqifiDeviceRegistry.cs

Tests (4) +93 / -0

DaqifiDeviceFactoryTests.csAdd reflection tests for factory return types and interface members +54/-0

Add reflection tests for factory return types and interface members

• Pins all public Connect*/Discover method return types to DaqifiStreamingDevice / Task<DaqifiStreamingDevice> using reflection. Adds a contract test ensuring IStreamingDevice exposes Channels/Metadata/GetChannelsSnapshot/ChannelsPopulated.

src/Daqifi.Core.Tests/Device/DaqifiDeviceFactoryTests.cs

DaqifiStreamingDeviceAsyncSurfaceTests.csUpdate minimal IStreamingDevice test stub to satisfy new members +4/-0

Update minimal IStreamingDevice test stub to satisfy new members

• Extends MinimalStreamingDevice to implement Metadata, Channels, GetChannelsSnapshot, and ChannelsPopulated as required by the widened interface.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceAsyncSurfaceTests.cs

FirmwareUpdateServiceTests.csUpdate firmware test fakes for widened IStreamingDevice contract +26/-0

Update firmware test fakes for widened IStreamingDevice contract

• Adds Metadata/Channels/GetChannelsSnapshot/ChannelsPopulated to multiple fake streaming device implementations used in firmware update tests.

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs

LanChipInfoProviderExtensionsTests.csUpdate LAN chip-info scripted device stub for new IStreamingDevice members +9/-0

Update LAN chip-info scripted device stub for new IStreamingDevice members

• Adds Metadata/Channels/GetChannelsSnapshot/ChannelsPopulated to the scripted device used by extension tests.

src/Daqifi.Core.Tests/Firmware/LanChipInfoProviderExtensionsTests.cs

Documentation (3) +89 / -101

README.mdRemove factory downcast from usage examples +26/-39

Remove factory downcast from usage examples

• Updates connection and feature examples to use the factory result directly (no cast/pattern-match). Adjusts network configuration wording to reference DaqifiStreamingDevice implementing INetworkConfigurable.

README.md

DEVICE_INTERFACES.mdUpdate interface docs and examples to match new return types +55/-62

Update interface docs and examples to match new return types

• Rewrites documentation to reflect that Connect* returns DaqifiStreamingDevice directly. Removes pattern-matching in examples and explains that Channels/Metadata are now on IStreamingDevice.

docs/DEVICE_INTERFACES.md

DaqifiAgent.csDocument why RequireStreaming still runtime-checks the registry device type +8/-0

Document why RequireStreaming still runtime-checks the registry device type

• Adds an explanatory comment clarifying that the cast remains because the registry stores base DaqifiDevice instances, even though factory-created devices are streaming devices in practice.

src/Daqifi.Mcp/DaqifiAgent.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Racy channel enumeration in docs ✓ Resolved 🐞 Bug ☼ Reliability

Description

The Channel Management example now enumerates the live device.Channels collection (e.g.,
First(...) / OfType(...)), but DaqifiDevice.Channels is a live view that can be repopulated
concurrently; this can throw InvalidOperationException or produce inconsistent results. The docs
should use a single GetChannelsSnapshot() and run all LINQ queries against that snapshot.

Code

docs/DEVICE_INTERFACES.md[R863-865]

+// Channels are populated after a status message is received from the device.
+var ai0 = device.Channels.First(c => c.Type == ChannelType.Analog && c.ChannelNumber == 0);
+var ai2 = device.Channels.First(c => c.Type == ChannelType.Analog && c.ChannelNumber == 2);

Relevance

●●● Strong

Direct precedent: PR #291 accepted changing docs to snapshot channels (GetChannelsSnapshot) to avoid
racy live enumeration.

PR-#291

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docs example uses device.Channels.First(...) and device.Channels.OfType(...).First(...),
while the implementation documents that Channels is a live view and recommends
GetChannelsSnapshot() to avoid concurrent-mutation enumeration failures.

docs/DEVICE_INTERFACES.md[849-889]
src/Daqifi.Core/Device/DaqifiDevice.cs[243-270]
PR-#291

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The docs sample enumerates `device.Channels` directly, but `Channels` is a live view that can be repopulated concurrently, causing timing-dependent `InvalidOperationException` during enumeration.

## Issue Context
`DaqifiDevice.Channels` is explicitly documented as a live view, and the codebase provides `GetChannelsSnapshot()` for safe enumeration under concurrent repopulation.

## Fix Focus Areas
- docs/DEVICE_INTERFACES.md[849-889]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 189cd26