feat: Add SD card operations to core library

[tylerkron]

User description

Summary

• Add ISdCardOperations interface and implementation on DaqifiStreamingDevice for SD card file listing and logging control
• Add SdCardFileInfo model and SdCardFileListParser for parsing device file list responses
• Add ExecuteTextCommandAsync on DaqifiDevice for temporarily switching from protobuf to text consumer to collect text responses
• Add unit tests for parser, model, and SD card operations

Implements issue #52, sub-task 7.2 (SD card operations migration from desktop to core).

Test plan

• dotnet build Daqifi.Core.sln compiles with 0 errors
• dotnet test passes all 441 tests on net8.0 and net9.0
• Hardware verification with example app (--sd-list, --sd-log-start, --sd-log-stop)

🤖 Generated with Claude Code

---

PR Type

Enhancement

---

Description

• Add ISdCardOperations interface with file listing and logging control

• Implement SD card operations on DaqifiStreamingDevice class

• Add SdCardFileInfo model and SdCardFileListParser for parsing device responses

• Add ExecuteTextCommandAsync method for temporary text consumer switching

• Comprehensive unit test coverage for all new components

---

Diagram Walkthrough

flowchart LR
  A["DaqifiDevice"] -->|extends| B["DaqifiStreamingDevice"]
  B -->|implements| C["ISdCardOperations"]
  C -->|uses| D["SdCardFileInfo"]
  C -->|uses| E["SdCardFileListParser"]
  B -->|calls| F["ExecuteTextCommandAsync"]
  F -->|manages| G["Text Consumer"]
  G -->|collects| H["Device Responses"]

Loading

File Walkthrough

	Relevant files
