Skip to content

fix: add LAN disable and SD stream interface setup to StartSdCardLoggingAsync #153

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 3 commits into from
Mar 20, 2026
Merged

fix: add LAN disable and SD stream interface setup to StartSdCardLoggingAsync #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3cabae6
fix: add LAN disable and SD stream interface setup to StartSdCardLogg…
tylerkron Mar 20, 2026
5c1e1c6
fix: guard StartSdCardLoggingAsync against non-USB connections
tylerkron Mar 20, 2026
6eb9520
Merge branch 'main' into fix/sd-card-logging-spi-bus-setup
tylerkron Mar 20, 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
78 changes: 60 additions & 18 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 @@ -113,11 +113,26 @@ public async Task StartSdCardLoggingAsync_SendsCorrectCommandSequence()

// Assert
var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
Assert.Equal(4, sentCommands.Count);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.bin\"", sentCommands[1]);
Assert.Equal("SYSTem:STReam:FORmat 0", sentCommands[2]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[3]);
Assert.Equal(6, sentCommands.Count);
Assert.Equal("SYSTem:COMMunicate:LAN:ENAbled 0", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[1]);
Assert.Equal("SYSTem:STReam:INTerface 2", sentCommands[2]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.bin\"", sentCommands[3]);
Assert.Equal("SYSTem:STReam:FORmat 0", sentCommands[4]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[5]);
}

[Fact]
public async Task StartSdCardLoggingAsync_OverNonUsbConnection_ThrowsInvalidOperationException()
{
// Arrange — use a device that reports IsUsbConnection = false
var device = new TestableNonUsbStreamingDevice("TestDevice");
device.Connect();

// Act & Assert
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => device.StartSdCardLoggingAsync("test.bin"));
Assert.Contains("USB", ex.Message);
}

[Fact]
Expand Down Expand Up @@ -182,9 +197,10 @@ public async Task StopSdCardLoggingAsync_SendsCorrectCommands()

// Assert
var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
Assert.Equal(2, sentCommands.Count);
Assert.Equal(3, sentCommands.Count);
Assert.Equal("SYSTem:StopStreamData", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:ENAble 0", sentCommands[1]);
Assert.Equal("SYSTem:STReam:INTerface 0", sentCommands[2]); // Restore USB
}

[Fact]
Expand Down Expand Up @@ -285,11 +301,13 @@ public async Task StartSdCardLoggingAsync_WithJsonFormat_SendsJsonFormatCommand(

// Assert
var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
Assert.Equal(4, sentCommands.Count);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.json\"", sentCommands[1]);
Assert.Equal("SYSTem:STReam:FORmat 1", sentCommands[2]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[3]);
Assert.Equal(6, sentCommands.Count);
Assert.Equal("SYSTem:COMMunicate:LAN:ENAbled 0", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[1]);
Assert.Equal("SYSTem:STReam:INTerface 2", sentCommands[2]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.json\"", sentCommands[3]);
Assert.Equal("SYSTem:STReam:FORmat 1", sentCommands[4]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[5]);
}

[Fact]
Expand All @@ -304,11 +322,13 @@ public async Task StartSdCardLoggingAsync_WithCsvFormat_SendsCsvFormatCommand()

// Assert
var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
Assert.Equal(4, sentCommands.Count);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.csv\"", sentCommands[1]);
Assert.Equal("SYSTem:STReam:FORmat 2", sentCommands[2]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[3]);
Assert.Equal(6, sentCommands.Count);
Assert.Equal("SYSTem:COMMunicate:LAN:ENAbled 0", sentCommands[0]);
Assert.Equal("SYSTem:STORage:SD:ENAble 1", sentCommands[1]);
Assert.Equal("SYSTem:STReam:INTerface 2", sentCommands[2]);
Assert.Equal("SYSTem:STORage:SD:LOGging \"mylog.csv\"", sentCommands[3]);
Assert.Equal("SYSTem:STReam:FORmat 2", sentCommands[4]);
Assert.Equal("SYSTem:StartStreamData 100", sentCommands[5]);
}

