Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tylerkron merged 2 commits into from
May 23, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
309e7f0
fix: replace Thread.Sleep with Task.Delay in async SD paths (closes #…
tylerkron May 23, 2026
5bb512f
test: cancel deterministically in SD settle-delay regression tests
tylerkron May 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,46 @@ public async Task GetSdCardFilesAsync_UpdatesSdCardFilesProperty()
Assert.Equal("test.bin", device.SdCardFiles[0].FileName);
}

[Fact]
public async Task GetSdCardFilesAsync_HonorsCancellationDuringSettleDelay()
{
// Regression for #221: the SD interface settle wait used Thread.Sleep,
// which ignored the CancellationToken. After the fix the wait is
// await Task.Delay(..., ct), so cancelling while the operation is
// suspended in the delay must propagate as OperationCanceledException.
var device = new TestableSdCardStreamingDevice("TestDevice");
device.CannedTextResponse = new List<string> { "Daqifi/test.bin" };
device.Connect();

using var cts = new CancellationTokenSource();

// The sync portion of GetSdCardFilesAsync runs through the setup
// lambda's PrepareSdInterface and suspends at Task.Delay(..., ct).
// Once it returns a pending task, we cancel synchronously: Task.Delay
// observes the cancellation immediately. Under the old Thread.Sleep
// code the cancel would be ignored and the call would complete
// normally — no OperationCanceledException would be thrown.
var opTask = device.GetSdCardFilesAsync(cts.Token);
cts.Cancel();

await Assert.ThrowsAnyAsync<OperationCanceledException>(() => opTask);
}

[Fact]
public async Task DeleteSdCardFileAsync_HonorsCancellationDuringSettleDelay()
{
// Regression for #221 — symmetric with GetSdCardFilesAsync above.
var device = new TestableSdCardStreamingDevice("TestDevice");
device.CannedTextResponse = new List<string> { "Daqifi/other.bin" };
device.Connect();

using var cts = new CancellationTokenSource();
var opTask = device.DeleteSdCardFileAsync("data.bin", cts.Token);
cts.Cancel();

await Assert.ThrowsAnyAsync<OperationCanceledException>(() => opTask);
}

[Fact]
public async Task StartSdCardLoggingAsync_SendsCorrectCommandSequence()
{
Expand Down Expand Up @@ -1238,6 +1278,20 @@ protected override Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
: new List<string>();
return Task.FromResult<IReadOnlyList<string>>(response);
}

protected override async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
Func<CancellationToken, Task> setupActionAsync,
int responseTimeoutMs = 1000,
int completionTimeoutMs = 250,
CancellationToken cancellationToken = default)
{
await setupActionAsync(cancellationToken).ConfigureAwait(false);
ExecuteTextCommandCallCount++;
var response = ResponseSequence.Count > 0
? ResponseSequence.Dequeue()
: new List<string>();
return response;
}
}

/// <summary>
Expand Down Expand Up @@ -1277,6 +1331,16 @@ protected override Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
setupAction();
return Task.FromResult<IReadOnlyList<string>>(CannedTextResponse);
}

protected override async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
Func<CancellationToken, Task> setupActionAsync,
int responseTimeoutMs = 1000,
int completionTimeoutMs = 250,
CancellationToken cancellationToken = default)
{
await setupActionAsync(cancellationToken).ConfigureAwait(false);
return CannedTextResponse;
}
}

/// <summary>
Expand Down Expand Up @@ -1316,6 +1380,16 @@ protected override Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
return Task.FromResult<IReadOnlyList<string>>(CannedTextResponse);
}

protected override async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
Func<CancellationToken, Task> setupActionAsync,
int responseTimeoutMs = 1000,
int completionTimeoutMs = 250,
CancellationToken cancellationToken = default)
{
await setupActionAsync(cancellationToken).ConfigureAwait(false);
return CannedTextResponse;
}

protected override async Task ExecuteRawCaptureAsync(
Func<Stream, CancellationToken, Task> rawAction,
CancellationToken cancellationToken = default)
Expand Down
50 changes: 45 additions & 5 deletions src/Daqifi.Core/Device/DaqifiDevice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public void Connect()
/// <remarks>
/// Waits up to 10 seconds to acquire <c>_textExchangeLock</c> before
/// tearing down the consumer / producer / transport. This prevents
/// a race where an in-flight <see cref="ExecuteTextCommandAsync"/>
/// a race where an in-flight <see cref="ExecuteTextCommandAsync(Action, int, int, CancellationToken)"/>
/// is mid-swap (text consumer running on the stream, protobuf
/// consumer not yet restarted) and Disconnect rips the transport
/// out from under it. If the wait times out, Disconnect proceeds
Expand Down Expand Up @@ -392,11 +392,50 @@ protected virtual async Task ExecuteRawCaptureAsync(
/// <returns>A list of text lines received from the device.</returns>
/// <exception cref="InvalidOperationException">Thrown when the device is not connected or has no transport.</exception>
/// <exception cref="OperationCanceledException">Thrown when the operation is canceled.</exception>
protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
protected virtual Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
Action setupAction,
int responseTimeoutMs = 1000,
int completionTimeoutMs = 250,
CancellationToken cancellationToken = default)
{
return ExecuteTextCommandCoreAsync(
_ => { setupAction(); return Task.CompletedTask; },
responseTimeoutMs,
completionTimeoutMs,
cancellationToken);
}