Enhancement	ISdCardOperations.cs SD card operations interface definition                                    src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs Define interface for SD card operations with three main methods Properties for logging status and file list retrieval Methods for getting files, starting and stopping SD card logging Full XML documentation with exception specifications +52/-0    SdCardFileInfo.cs SD card file information model                                                      src/Daqifi.Core/Device/SdCard/SdCardFileInfo.cs Model class representing SD card file metadata Properties for filename and optional creation date Constructor with null validation for filename +33/-0    SdCardFileListParser.cs SD card file list response parser                                                src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs Static parser for converting device responses to file info objects Strips Daqifi/ directory prefix from file paths Parses timestamps from log filenames matching pattern log_YYYYMMDD_HHMMSS.bin Handles empty lines and nested paths gracefully +87/-0    DaqifiDevice.cs Add text command execution capability                                        src/Daqifi.Core/Device/DaqifiDevice.cs Add ExecuteTextCommandAsync protected method for text-based command execution Temporarily switches from protobuf to line-based text consumer Restores protobuf consumer in finally block for reliability Includes comprehensive XML documentation and error handling +74/-0    DaqifiStreamingDevice.cs Implement SD card operations on streaming device                  src/Daqifi.Core/Device/DaqifiStreamingDevice.cs Implement ISdCardOperations interface on streaming device class Add IsLoggingToSdCard property and SdCardFiles collection Implement GetSdCardFilesAsync with SD interface preparation and restoration Implement StartSdCardLoggingAsync with auto-generated timestamped filenames Implement StopSdCardLoggingAsync with proper state management +119/-1
Tests	SdCardFileInfoTests.cs Unit tests for file info model                                                      src/Daqifi.Core.Tests/Device/SdCard/SdCardFileInfoTests.cs Test constructor with filename only Test constructor with filename and creation date Test null filename throws ArgumentNullException Test null creation date when not provided +50/-0    SdCardFileListParserTests.cs Unit tests for file list parser                                                    src/Daqifi.Core.Tests/Device/SdCard/SdCardFileListParserTests.cs Test parsing multiple files from response lines Test stripping Daqifi/ directory prefix Test date parsing from log filenames Test handling of non-log files and empty lines Test nested path extraction and invalid date format handling +123/-0  SdCardOperationsTests.cs Comprehensive unit tests for SD card operations                    src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs Test operations throw when device disconnected Test correct SCPI command sequences for file listing and logging Test response parsing and state management Test LAN interface restoration after SD operations Test auto-generated timestamped filenames Include TestableSdCardStreamingDevice mock for capturing sent messages +279/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SCPI command injection Description: User-controlled fileName is incorporated into an SD logging SCPI command via ScpiMessageProducer.SetSdLoggingFileName(logFileName) without visible validation/escaping, which could enable SCPI/command injection if the producer does not robustly escape quotes and delimiters (e.g., a filename containing " or ;). DaqifiStreamingDevice.cs [269-284] Referred Code public Task StartSdCardLoggingAsync(string? fileName = null, CancellationToken cancellationToken = default) { if (!IsConnected) { throw new InvalidOperationException("Device is not connected."); } cancellationToken.ThrowIfCancellationRequested(); var logFileName = fileName ?? $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; Send(ScpiMessageProducer.EnableStorageSd); Send(ScpiMessageProducer.SetSdLoggingFileName(logFileName)); Send(ScpiMessageProducer.SetProtobufStreamFormat); Send(ScpiMessageProducer.StartStreaming(StreamingFrequency));
Ticket Compliance
🟡	🎫 #52 🟢 SD card operation tests (with mock responses) 🔴 WiFi SSID and password configuration WiFi security mode selection (Open, WEP, WPA, WPA2) LAN settings (IP address, subnet, gateway) Network configuration persistence on device Network status queries Apply configuration and reboot sequence File retrieval from device SD card format commands Storage capacity queries File management (delete, rename) Device settings save/load Configuration validation Default configuration restoration Configuration versioning Cross-device configuration compatibility Firmware version comparison and validation Bootloader mode detection and entry Firmware upload protocol (HID-based) Upload progress tracking Firmware verification Device reboot after update Rollback on failure Device-level logging interface Diagnostic message capture Performance metrics (connection time, message latency) Error tracking and reporting Debug mode with verbose logging Unit tests for network configuration commands Text message consumer tests (SD card file listings) Firmware version comparison tests Configuration validation tests Mock bootloader communication tests Integration tests for complete workflows Error handling and rollback tests ⚪ Enable/disable SD card logging mode File listing (text-based response handling) Text message consumer for SD card responses (non-protobuf)
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing null checks: ParseFileList does not validate lines for null, which can cause a NullReferenceException when iterating external input instead of producing a clear, actionable error. Referred Code public static IReadOnlyList<SdCardFileInfo> ParseFileList(IEnumerable<string> lines) { var files = new List<SdCardFileInfo>(); foreach (var line in lines) { if (string.IsNullOrWhiteSpace(line)) { continue; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unsanitized SCPI input: StartSdCardLoggingAsync forwards user-provided fileName into an SCPI command without validating/escaping potentially dangerous characters (e.g., quotes/newlines), which can enable command injection or malformed device commands. Referred Code public Task StartSdCardLoggingAsync(string? fileName = null, CancellationToken cancellationToken = default) { if (!IsConnected) { throw new InvalidOperationException("Device is not connected."); } cancellationToken.ThrowIfCancellationRequested(); var logFileName = fileName ?? $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; Send(ScpiMessageProducer.EnableStorageSd); Send(ScpiMessageProducer.SetSdLoggingFileName(logFileName)); Send(ScpiMessageProducer.SetProtobufStreamFormat); Send(ScpiMessageProducer.StartStreaming(StreamingFrequency)); Learn more about managing compliance generic rules or creating your own custom rules
⚪
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 671ee60

Category	Suggestion	Impact
Possible issue	Make response collection thread-safe To prevent a race condition, use a lock to ensure thread-safe access to the collectedLines list, which is modified by the message consumer thread and read by the main method thread. src/Daqifi.Core/Device/DaqifiDevice.cs [261-305] var collectedLines = new List<string>(); +var linesLock = new object(); +var lastMessageTime = DateTime.UtcNow; try { // Stop the protobuf consumer if (_messageConsumer != null) { _messageConsumer.MessageReceived -= OnInboundMessageReceived; _messageConsumer.StopSafely(); } // Create a temporary text consumer on the same stream using var textConsumer = new StreamMessageConsumer<string>( _transport.Stream, new LineBasedMessageParser()); textConsumer.MessageReceived += (_, e) => { - collectedLines.Add(e.Message.Data); + lock (linesLock) + { + collectedLines.Add(e.Message.Data); + lastMessageTime = DateTime.UtcNow; + } }; textConsumer.Start(); await Task.Delay(50, cancellationToken); // Execute the setup action (sends SCPI commands) setupAction(); - // Wait for responses using an inactivity-based timeout. - // Instead of a fixed delay, poll for new messages and stop when - // no new messages arrive within the timeout window. - var lastMessageTime = DateTime.UtcNow; var inactivityTimeout = TimeSpan.FromMilliseconds(responseTimeoutMs); var maxWait = TimeSpan.FromMilliseconds(responseTimeoutMs * 5); var startTime = DateTime.UtcNow; - while (DateTime.UtcNow - lastMessageTime < inactivityTimeout && - DateTime.UtcNow - startTime < maxWait) + while (true) { - var previousCount = collectedLines.Count; await Task.Delay(50, cancellationToken); - if (collectedLines.Count > previousCount) + + DateTime last; + lock (linesLock) { - lastMessageTime = DateTime.UtcNow; + last = lastMessageTime; + } + + if (DateTime.UtcNow - last >= inactivityTimeout || + DateTime.UtcNow - startTime >= maxWait) + { + break; } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a race condition where collectedLines is modified and read from different threads without synchronization, which could lead to data corruption or runtime exceptions.	Medium
	✅ Enforce consumer stop before switching Suggestion Impact: The commit now checks the return value of StopSafely (with a timeout) and, if it fails, performs a fallback stop via _messageConsumer.Stop() instead of ignoring the result; it does not throw an exception as suggested. code diff: // Stop the protobuf consumer if (_messageConsumer != null) { _messageConsumer.MessageReceived -= OnInboundMessageReceived; - _messageConsumer.StopSafely(); + var stopped = _messageConsumer.StopSafely(timeoutMs: 6000); + if (!stopped) + { + _messageConsumer.Stop(); + } } Check the boolean return value of _messageConsumer.StopSafely() and throw an exception if it returns false to prevent multiple consumers from reading the same stream concurrently. src/Daqifi.Core/Device/DaqifiDevice.cs [266-270] if (_messageConsumer != null) { _messageConsumer.MessageReceived -= OnInboundMessageReceived; - _messageConsumer.StopSafely(); + if (!_messageConsumer.StopSafely()) + { + throw new InvalidOperationException("Failed to stop protobuf consumer before executing text command."); + } } [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the return value of StopSafely() is ignored, which could lead to two consumers reading the same stream if the first one fails to stop, improving the robustness of the code.	Medium
	Prevent lost messages on restart To prevent a race condition and potential message loss, reorder the operations to first subscribe to the _messageConsumer.MessageReceived event and then call _messageConsumer.Start(). src/Daqifi.Core/Device/DaqifiDevice.cs [313-317] if (_messageConsumer != null) { + _messageConsumer.MessageReceived += OnInboundMessageReceived; _messageConsumer.Start(); - _messageConsumer.MessageReceived += OnInboundMessageReceived; } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a race condition where messages could be missed between starting the consumer and subscribing to its events, and the proposed fix is the standard practice to avoid this issue.	Medium
Update

---

Previous suggestions

✅ Suggestions up to commit 2e96955

Category	Suggestion	Impact
High-level	✅ Re-evaluate the consumer swapping mechanism Suggestion Impact: The commit removed the fixed Task.Delay-based wait and replaced it with an inactivity-based timeout loop that waits until no new text lines arrive, reducing brittleness. It did not implement a unified dispatcher or remove consumer swapping. code diff: - // Wait for responses - await Task.Delay(responseTimeoutMs, cancellationToken); + // Wait for responses using an inactivity-based timeout. + // Instead of a fixed delay, poll for new messages and stop when + // no new messages arrive within the timeout window. + var lastMessageTime = DateTime.UtcNow; + var inactivityTimeout = TimeSpan.FromMilliseconds(responseTimeoutMs); + var maxWait = TimeSpan.FromMilliseconds(responseTimeoutMs * 5); + var startTime = DateTime.UtcNow; + + while (DateTime.UtcNow - lastMessageTime < inactivityTimeout && + DateTime.UtcNow - startTime < maxWait) + { + var previousCount = collectedLines.Count; + await Task.Delay(50, cancellationToken); + if (collectedLines.Count > previousCount) + { + lastMessageTime = DateTime.UtcNow; + } + } The ExecuteTextCommandAsync method's strategy of swapping message consumers and using a fixed delay is unreliable. A better approach would be a unified message dispatcher that can handle both protobuf and text messages from a single stream, removing the need for swapping and fixed timers. Examples: src/Daqifi.Core/Device/DaqifiDevice.cs [244-305] protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync( Action setupAction, int responseTimeoutMs = 1000, CancellationToken cancellationToken = default) { if (!IsConnected) { throw new InvalidOperationException("Device is not connected."); } ... (clipped 52 lines) Solution Walkthrough: Before: // In DaqifiDevice.cs protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(...) { // 1. Stop the main protobuf message consumer _messageConsumer.StopSafely(); try { // 2. Create and start a temporary text consumer using var textConsumer = new StreamMessageConsumer<string>(...); textConsumer.Start(); // 3. Send commands to the device setupAction(); // 4. Wait for a fixed time, hoping all responses arrive await Task.Delay(responseTimeoutMs, ...); } finally { // 5. Restart the main protobuf consumer _messageConsumer.Start(); } return collectedLines; } After: // In a new UnifiedMessageConsumer or similar private void ProcessStreamData() { while (running) { // Read from stream and distinguish message type if (IsProtobufMessage(stream)) { var message = ParseProtobuf(stream); RaiseProtobufMessageEvent(message); } else // Assumed to be text { var line = ReadLine(stream); RaiseTextMessageEvent(line); } } } // In DaqifiDevice.cs protected virtual async Task<IReadOnlyList<string>> ExecuteTextCommandAsync(...) { // 1. Register a temporary handler for text messages var responseTask = CollectTextResponsesUntilComplete(); // 2. Send commands setupAction(); // 3. Asynchronously wait for the handler to signal completion return await responseTask; } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant architectural weakness in ExecuteTextCommandAsync; the consumer-swapping and fixed-delay Task.Delay are brittle and can lead to race conditions or missed/incomplete device responses.	High
Possible issue	✅ Replace fixed delay with inactivity-based timeout Suggestion Impact: The fixed delay waiting for responses was replaced with an inactivity-based polling loop that updates the last-message timestamp when new lines arrive; the commit also adds an overall max-wait cap. code diff: - // Wait for responses - await Task.Delay(responseTimeoutMs, cancellationToken); + // Wait for responses using an inactivity-based timeout. + // Instead of a fixed delay, poll for new messages and stop when + // no new messages arrive within the timeout window. + var lastMessageTime = DateTime.UtcNow; + var inactivityTimeout = TimeSpan.FromMilliseconds(responseTimeoutMs); + var maxWait = TimeSpan.FromMilliseconds(responseTimeoutMs * 5); + var startTime = DateTime.UtcNow; + + while (DateTime.UtcNow - lastMessageTime < inactivityTimeout && + DateTime.UtcNow - startTime < maxWait) + { + var previousCount = collectedLines.Count; + await Task.Delay(50, cancellationToken); + if (collectedLines.Count > previousCount) + { + lastMessageTime = DateTime.UtcNow; + } + } Replace the fixed Task.Delay in ExecuteTextCommandAsync with a polling loop that waits for a period of inactivity. This ensures all device responses are received before timing out, improving reliability. src/Daqifi.Core/Device/DaqifiDevice.cs [282-292] textConsumer.Start(); - await Task.Delay(50, cancellationToken); // Execute the setup action (sends SCPI commands) setupAction(); - // Wait for responses - await Task.Delay(responseTimeoutMs, cancellationToken); + // Wait for responses with an inactivity timeout + var lastMessageTime = DateTime.UtcNow; + var inactivityTimeout = TimeSpan.FromMilliseconds(responseTimeoutMs); + + while (DateTime.UtcNow - lastMessageTime < inactivityTimeout) + { + var originalCount = collectedLines.Count; + await Task.Delay(50, cancellationToken); // Poll for new messages + if (collectedLines.Count > originalCount) + { + lastMessageTime = DateTime.UtcNow; + } + } // Stop the text consumer textConsumer.StopSafely(); [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that using a fixed Task.Delay for network responses is unreliable and can lead to truncated data. The proposed inactivity-based timeout is a much more robust solution that significantly improves the reliability of the ExecuteTextCommandAsync method.	Medium
General	✅ Use StopStreaming abstraction Suggestion Impact: StopSdCardLoggingAsync now calls StopStreaming() instead of sending the StopStreaming SCPI message directly, and the redundant IsStreaming = false line was removed. code diff: - Send(ScpiMessageProducer.StopStreaming); + StopStreaming(); Send(ScpiMessageProducer.DisableStorageSd); _isLoggingToSdCard = false; - IsStreaming = false; In StopSdCardLoggingAsync, replace the direct call to Send(ScpiMessageProducer.StopStreaming) with a call to the existing StopStreaming() method to ensure consistent state management. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [307-311] -Send(ScpiMessageProducer.StopStreaming); +StopStreaming(); Send(ScpiMessageProducer.DisableStorageSd); _isLoggingToSdCard = false; -IsStreaming = false; [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly promotes code reuse and encapsulation by using the existing StopStreaming() method. This centralizes state management for streaming, improving maintainability and reducing the risk of inconsistencies.	Medium
	✅ Enforce non-empty log filename Suggestion Impact: Updated log filename selection to use string.IsNullOrWhiteSpace and fall back to a timestamped default filename when the provided fileName is null/empty/whitespace. code diff: - var logFileName = fileName ?? $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; + var logFileName = !string.IsNullOrWhiteSpace(fileName) + ? fileName! + : $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; In StartSdCardLoggingAsync, use string.IsNullOrWhiteSpace to check the fileName parameter. If it is null, empty, or whitespace, generate a timestamped filename. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [278] -var logFileName = fileName ?? $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; +var logFileName = !string.IsNullOrWhiteSpace(fileName) + ? fileName! + : $"log_{DateTime.Now:yyyyMMdd_HHmmss}.bin"; [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: This is a good defensive programming practice that prevents sending invalid empty or whitespace filenames to the device, making the StartSdCardLoggingAsync method more robust against improper usage.	Low
	✅ Add null-guard to parser input Suggestion Impact: Implemented the suggested null check at the start of `ParseFileList`, throwing `ArgumentNullException` if `lines` is null. code diff: + if (lines == null) + { + throw new ArgumentNullException(nameof(lines)); + } + Add a null check for the lines parameter in ParseFileList to prevent a NullReferenceException if null is passed as an argument. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [25-27] public static IReadOnlyList<SdCardFileInfo> ParseFileList(IEnumerable<string> lines) { + if (lines == null) + { + throw new ArgumentNullException(nameof(lines)); + } var files = new List<SdCardFileInfo>(); [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: The suggestion adds a standard null check for a public method's parameter, which is good practice for creating a robust API and preventing NullReferenceException from unexpected inputs.	Low

[tylerkron]

/improve

[qodo-code-review]

Persistent suggestions updated to latest commit 671ee60