[Fact]
Expand Down Expand Up @@ -695,8 +715,8 @@ await Assert.ThrowsAsync<InvalidOperationException>(
[Fact]
public async Task DownloadSdCardFileAsync_WhenNotSerialTransport_Throws()
{
// Arrange — use the testable device which has no transport (simulates non-USB)
var device = new TestableSdCardStreamingDevice("TestDevice");
// Arrange — use a device that reports IsUsbConnection = false
var device = new TestableNonUsbStreamingDevice("TestDevice");
device.Connect();
using var stream = new MemoryStream();

Expand Down Expand Up @@ -903,6 +923,11 @@ private class TestableSdCardStreamingDevice : DaqifiStreamingDevice
public List<IOutboundMessage<string>> SentMessages { get; } = new();
public List<string> CannedTextResponse { get; set; } = new();

/// <summary>
/// Simulates a USB connection so SD card operations are allowed.
/// </summary>
public override bool IsUsbConnection => true;

public TestableSdCardStreamingDevice(string name, IPAddress? ipAddress = null)
: base(name, ipAddress)
{
Expand Down Expand Up @@ -978,5 +1003,22 @@ protected override async Task ExecuteRawCaptureAsync(
await rawAction(fakeStream, cancellationToken);
}
}
/// <summary>
/// A testable device that reports IsUsbConnection = false to verify
/// that SD card operations reject non-USB connections.
/// </summary>
private class TestableNonUsbStreamingDevice : DaqifiStreamingDevice
{
public TestableNonUsbStreamingDevice(string name, IPAddress? ipAddress = null)
: base(name, ipAddress)
{
}

public override bool IsUsbConnection => false;

public override void Send<T>(IOutboundMessage<T> message)
{
}
}
}
}
22 changes: 22 additions & 0 deletions src/Daqifi.Core/Device/DaqifiStreamingDevice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,13 @@ public async Task StartSdCardLoggingAsync(string? fileName = null, string? chann
throw new InvalidOperationException("Device is not connected.");
}

if (!IsUsbConnection)
{
throw new InvalidOperationException(
"SD card logging requires a USB/serial connection. " +
"The SD card and WiFi/LAN share the SPI bus, so SD operations cannot be performed over a network connection.");
}

cancellationToken.ThrowIfCancellationRequested();

var extension = format switch
Expand All @@ -399,9 +406,18 @@ public async Task StartSdCardLoggingAsync(string? fileName = null, string? chann
// SdCardLogFormat integer values map 1:1 to SYSTem:STReam:FORmat SCPI arguments
var formatCommand = new ScpiMessage($"SYSTem:STReam:FORmat {(int)format}");

// SD card and LAN share the SPI bus on the hardware, so LAN must be
// disabled before the SD card can be used.
Send(ScpiMessageProducer.DisableNetworkLan);
await Task.Delay(100, cancellationToken);

Send(ScpiMessageProducer.EnableStorageSd);
await Task.Delay(100, cancellationToken);

// Route the data stream to the SD card interface.
Send(ScpiMessageProducer.SetStreamInterface(StreamInterface.SdCard));
await Task.Delay(100, cancellationToken);

Send(ScpiMessageProducer.SetSdLoggingFileName(logFileName));
await Task.Delay(100, cancellationToken);

Expand Down Expand Up @@ -439,6 +455,12 @@ public Task StopSdCardLoggingAsync(CancellationToken cancellationToken = default
StopStreaming();
Send(ScpiMessageProducer.DisableStorageSd);

// Restore stream interface to USB so subsequent non-SD operations work.
if (IsUsbConnection)
{
Send(ScpiMessageProducer.SetStreamInterface(StreamInterface.Usb));
}

Comment on lines 455 to +463
Copy link
Copy Markdown

@qodo-code-review qodo-code-review bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Lan not restored on stop 🐞 Bug ⛯ Reliability

StopSdCardLoggingAsync disables SD storage but never re-enables LAN after StartSdCardLoggingAsync
disabled it, leaving the device in a LAN-disabled state after logging. This can break later network
operations unless callers know to manually call PrepareLanInterface.
Agent Prompt 
### Issue description
`StopSdCardLoggingAsync` does not restore the LAN interface after `StartSdCardLoggingAsync` disables it, leaving the device in a LAN-disabled state.

### Issue Context
The class already defines `PrepareLanInterface()` as the standard way to restore LAN (it disables SD storage and enables LAN). Other SD operations restore LAN in `finally` blocks.

### Fix Focus Areas
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[439-460]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[272-285]

### Suggested fix
- In `StopSdCardLoggingAsync`, after `Send(DisableStorageSd)`, restore LAN using one of:
  - Call `PrepareLanInterface()` (may resend DisableStorageSd but is idempotent), or
  - Send `ScpiMessageProducer.EnableNetworkLan` explicitly.
- Consider adding a short settle delay (reuse `SD_INTERFACE_SETTLE_DELAY_MS`) if subsequent commands are likely to follow immediately.

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

_isLoggingToSdCard = false;

return Task.CompletedTask;
Expand Down
Loading