fix: replace Thread.Sleep with Task.Delay in async SD paths

[tylerkron]

Closes #221.

Summary

• DaqifiStreamingDevice had 4 Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS) calls inside *Async setup-action lambdas (GetSdCardFilesAsync, DeleteSdCardFileAsync). Each blocked a thread-pool thread for the 100ms settle window and ignored the operation's CancellationToken.
• Added a Func<CancellationToken, Task> overload of ExecuteTextCommandAsync so setup lambdas can await Task.Delay(..., ct). Refactored the existing Action overload + new overload into a shared private async core to avoid duplicating the ~200-line consumer-swap/timeout body.
• Converted the 4 affected call sites to async ct => { …; await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); … }.

The Thread.Sleep in WifiBridgeActivator.Activate() and the background-thread Thread.Sleep calls in MessageProducer / StreamMessageConsumer are intentionally untouched (per the issue).

Test plan

• dotnet test --framework net9.0 — 995 passed, 0 failed, 2 skipped (real-hardware integration tests)
• dotnet test --framework net10.0 — same result
• dotnet build — 0 warnings, 0 errors (repo enforces TreatWarningsAsErrors)
• New regression tests:
  • GetSdCardFilesAsync_HonorsCancellationDuringSettleDelay — cancels the token 20ms in, asserts OperationCanceledException within 80ms (well under the 100ms settle window). Would have failed under Thread.Sleep since Thread.Sleep cannot be cancelled mid-sleep.
  • DeleteSdCardFileAsync_HonorsCancellationDuringSettleDelay — symmetric.
• HIL smoke test (PR feat: hardware-in-the-loop smoke test + /hil-smoke-test skill #219) — defer to CI / hardware run

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Replace Thread.Sleep with Task.Delay in async SD operations

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Replace Thread.Sleep with Task.Delay in SD card async operations
  - Eliminates thread-pool thread blocking during 100ms settle delays
  - Honors CancellationToken during settle window instead of ignoring it
• Add async overload of ExecuteTextCommandAsync accepting Func
  - Refactored both overloads into shared private ExecuteTextCommandCoreAsync core
  - Eliminates ~200 lines of duplicated consumer-swap/timeout logic
• Add regression tests for cancellation during SD settle delays
  - GetSdCardFilesAsync_HonorsCancellationDuringSettleDelay validates early cancellation
  - DeleteSdCardFileAsync_HonorsCancellationDuringSettleDelay validates symmetric behavior

Diagram

flowchart LR
  A["ExecuteTextCommandAsync<br/>Action overload"] --> C["ExecuteTextCommandCoreAsync<br/>shared private core"]
  B["ExecuteTextCommandAsync<br/>Func async overload"] --> C
  C --> D["Awaitable Task.Delay<br/>with CancellationToken"]
  E["GetSdCardFilesAsync<br/>setup lambda"] --> D
  F["DeleteSdCardFileAsync<br/>setup lambda"] --> D
  G["Regression tests"] --> H["Verify cancellation<br/>propagates promptly"]

Loading

File Changes

1. src/Daqifi.Core/Device/DaqifiDevice.cs ✨ Enhancement +45/-5

Add async overload and refactor shared core logic

• Refactored ExecuteTextCommandAsync(Action) to delegate to new ExecuteTextCommandCoreAsync
• Added new ExecuteTextCommandAsync(Func) overload for async setup actions
• Moved ~200 lines of consumer-swap/timeout logic into private ExecuteTextCommandCoreAsync
• Updated setup action invocation to await setupActionAsync(cancellationToken) instead of calling
 setupAction()
• Updated XML documentation references to disambiguate between overloads

src/Daqifi.Core/Device/DaqifiDevice.cs

---

2. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs 🐞 Bug fix +8/-8

Replace Thread.Sleep with Task.Delay in SD operations

• Converted 4 Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS) calls to await Task.Delay(..., ct)
• Updated GetSdCardFilesAsync initial and retry setup lambdas to async with cancellation token
• Updated DeleteSdCardFileAsync initial and retry setup lambdas to async with cancellation token
• All setup lambdas now properly propagate CancellationToken to settle delay

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

---

3. src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs 🧪 Tests +77/-0

Add cancellation regression tests and test overloads

• Added GetSdCardFilesAsync_HonorsCancellationDuringSettleDelay regression test
• Added DeleteSdCardFileAsync_HonorsCancellationDuringSettleDelay regression test
• Both tests verify cancellation fires within 80ms of 100ms settle delay window
• Added async overload implementations to three testable device classes

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs

---

[qodo-code-review]

Code Review by Qodo

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

1. Flaky cancel timing asserts ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new regression tests assert cancellation completes in <80ms using real-time Stopwatch, which can
intermittently fail under CI scheduler jitter even when cancellation is correctly honored. The
cancellation behavior assertion is valuable, but the hard upper bound is not robust given SD settle
is 100ms and timers/scheduling are nondeterministic.

Code

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[R243-273]

Evidence

The tests hard-code a <80ms wall-clock bound while documenting the settle delay as 100ms; this is
inherently susceptible to CI timing jitter. The settle delay constant in production code is 100ms,
confirming the test’s assumption and the tightness of the bound.

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[232-273]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[32-38]

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 new cancellation regression tests rely on strict wall-clock timing ("< 80ms") to prove cancellation happened before the 100ms settle delay completes. This makes the tests prone to intermittent failures on loaded CI agents due to timer resolution and scheduling jitter, even when the implementation is correct.

### Issue Context
The intent is to ensure the SD settle wait is cancellable (now `Task.Delay(..., ct)` instead of `Thread.Sleep`). That can be validated without a tight elapsed-ms upper bound.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[232-273]

### Suggested fix
- Keep `Assert.ThrowsAnyAsync<OperationCanceledException>` (this is the key regression assertion).
- Replace the strict `ElapsedMilliseconds < 80` check with a more robust approach, e.g.:
 - Bound completion with `Task.WhenAny(opTask, Task.Delay(500))` and assert the opTask wins, or
 - Relax the threshold substantially (e.g., `< 250ms`) while still ensuring it completes well before a full settle+retry+timeouts path.
- Optionally, cancel deterministically after the operation has started (e.g., start the operation, `await Task.Delay(20)`, then `cts.Cancel()`), instead of using `CancellationTokenSource(TimeSpan.FromMilliseconds(20))` which starts counting immediately and can cancel before the operation reaches the intended point on slower machines.

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

---