/// <summary>
/// Async overload of <see cref="ExecuteTextCommandAsync(Action, int, int, CancellationToken)"/>
/// that accepts an async setup action so callers can <c>await</c> cancellable operations
/// (e.g. <see cref="Task.Delay(int, CancellationToken)"/>) between SCPI commands without
/// blocking the thread-pool thread.
/// </summary>
/// <param name="setupActionAsync">An async function that sends SCPI commands to the device while the text consumer is active. Receives the operation's cancellation token.</param>
/// <param name="responseTimeoutMs">The time in milliseconds to wait for the first text response after sending commands.</param>
/// <param name="completionTimeoutMs">The time in milliseconds of inactivity after the first response before considering the response complete. Defaults to 250ms.</param>
/// <param name="cancellationToken">A cancellation token to observe while waiting for the task to complete.</param>
/// <returns>A list of text lines received from the device.</returns>
/// <exception cref="InvalidOperationException">Thrown when the device is not connected or has no transport.</exception>
/// <exception cref="OperationCanceledException">Thrown when the operation is canceled.</exception>
protected virtual Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
Func<CancellationToken, Task> setupActionAsync,
int responseTimeoutMs = 1000,
int completionTimeoutMs = 250,
CancellationToken cancellationToken = default)
{
return ExecuteTextCommandCoreAsync(
setupActionAsync,
responseTimeoutMs,
completionTimeoutMs,
cancellationToken);
}

private async Task<IReadOnlyList<string>> ExecuteTextCommandCoreAsync(
Func<CancellationToken, Task> setupActionAsync,
int responseTimeoutMs,
int completionTimeoutMs,
CancellationToken cancellationToken)
{
if (responseTimeoutMs <= 0)
throw new ArgumentOutOfRangeException(nameof(responseTimeoutMs), responseTimeoutMs, "Timeout must be positive.");
Expand Down Expand Up @@ -511,8 +550,9 @@ protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(

Trace.WriteLine($"[ExecuteTextCommandAsync] Text consumer started at {sw.ElapsedMilliseconds}ms");

// Execute the setup action (sends SCPI commands)
setupAction();
// Execute the setup action (sends SCPI commands). ConfigureAwait(false)
// matches the surrounding lock-protected awaits.
await setupActionAsync(cancellationToken).ConfigureAwait(false);

Trace.WriteLine($"[ExecuteTextCommandAsync] Setup action completed at {sw.ElapsedMilliseconds}ms");

Expand Down Expand Up @@ -627,7 +667,7 @@ protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(
/// on hardware faults, or discard them if known-stale.
/// </para>
/// <para>
/// Each iteration uses <see cref="ExecuteTextCommandAsync"/>, which
/// Each iteration uses <see cref="ExecuteTextCommandAsync(Action, int, int, CancellationToken)"/>, which
/// pauses the protobuf consumer for the duration of the text exchange.
/// Avoid calling this during active streaming or concurrently with
/// other text commands.
Expand Down
16 changes: 8 additions & 8 deletions src/Daqifi.Core/Device/DaqifiStreamingDevice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,14 +340,14 @@ public async Task<IReadOnlyList<SdCardFileInfo>> GetSdCardFilesAsync(Cancellatio
IReadOnlyList<string> lines;
try
{
lines = await ExecuteTextCommandAsync(() =>
lines = await ExecuteTextCommandAsync(async ct =>
{
PrepareSdInterface();

// Allow the device firmware to complete the SPI bus switch
// before querying the SD card. Without this delay, the device
// can return SCPI error -200 (Execution error).
Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS);
await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false);

Send(ScpiMessageProducer.GetSdFileList);
}, responseTimeoutMs: 3000, cancellationToken: cancellationToken);
Expand All @@ -362,10 +362,10 @@ public async Task<IReadOnlyList<SdCardFileInfo>> GetSdCardFilesAsync(Cancellatio

await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken);

lines = await ExecuteTextCommandAsync(() =>
lines = await ExecuteTextCommandAsync(async ct =>
{
PrepareSdInterface();
Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS);
await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false);
Send(ScpiMessageProducer.GetSdFileList);
}, responseTimeoutMs: 3000, cancellationToken: cancellationToken);

Expand Down Expand Up @@ -546,10 +546,10 @@ public async Task DeleteSdCardFileAsync(string fileName, CancellationToken cance
IReadOnlyList<string> lines;
try
{
lines = await ExecuteTextCommandAsync(() =>
lines = await ExecuteTextCommandAsync(async ct =>
{
PrepareSdInterface();
Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS);
await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false);
Send(ScpiMessageProducer.DeleteSdFile(fileName));
Send(ScpiMessageProducer.GetSdFileList);
}, responseTimeoutMs: 3000, cancellationToken: cancellationToken);
Expand All @@ -562,10 +562,10 @@ public async Task DeleteSdCardFileAsync(string fileName, CancellationToken cance

await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, cancellationToken);

lines = await ExecuteTextCommandAsync(() =>
lines = await ExecuteTextCommandAsync(async ct =>
{
PrepareSdInterface();
Thread.Sleep(SD_INTERFACE_SETTLE_DELAY_MS);
await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false);
Send(ScpiMessageProducer.DeleteSdFile(fileName));
Send(ScpiMessageProducer.GetSdFileList);
}, responseTimeoutMs: 3000, cancellationToken: cancellationToken);
Expand Down