fix: merge analog/digital output rows and enable channels for SD logging

[tylerkron]

Summary

• Streaming output merge: The device sends analog and digital data in separate protobuf messages with the same timestamp. Previously each was written as its own row, producing unusable CSV/JSONL output (e.g. {"analog":[...],"digital":""} followed by {"analog":[],"digital":"00-00"}). Now analog and digital are buffered and merged into a single row per sample across all output formats (text, CSV, JSONL).
• SD card channel enablement: Before calling StartSdCardLoggingAsync, the app now explicitly enables ADC channels (using the device's reported AnalogInputChannels count if no --channels mask is provided) and DIO ports. Without this, the device had no channels enabled and log files were empty.

Depends on

• fix: add LAN disable and SD stream interface setup to StartSdCardLoggingAsync daqifi-core#153 (Core now handles LAN disable and SD stream interface setup)

Test plan

• --serial /dev/cu.usbmodem101 --rate 10 --duration 5 --format csv produces one row per sample with both analog and digital data
• --serial /dev/cu.usbmodem101 --rate 10 --duration 5 --format jsonl produces one JSON object per sample with both analog and digital
• --serial /dev/cu.usbmodem101 --sd-log-start --sd-log-format csv followed by --sd-list shows a new file
• --serial /dev/cu.usbmodem101 --sd-log-start --sd-log-format protobuf followed by --sd-list shows a new file

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Merge analog/digital rows and enable channels for SD logging

🐞 Bug fix

Walkthroughs

Description

• Merge analog and digital protobuf messages into single output rows
  - Device sends separate messages with same timestamp; now buffered and combined
  - Fixes CSV/JSONL output for data analysis tools
• Enable ADC channels and DIO ports before SD card logging
  - Automatically enables all channels if none specified via --channels
  - Prevents empty log files due to disabled channels
• Auto-parse CSV and JSON files after SD download
  - Previously only .bin files were parsed; now all formats supported
• Pass device calibration config to SD card file parser
  - Enables proper voltage scaling of raw ADC values in downloaded files

Diagram

flowchart LR
  A["Device sends analog<br/>and digital messages<br/>with same timestamp"]
  B["Buffer analog message<br/>in pendingAnalog"]
  C["Match digital message<br/>by timestamp"]
  D["Merge into single<br/>output row"]
  E["Write CSV/JSONL/text"]
  F["Enable ADC channels<br/>and DIO ports"]
  G["Start SD card logging"]
  H["Download and parse<br/>SD file with device<br/>calibration config"]
  
  A --> B
  B --> C
  C --> D
  D --> E
  F --> G
  G --> H

Loading

File Changes

1. Program.cs 🐞 Bug fix +140/-42

Merge streaming messages and enable SD logging channels

• Implemented analog/digital message buffering and merging in RunStreamingSessionAsync
 - Added pendingAnalog buffer and lock for thread-safe message pairing
 - Refactored message handling to match analog and digital by timestamp
 - Flush buffered analog-only messages at stream end
• Replaced WriteStreamSample with WriteMergedSample accepting paired messages
 - Updated ToTextLine, ToCsvLine, ToJsonLine to accept analog and optional digital messages
 - Prefer paired digital message; fall back to analog message's own digital data
• Added channel enablement before SD card logging in RunSdCardOperationAsync
 - Auto-generate channel mask from device's AnalogInputChannels if not specified
 - Send EnableAdcChannels and EnableDioPorts commands with delays
• Extended SD file parsing to support CSV and JSON formats
 - Pass device calibration config via SdCardDeviceConfiguration.FromDevice()
 - Updated RunSdCardParseAsync signature to accept optional deviceConfig parameter
 - Removed hardcoded FallbackTimestampFrequency in favor of device config

Program.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. SD mask not validated ☑ 🐞 Bug ⛯ Reliability

Description

RunSdCardOperationAsync sends EnableAdcChannels(channelMask) using the user-provided --channels
value without validating that it contains only '0'/'1'. An invalid mask (e.g., containing
spaces/other characters) will be sent to the device and can cause SD logging setup to fail with
unclear feedback.

Code

Program.cs[R716-730]

+                var channelMask = options.ChannelMask;
+                if (string.IsNullOrWhiteSpace(channelMask))
+                {
+                    var adcCount = streamingDevice.Metadata.Capabilities.AnalogInputChannels;
+                    if (adcCount > 0)
+                    {
+                        channelMask = new string('1', adcCount);
+                    }
+                }
+
+                if (!string.IsNullOrWhiteSpace(channelMask))
+                {
+                    streamingDevice.Send(ScpiMessageProducer.EnableAdcChannels(channelMask));
+                    await Task.Delay(100);
+                }

Evidence

The streaming path validates ChannelMask using IsValidChannelMask before sending SCPI, but the SD
logging start path now sends the mask directly (new behavior) without validation; IsValidChannelMask
exists and checks characters are only '0'/'1'.

Program.cs[260-269]
Program.cs[710-730]
Program.cs[1284-1295]

Agent prompt

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

## Issue description
During `--sd-log-start`, the CLI now sends `EnableAdcChannels(channelMask)` but does not validate the user-provided `--channels` string. This is inconsistent with the streaming path and can send malformed masks to the device.
## Issue Context
`IsValidChannelMask` exists and is already used for streaming sessions. SD logging should reject invalid masks with a clear error message before sending SCPI commands.
## Fix Focus Areas
- If `options.ChannelMask` is non-empty, validate it with `IsValidChannelMask` and fail fast with a clear error (mirroring streaming behavior).
- Optionally validate the computed default mask too (should always pass, but keeps behavior uniform).
## Fix Focus Areas (code references)
- Program.cs[710-730]
- Program.cs[260-269]
- Program.cs[1284-1295]

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

---

2. Sample count can inflate 🐞 Bug ✓ Correctness

Description

WriteMergedSample increments messageCount before enforcing messageLimit, so calls that return early
still increase the counter without writing an output row. This can make --min-samples validation
incorrect and can misrepresent how many rows were actually emitted when --limit is used.

Code

Program.cs[R1152-1156]

+        var currentCount = Interlocked.Increment(ref messageCount);
+        if (messageLimit > 0 && currentCount > messageLimit)
+        {
+            return;
+        }

Evidence

WriteMergedSample increments the shared counter and returns when over the limit, leaving
messageCount larger than the number of rows written; later, messageCount is used as the sample count
for validation.

Program.cs[1143-1175]
Program.cs[286-301]

Agent prompt

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

## Issue description
`WriteMergedSample` increments `messageCount` even when `messageLimit` is exceeded and the method returns without writing a row. This can inflate `messageCount` relative to emitted rows and affect `--min-samples` validation.
## Issue Context
`messageCount` is used as the number of samples/rows produced during streaming and is checked against `options.MinSamples` after streaming stops.
## Fix Focus Areas
- Ensure `messageCount` only increments when a row is actually written (e.g., decrement before returning, or compute an increment only when under the limit).
- Keep the limit behavior consistent with cancellation (`stopCts.Cancel()`) when the last allowed row is written.
## Fix Focus Areas (code references)
- Program.cs[1143-1175]
- Program.cs[286-301]

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

---

3. Help text now misleading 🐞 Bug ⚙ Maintainability

Description

The CLI now auto-parses downloaded SD logs with .csv and .json extensions, but the help text still
claims --sd-parse parses only .bin files. This creates a user-facing mismatch between behavior and
documentation.

Code

Program.cs[R809-821]

+                    // Parse the downloaded file (any supported format)
                var downloadExt = Path.GetExtension(result.FilePath).ToLowerInvariant();
-                    if (downloadExt == ".bin")
+                    if (downloadExt is ".bin" or ".csv" or ".json")
                {
                    Console.WriteLine();
                    Console.WriteLine("--- Parsing downloaded file ---");
                    options.SdParsePath = result.FilePath;
-                        return await RunSdCardParseAsync(options);
+
+                        // Pass the connected device's config so the parser can
+                        // scale raw ADC values using the device's calibration.
+                        var deviceConfig = SdCardDeviceConfiguration.FromDevice((DaqifiDevice)device);
+                        return await RunSdCardParseAsync(options, deviceConfig);
                }

Evidence

The SD download flow triggers parsing for .bin/.csv/.json, while PrintHelp still documents
--sd-parse as .bin-only.

Program.cs[809-821]
Program.cs[1328-1337]

Agent prompt

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

## Issue description
Help text says `--sd-parse` parses only `.bin`, but the app now parses `.bin`, `.csv`, and `.json` (at least in the auto-parse-on-download path). This inconsistency will confuse users.
## Issue Context
`PrintHelp()` is the source of CLI usage docs shown to users.
## Fix Focus Areas
- Update `--sd-parse` help text to reflect supported extensions (`.bin`, `.csv`, `.json`) or clarify the supported formats.
## Fix Focus Areas (code references)
- Program.cs[809-821]
- Program.cs[1328-1337]

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

---

ⓘ The new review experience is currently in Beta. Learn more