feat: continuous/stateful discovery (ContinuousDeviceFinder + DeviceLost)

[tylerkron]

Summary

Closes #245.

Adds ContinuousDeviceFinder — a stateful primitive that wraps any IDeviceFinder and turns its per-call discovery into a continuous scan. It owns the scan cadence, maintains a deduplicated live set across passes, raises DeviceDiscovered the first time each device appears, and raises DeviceLost once a device has been absent for a configurable number of consecutive passes. A UI can bind directly to Devices + these events instead of writing its own polling loops and stale-removal.

This unblocks the daqifi-desktop cleanup (daqifi/daqifi-desktop#616), which today wraps the finders in three hand-written polling loops with magic delays plus per-transport dedup/stale-removal.

Design

• Wraps a single finder. One instance = one transport's cadence + live set. To track WiFi/Serial/HID together, create one per transport (each with its own interval) and merge their events. Simpler and far more testable than a multi-finder aggregator, and maps exactly to desktop's existing per-transport dedup.
• Uses each pass's returned device list as the authoritative "seen this pass" set (not the inner event), so dedup is clean with no double-counting.
• Per-transport identity (overridable via ContinuousDiscoveryOptions.IdentitySelector):
  • WiFi → MAC, then serial number, then IP
  • Serial → serial number (survives COM-port reassignment), then port name
  • HID → device path (bootloaders may report empty/duplicate serials), then serial number
  • Each key is prefixed by ConnectionType, so the same physical unit seen over two transports appears as two distinct connection options.
• MissThreshold (default 2) tolerates a dropped UDP reply before reporting a device lost.
• Resilient loop: a throwing discovery pass, identity selector, or event subscriber is surfaced via ScanError and never kills the background loop — matching the existing finders' "swallow subscriber exceptions; keep receiving" contract.

API

var continuous = new ContinuousDeviceFinder(new WiFiDeviceFinder(), new ContinuousDiscoveryOptions
{
    PassTimeout = TimeSpan.FromSeconds(3), // listen window per pass
    Interval = TimeSpan.FromSeconds(1),    // gap between passes
    MissThreshold = 2,
});
continuous.DeviceDiscovered += (_, e) => /* add to UI list */;
continuous.DeviceLost       += (_, e) => /* remove from UI list */;
continuous.ScanError        += (_, e) => /* log e.Exception */;
continuous.Start();
// ...
await continuous.StopAsync();
continuous.Dispose(); // also disposes the wrapped finder unless LeaveInnerFinderOpen is set

Files

• ContinuousDeviceFinder.cs — scan loop, identity-keyed live set, DeviceDiscovered/DeviceLost/ScanError, Devices snapshot, Start()/StopAsync(), IDisposable.
• ContinuousDiscoveryOptions.cs — Interval, PassTimeout, MissThreshold, IdentitySelector, LeaveInnerFinderOpen.
• DeviceLostEventArgs.cs, ContinuousDiscoveryErrorEventArgs.cs.
• ContinuousDeviceFinderTests.cs — 30 tests (deterministic reconcile logic + event-driven loop integration).
• docs/DEVICE_INTERFACES.md — "Continuous Discovery" usage section.

Testing

• dotnet test — 1043 passed, 2 skipped (pre-existing), 0 failed on both net9.0 and net10.0.
• TreatWarningsAsErrors is on, so the clean build also confirms lint/XML-doc coverage.

Notes for the desktop adoption (no change here)

The MissThreshold default of 2 means a device is reported lost after ~2 passes absent (≈ 2 × (PassTimeout + Interval)). That's conservative for lossy WiFi UDP; the reliable Serial/HID instances may want MissThreshold = 1 to drop ghosts faster.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Add ContinuousDeviceFinder for stateful continuous device discovery
✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Description

• Introduce ContinuousDeviceFinder to turn one-shot discovery into a continuous live device set.
• Emit DeviceDiscovered once per device and DeviceLost after configurable missed passes.
• Add deterministic unit/integration tests and document UI-oriented continuous discovery usage.

Diagram

graph TD
  UI["UI / ViewModel"] --> CDF(["ContinuousDeviceFinder"]) --> LIVE[("Live device set")]
  CDF --> EVT["Events: Discovered/Lost/Error"]
  CDF --> IF{{"IDeviceFinder"}} --> INNER["Transport finder"]

  subgraph Legend
    direction LR
    _ui["Consumer"] ~~~ _svc(["Component"]) ~~~ _db[("State")] ~~~ _if{{"Interface"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Multi-finder aggregator (one ContinuousDeviceFinder for all transports)

• ➕ Single combined live set and unified events for the UI
• ➕ Could deduplicate across transports if desired
• ➖ More complex identity semantics (cross-transport collisions/merges)
• ➖ Harder to test deterministically; more branching per transport cadence
• ➖ Less aligned with current per-transport desktop polling loops

2. Expose discovery as IAsyncEnumerable stream

• ➕ Naturally models continuous discovery and cancellation
• ➕ Encourages composition (merge, throttle, buffer) with LINQ-like operators
• ➖ Bigger API surface change vs. current event-driven patterns
• ➖ UI consumers often still need a live set + stale removal logic

3. Use Reactive Extensions (IObservable) for discovery events

• ➕ Rich composition primitives (merge, retry, debounce) out of the box
• ➕ Clear separation between producer and subscribers
• ➖ Introduces a new dependency and learning curve
• ➖ Overkill if consumers primarily need a simple live set + two events

Recommendation: The PR’s approach (one ContinuousDeviceFinder per transport, event-based, with a live-set snapshot) is the best fit for current consumers: it keeps identity/dedup rules transport-scoped, preserves existing event conventions (swallow subscriber exceptions), and remains highly testable. A multi-finder aggregator is the main alternative but adds complexity and ambiguous cross-transport identity behavior.

Files changed (6) +1300 / -0

Enhancement (4) +709 / -0

ContinuousDeviceFinder.csImplement ContinuousDeviceFinder with scan cadence, live set reconciliation, and events +605/-0

Implement ContinuousDeviceFinder with scan cadence, live set reconciliation, and events

• Adds a stateful wrapper that repeatedly calls an inner IDeviceFinder with a pass timeout and interval, maintaining an identity-keyed live set with miss counters. Raises DeviceDiscovered/DeviceLost/ScanError safely (subscriber exceptions surfaced via ScanError), provides Devices snapshots, and supports Start/StopAsync and disposal semantics (optionally leaving the inner finder open).

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs

ContinuousDiscoveryErrorEventArgs.csAdd ScanError event args for surfacing continuous discovery failures +25/-0

Add ScanError event args for surfacing continuous discovery failures

• Defines ContinuousDiscoveryErrorEventArgs carrying the underlying exception from a failed discovery pass (or other loop error), with null validation.

src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryErrorEventArgs.cs

ContinuousDiscoveryOptions.csAdd options for scan interval, pass timeout, miss threshold, and identity selection +54/-0

Add options for scan interval, pass timeout, miss threshold, and identity selection

• Introduces ContinuousDiscoveryOptions to configure cadence (Interval, PassTimeout), stale detection (MissThreshold), identity computation (IdentitySelector), and ownership (LeaveInnerFinderOpen). Defaults are tuned for WiFi-style discovery windows and lossy transports.

src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryOptions.cs

DeviceLostEventArgs.csAdd DeviceLost event args for reporting removed devices from the live set +25/-0

Add DeviceLost event args for reporting removed devices from the live set

• Defines DeviceLostEventArgs carrying the last-seen IDeviceInfo for a device that exceeded the miss threshold, with null validation.

src/Daqifi.Core/Device/Discovery/DeviceLostEventArgs.cs

Tests (1) +554 / -0

ContinuousDeviceFinderTests.csAdd unit + integration tests for reconciliation, lifecycle, and loop resilience +554/-0

Add unit + integration tests for reconciliation, lifecycle, and loop resilience

• Introduces comprehensive tests covering deduplication, metadata refresh, miss-threshold-based loss, identity rules, and error containment. Includes loop-level tests verifying background scanning raises events and survives pass/selector exceptions, plus small fakes for deterministic pass scripting.

src/Daqifi.Core.Tests/Device/Discovery/ContinuousDeviceFinderTests.cs

Documentation (1) +37 / -0

DEVICE_INTERFACES.mdDocument continuous discovery usage and multi-transport composition guidance +37/-0

Document continuous discovery usage and multi-transport composition guidance

• Adds a new "Continuous Discovery" section describing how to wrap an IDeviceFinder in ContinuousDeviceFinder for a UI-friendly live device list. Provides sample code, explains per-transport instances, and notes Devices returns a thread-safe snapshot.

docs/DEVICE_INTERFACES.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 feat: Continuous/stateful discovery (live deduplicated device set + DeviceLost)

1. Dispose may outlive loop ✓ Resolved 🐞 Bug ☼ Reliability

Description

ContinuousDeviceFinder.Dispose() only waits 5 seconds for the scan loop to exit, so with PassTimeout
> 5s (or a slow/hung inner finder) the loop can keep running and raising events after Dispose
returns and/or after the inner finder is disposed. This can cause post-dispose callbacks, spurious
ScanError noise, and background-task/resource leaks.

Code

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[R575-596]

+        cts?.Cancel();
+
+        // Best-effort wait so the loop stops raising events before we return. The loop
+        // uses ConfigureAwait(false) throughout, so this cannot deadlock on a captured
+        // synchronization context. Bounded so Dispose never hangs on a stuck pass.
+        try
+        {
+            loopTask?.Wait(TimeSpan.FromSeconds(5));
+        }
+        catch
+        {
+            // Ignore faults observed during shutdown.
+        }
+
+        cts?.Dispose();
+
+        if (!_leaveInnerFinderOpen && _finder is IDisposable disposableFinder)
+        {
+            try
+            {
+                disposableFinder.Dispose();
+            }

Evidence

Dispose uses a fixed 5-second wait and then may dispose the inner finder even if the loop is still
running, while PassTimeout is configurable to any positive duration and the scan loop does not pass
its cancellation token into discovery.

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[401-405]
src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[575-602]
src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryOptions.cs[19-27]

Agent prompt

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

### Issue description
`Dispose()` uses a hard-coded 5-second wait (`loopTask?.Wait(TimeSpan.FromSeconds(5))`). If a discovery pass takes longer than 5 seconds (because `PassTimeout` is configured larger, or because a finder misbehaves), the scan loop can outlive `Dispose()` and continue raising events. `Dispose()` may then also dispose the wrapped finder while the loop is still in-flight.

### Issue Context
- `PassTimeout` is user-configurable and only validated to be `> 0`.
- The scan loop calls `DiscoverAsync(_passTimeout)` without linking the loop cancellation token, so `StopAsync()/Dispose()` cancellation cannot interrupt an in-flight pass.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[394-442]
- src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[555-602]
- src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryOptions.cs[10-54]

### Suggested fix approach
1. In `ScanLoopAsync`, run each pass using the *cancellation-token* overload and implement the timeout yourself with a linked CTS:
  - `using var passCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);`
  - `passCts.CancelAfter(_passTimeout);`
  - `await _finder.DiscoverAsync(passCts.Token)`
  This ensures `StopAsync()/Dispose()` cancellation can promptly stop a pass.
2. Update exception handling so cancellation due to `passCts` timeout is treated as a normal pass end (not `ScanError`), while cancellation due to the outer `cancellationToken` exits the loop.
3. In `Dispose()`, avoid a fixed 5s wait; instead, stop the loop deterministically (e.g., call `StopAsync().GetAwaiter().GetResult()`), or bound the wait using `_passTimeout` (and ensure the pass is cancellable as per #1).

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

---

2. Null identity collapses devices ✓ Resolved 🐞 Bug ≡ Correctness

Description

If ContinuousDiscoveryOptions.IdentitySelector returns null, GetIdentity converts it to
string.Empty, causing all such devices to share the same dictionary key and breaking deduplication
and DeviceLost semantics silently. This can collapse multiple devices into one tracked entry without
any ScanError signal.

Code

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[R332-337]

+    private string GetIdentity(IDeviceInfo device)
+    {
+        if (_identitySelector != null)
+        {
+            return _identitySelector(device) ?? string.Empty;
+        }

Evidence

The options allow providing an IdentitySelector, and GetIdentity maps a null result to an empty
string key, which will collide in the dictionary and merge device entries unexpectedly.

src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryOptions.cs[37-45]
src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[332-337]
src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[58-60]

Agent prompt

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

### Issue description
`GetIdentity()` treats a null return from a custom `IdentitySelector` as `string.Empty`. This makes all devices with a null identity collide into the same key, corrupting the live set.

### Issue Context
`IdentitySelector` is intended to provide stable per-device keys; a null/empty key is almost certainly a caller bug and should not silently change tracking semantics.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[328-340]
- src/Daqifi.Core/Device/Discovery/ContinuousDiscoveryOptions.cs[37-45]

### Suggested fix approach
- Replace `return _identitySelector(device) ?? string.Empty;` with explicit validation:
 - If the selector returns `null`/empty/whitespace, either:
   1) fall back to `DefaultIdentity(device)` (prevents collisions), or
   2) treat it as an invalid key and skip tracking that device for the pass.
- If you choose (2), ensure the error is surfaced (e.g., via `ScanError`) *without* raising `ScanError` while holding `_devicesLock` (to avoid deadlocks); collect errors and raise them after releasing the lock.

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

---

3. Start/Dispose race ✓ Resolved 🐞 Bug ☼ Reliability

Description

Start() checks _disposed outside the lifecycle lock, so a concurrent Dispose() can set
_disposed=true after the check but before Start() creates the CTS and launches the loop. This can
start background scanning on an instance that has already been disposed.

Code

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[R182-197]

+    public void Start()
+    {
+        ThrowIfDisposed();
+
+        lock (_lifecycleLock)
+        {
+            if (_running)
+            {
+                throw new InvalidOperationException("Continuous discovery is already running.");
+            }
+
+            _cts = new CancellationTokenSource();
+            var token = _cts.Token;
+            _running = true;
+            _loopTask = Task.Run(() => ScanLoopAsync(token));
+        }

Evidence

Start reads _disposed before taking the lifecycle lock, but Dispose writes _disposed before taking
the same lock; this allows Start to miss the dispose and still start the loop.

src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[182-197]
src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[555-573]

Agent prompt

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

### Issue description
`Start()` calls `ThrowIfDisposed()` before acquiring `_lifecycleLock`, while `Dispose()` sets `_disposed = true` before acquiring `_lifecycleLock`. This permits a race where `Start()` observes `_disposed == false` and proceeds to start the loop even though `Dispose()` is concurrently disposing the object.

### Issue Context
The class otherwise appears to be designed for thread-safe lifecycle control (`_lifecycleLock`, `_running`, `_cts`, `_loopTask`), so this race is inconsistent with the intended design.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[177-239]
- src/Daqifi.Core/Device/Discovery/ContinuousDeviceFinder.cs[539-573]

### Suggested fix approach
- Move the disposed check inside the `_lifecycleLock` critical section in `Start()` (and ideally in `StopAsync()` too), e.g.:
 - `lock (_lifecycleLock) { if (_disposed) throw ...; if (_running) throw ...; ... }`
- Optionally, make `_disposed` updates/checks atomic (e.g., `Interlocked.Exchange` on an `int`) if you want to guarantee idempotence under concurrent calls to `Dispose()`.
- Ensure `Dispose()` and `Start()` cannot interleave into an invalid state (disposed-but-running).

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

---

[tylerkron]

Qodo review — all 3 bugs addressed in 31b61b5

1. Dispose may outlive loop ✅ Fixed (details in the inline reply). Passes are now cancellable via a linked CTS + CancelAfter(PassTimeout); Stop/Dispose interrupt an in-flight pass promptly, and Dispose waits for the loop to stop before disposing the inner finder instead of using a fixed 5s wait.

2. Start/Dispose race ✅ Fixed. Start() now checks _disposed inside _lifecycleLock, and Dispose() sets _disposed under that same lock (and is idempotent there). This closes the window where a concurrent Dispose() could set _disposed = true after Start()'s check but before it launched the loop, so the loop can no longer start on a disposed instance.

3. Null identity collapses devices ✅ Fixed. GetIdentity previously mapped a null/empty IdentitySelector result to string.Empty, collapsing distinct devices onto one key. It now falls back to the built-in per-transport DefaultIdentity (your option 1) when the selector returns null/empty/whitespace, preserving dedup and DeviceLost semantics. Added a test asserting two distinct devices stay distinct when the selector returns null.

Full suite: 1046 passed, 2 skipped, 0 failed on both net9.0 and net10.0 (warnings-as-errors clean).

[tylerkron]

Bench test against real hardware ✅

Built the DAQiFi Core example CLI against this branch's local Daqifi.Core (via -p:DaqifiCoreProjectPath=…/src/Daqifi.Core/Daqifi.Core.csproj) and ran a smoke test over USB/serial against a real DAQiFi device.

Confirmed the branch build was loaded: the example app's bin/Release/net9.0/Daqifi.Core.dll (rebuilt at run time) contains the new ContinuousDeviceFinder type — i.e. the run exercised this worktree's core, not the published NuGet package.

Command

Daqifi.Core.Cli --serial /dev/cu.usbmodem2101 --show-status --rate 10 --duration 3 --channels 3 --min-samples 1

Result

Check	Outcome
Connection	✅ Connected @ 9600 baud
Initialization	✅ Populated device — 2-channel data flowed (channels 0+1)
Streaming	✅ ~29 samples over 3 s at 10 Hz (matches requested rate)
Sample shape	analog=[ch0, ch1], e.g. [5, 19] — both enabled channels present
Shutdown	✅ Streaming stopped → status Disconnected, clean exit
Exit code	0 (--min-samples 1 satisfied)

This validates that the branch loads and drives real hardware end-to-end (connect → initialize → stream → stop → disconnect). The new ContinuousDeviceFinder itself is covered by the unit tests in this PR; the example CLI does not yet have a continuous-discovery mode (assessing separately whether to add one).

Note: the bench build required a temporary throwing shim for DaqifiStreamingDevice.GetSdCardStorageAsync — an unrelated API the example app's local WIP references that doesn't exist on this branch. The shim was deleted after the run; no example-app source was modified.

[tylerkron]

Follow-up tracked for exercising this API in the example CLI: daqifi/daqifi-core-example-app#29 (--watch / --watch-serial continuous-discovery demo). It's blocked on a published Daqifi.Core version containing ContinuousDeviceFinder, so it's a separate change in the example repo and does not gate this PR.