feat: Import SD card logging sessions into application#375
Conversation
Add the ability to import SD card .bin log files into the desktop app's SQLite database for viewing, analysis, and CSV export. This implements the desktop side of #126, wiring up the core library's SdCardFileParser and ISdCardOperations to the desktop data model. New SdCardSessionImporter service with three entry points: - ImportFromFileAsync: import a local .bin file (no device needed) - ImportFromStreamAsync: import from a stream - ImportFromDeviceAsync: download from USB device then import Maps core SdCardLogEntry to desktop DataSample entities (one per analog/ digital channel per sample), creates LoggingSession in SQLite, and uses batch bulk inserts for performance. UI additions: - "Import .bin File" button in APPLICATION LOGS tab for local file import - Per-file "Import" button in DEVICE LOGS file list for USB device import - "Import All" button in DEVICE LOGS header to batch import all files - Progress overlay during download and import operations Also adds DownloadSdCardFileAsync and DeleteSdCardFileAsync to the desktop IStreamingDevice interface, and conditional local core project reference support via DaqifiCoreProjectPath MSBuild property. Closes #374 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||||||||||
- Update Daqifi.Core NuGet from 0.14.0 to 0.15.0 across all projects to include SD card parsing/download types needed for import feature - Add null safety check for downloadResult.FilePath before use - Validate downloaded file is in expected temp directory - Log cleanup errors instead of silently swallowing them - Sanitize user-facing error messages to avoid leaking internal details - Add audit logging for import, download, and delete operations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix StartSdCardLogging sending individual channel enable commands instead of a combined bitmask. Each EnableAdcChannels SCPI command replaces the previous setting, so sending one-channel-at-a-time resulted in only the last channel being logged. Now builds a single bitmask for all active analog channels before sending. - Suppress streaming data from appearing on the Live Graph when in Log to Device mode by checking Mode in OnStreamMessageReceived. - Add diagnostic logging to SdCardSessionImporter for device config, first sample details, and channel discovery to aid debugging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stics - Changed ImportFromDeviceAsync to use ParseAsync with the original device filename instead of ParseFileAsync with the temp path. This fixes: 1. Session names showing temp GUIDs instead of the original log filename 2. Date extraction from log filenames (e.g., log_20260206_101851.bin) - Added download result diagnostics (reported size, disk size, temp path) - Added empty file detection with descriptive error message - Added warning log when 0 samples are imported for debugging Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Log first two samples (instead of just first) to verify timestamps are properly spaced after the core parser fix - Add warning when TimestampFrequency is 0, indicating firmware may not include TimestampFreq in logged messages - Include sample index in diagnostic log for easier debugging Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Device firmware does not include TimestampFreq in SD card log data, causing all imported samples to have identical timestamps. Changes: - Store TimestampFrequency on AbstractStreamingDevice from status messages - Expose TimestampFrequency on IStreamingDevice interface - Pass device's TimestampFrequency as FallbackTimestampFrequency when parsing SD card files via ImportFromDeviceAsync - Add diagnostic logging for first two samples to verify timestamp spacing - Add warning when no TimestampFrequency is available Requires daqifi-core PR #117 (FallbackTimestampFrequency support) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closes daqifi/daqifi-core#137 Daqifi.Core 0.18.1 (daqifi/daqifi-core#138) adds: - DaqifiDevice.TimestampFrequency populated from status messages - SdCardFileListParser filters plain ERROR lines and control-char filenames - DaqifiStreamingDevice.ContainsScpiError matches both **ERROR and ERROR Desktop changes: - Bump Daqifi.Core 0.15.1 → 0.18.1 across all three projects - Bump System.IO.Ports 10.0.2 → 10.0.3 (new Core transitive requirement) - AbstractStreamingDevice.TimestampFrequency now delegates to the underlying Core device instead of re-extracting from protobuf - Remove ContainsScpiErrorEntry and IsScpiErrorLine helpers — Core's SdCardFileListParser now filters these before returning file lists - Remove the Desktop-level SCPI error retry in RefreshSdCardFiles — Core already retries internally in GetSdCardFilesAsync - Simplify IsImportableSdLogFileName — drop redundant IsScpiErrorLine and control-character guards, keep only the .bin extension filter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflicts by taking newer package versions from main (Daqifi.Core 0.19.1, System.IO.Ports 10.0.5, Google.Protobuf 3.34.0) and preserving both CoreDeviceForStreaming (SD card feature) and CoreDeviceForNetworkConfiguration (main) in AbstractStreamingDevice, SerialStreamingDevice, and DaqifiStreamingDevice. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix CS1503: convert uint channelMask to binary string for StartSdCardLoggingAsync which expects string? - Fix CS0246: add missing using for SdCardSessionImporter/ImportProgress in DaqifiViewModel - Remove dead BuildActiveAnalogChannelMask() (superseded by inline uint mask in StartSdCardLogging) - EnsureSdOperationsQuiesced now throws instead of silently stopping streaming, preventing unexpected data loss Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test device only overrode CoreDeviceForNetworkConfiguration, so StopStreaming() threw when UpdateNetworkConfiguration called it with IsStreaming=true. Expose the same RecordingCoreStreamingDevice for both properties. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
StopStreaming now delegates to the Core layer, so the stop command is recorded as core:SYSTem:StopStreamData rather than desktop:... Update the assertion to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Core layer's StopStreaming() is a no-op when its own IsStreaming is false. The test only set the Desktop-level flag, so the stop command was never sent. Call StartStreaming() on the Core device (and clear recorded commands) so that StopStreaming() flows through correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/agentic_review |
Code Review by Qodo
1.
|
| using var context = _loggingContext.CreateDbContext(); | ||
|
|
||
| var sessionName = options.SessionNameOverride | ||
| ?? $"SD Import - {Path.GetFileNameWithoutExtension(logSession.FileName)}"; | ||
|
|
||
| // Check for existing session with same name | ||
| if (options.OverwriteExistingSession) | ||
| { | ||
| var existing = context.Sessions.FirstOrDefault(s => s.Name == sessionName); | ||
| if (existing != null) | ||
| { | ||
| context.Sessions.Remove(existing); | ||
| context.SaveChanges(); | ||
| } | ||
| } | ||
|
|
||
| // Generate new session ID (same pattern as LoggingManager.OnActiveChanged) | ||
| var ids = context.Sessions.AsNoTracking().Select(s => s.ID).ToList(); | ||
| var newId = ids.Count > 0 ? ids.Max() + 1 : 0; | ||
|
|
||
| var session = new LoggingSession(newId, sessionName) | ||
| { | ||
| SessionStart = logSession.FileCreatedDate ?? DateTime.Now | ||
| }; | ||
|
|
||
| context.Sessions.Add(session); | ||
| context.SaveChanges(); | ||
|
|
||
| return session; | ||
| } | ||
|
|
||
| private async Task FlushBatchAsync(List<DataSample> batch, CancellationToken ct) | ||
| { | ||
| ct.ThrowIfCancellationRequested(); | ||
|
|
||
| using var context = _loggingContext.CreateDbContext(); | ||
| using var transaction = context.Database.BeginTransaction(); | ||
| context.BulkInsert(batch); | ||
| transaction.Commit(); | ||
| } |
There was a problem hiding this comment.
2. Importer uses sync ef 📘 Rule violation ➹ Performance
The importer performs synchronous Entity Framework operations (SaveChanges, queries, and transactions) instead of async equivalents. This can block threads and reduce UI responsiveness during large imports.
Agent Prompt
## Issue description
The new SD card import path performs synchronous EF Core operations (queries, SaveChanges, transactions/bulk insert), which violates the async database operation standard.
## Issue Context
Imports can process large datasets; blocking DB operations can freeze UI or reduce responsiveness.
## Fix Focus Areas
- Daqifi.Desktop/Loggers/SdCardSessionImporter.cs[282-313]
- Daqifi.Desktop/Loggers/SdCardSessionImporter.cs[315-323]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public async Task<LoggingSession> ImportFromFileAsync( | ||
| string filePath, | ||
| ImportOptions? options = null, | ||
| IProgress<ImportProgress>? progress = null, | ||
| CancellationToken ct = default) | ||
| { | ||
| _logger.Information($"Starting file import from '{Path.GetFileName(filePath)}'"); | ||
| var parser = new SdCardFileParser(); | ||
| var logSession = await parser.ParseFileAsync(filePath, null, ct); | ||
| return await ImportSessionAsync(logSession, options, progress, ct); |
There was a problem hiding this comment.
3. Import inputs not validated 📘 Rule violation ⛨ Security
User-provided inputs (filePath and fileName) are consumed without validation (existence/format/whitespace/extension checks). This can cause crashes or unintended file access and violates the input validation requirements.
Agent Prompt
## Issue description
Importer boundary methods accept user/device-provided file identifiers and consume them without validation or normalization.
## Issue Context
Per compliance rules, all user inputs must be validated and normalized (e.g., trim, reject whitespace-only, check existence/format) before being used for I/O.
## Fix Focus Areas
- Daqifi.Desktop/Loggers/SdCardSessionImporter.cs[28-54]
- Daqifi.Desktop/Loggers/SdCardSessionImporter.cs[59-85]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // The Core package resumes StartSdCardLoggingAsync continuations on the caller's | ||
| // synchronization context. Running it on the thread pool prevents UI deadlocks. | ||
| Task.Run(() => coreDevice.StartSdCardLoggingAsync(channelMask: analogChannelMask)).GetAwaiter().GetResult(); | ||
| var channelMaskString = Convert.ToString((long)analogChannelMask, 2); | ||
| Task.Run(() => coreDevice.StartSdCardLoggingAsync(channelMask: channelMaskString)).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
5. getresult() blocks ui 📘 Rule violation ➹ Performance
StartSdCardLogging performs a blocking wait using GetAwaiter().GetResult(), which can block the UI thread and risks deadlocks. This violates the async/await and UI responsiveness requirement.
Agent Prompt
## Issue description
A blocking wait (`GetAwaiter().GetResult()`) is used around async SD card logging start, which can block the UI thread.
## Issue Context
Compliance requires async/await for I/O and forbids blocking waits on UI-sensitive code paths.
## Fix Focus Areas
- Daqifi.Desktop/Device/AbstractStreamingDevice.cs[472-475]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (analogChannelMask != 0) | ||
| { | ||
| SendMessage(ScpiMessageProducer.EnableAdcChannels( | ||
| analogChannelMask.ToString(CultureInfo.InvariantCulture))); | ||
| } |
There was a problem hiding this comment.
6. Adc mask mis-sent 🐞 Bug ✓ Correctness
StartSdCardLogging sends EnableAdcChannels using a decimal string via the desktop transport, even though SD logging configuration is expected to be owned by Core using a combined binary mask; this can momentarily apply an incorrect channel mask or cause redundant/misordered channel configuration.
Agent Prompt
### Issue description
`StartSdCardLogging()` currently sends `EnableAdcChannels` from the desktop layer using a decimal-encoded mask, while Core SD logging is expected to own channel-mask configuration using a combined binary mask. This duplication/mismatch can apply the wrong channel mask or introduce ordering issues.
### Issue Context
Unit tests indicate the desired behavior: Core should receive a single combined binary mask for SD logging channels.
### Fix Focus Areas
- Daqifi.Desktop/Device/AbstractStreamingDevice.cs[463-476]
- Daqifi.Desktop.Test/Device/AbstractStreamingDeviceTests.cs[565-590]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // Create the logging session in the database | ||
| var session = CreateSession(logSession, options); | ||
|
|
||
| if ((config?.TimestampFrequency ?? 0) == 0) | ||
| { | ||
| _logger.Warning( | ||
| "No TimestampFrequency found in SD card file. " + | ||
| "Timestamps may not be properly reconstructed. " + | ||
| "Device firmware may not include TimestampFreq in logged messages."); | ||
| } | ||
|
|
||
| // Bulk-insert samples | ||
| var batch = new List<DataSample>(); | ||
| long samplesProcessed = 0; | ||
| var sampleIndex = 0; | ||
|
|
||
| await foreach (var entry in logSession.Samples.WithCancellation(ct)) | ||
| { | ||
| // Log first and second sample for diagnostics (to verify timestamps are spaced correctly) | ||
| if (sampleIndex < 2) | ||
| { | ||
| _logger.Information( | ||
| $"Sample[{sampleIndex}]: AnalogValues.Count={entry.AnalogValues.Count}, " + | ||
| $"DigitalData=0x{entry.DigitalData:X8}, Timestamp={entry.Timestamp:O}"); | ||
| } | ||
|
|
||
| sampleIndex++; | ||
|
|
||
| // If we didn't have config, discover channel count from first entry | ||
| if (analogPortCount == 0 && entry.AnalogValues.Count > 0) | ||
| { | ||
| analogPortCount = entry.AnalogValues.Count; | ||
| _logger.Information($"Discovered {analogPortCount} analog channels from first sample"); | ||
| AssignChannelColors(channelColors, analogPortCount, digitalPortCount); | ||
| } | ||
|
|
||
| // Create analog samples | ||
| for (var i = 0; i < entry.AnalogValues.Count; i++) | ||
| { | ||
| var channelName = $"AI{i}"; | ||
| batch.Add(new DataSample | ||
| { | ||
| LoggingSessionID = session.ID, | ||
| ChannelName = channelName, | ||
| DeviceName = deviceName, | ||
| DeviceSerialNo = deviceSerialNo, | ||
| Color = channelColors.GetValueOrDefault(channelName, "#D32F2F"), | ||
| Type = ChannelType.Analog, | ||
| Value = entry.AnalogValues[i], | ||
| TimestampTicks = entry.Timestamp.Ticks | ||
| }); | ||
| } | ||
|
|
||
| // Create digital samples (one per bit) | ||
| for (var i = 0; i < digitalPortCount; i++) | ||
| { | ||
| var channelName = $"DI{i}"; | ||
| var bitValue = (entry.DigitalData & (1u << i)) != 0 ? 1.0 : 0.0; | ||
| batch.Add(new DataSample | ||
| { | ||
| LoggingSessionID = session.ID, | ||
| ChannelName = channelName, | ||
| DeviceName = deviceName, | ||
| DeviceSerialNo = deviceSerialNo, | ||
| Color = channelColors.GetValueOrDefault(channelName, "#757575"), | ||
| Type = ChannelType.Digital, | ||
| Value = bitValue, | ||
| TimestampTicks = entry.Timestamp.Ticks | ||
| }); | ||
| } | ||
|
|
||
| // Flush batch when full | ||
| if (batch.Count >= BatchSize) | ||
| { | ||
| await FlushBatchAsync(batch, ct); | ||
| samplesProcessed += batch.Count; | ||
| batch.Clear(); | ||
| progress?.Report(new ImportProgress(samplesProcessed, null)); | ||
| } | ||
| } | ||
|
|
||
| // Flush remaining samples | ||
| if (batch.Count > 0) | ||
| { | ||
| await FlushBatchAsync(batch, ct); | ||
| samplesProcessed += batch.Count; | ||
| batch.Clear(); | ||
| progress?.Report(new ImportProgress(samplesProcessed, null)); | ||
| } | ||
|
|
||
| if (samplesProcessed == 0) | ||
| { | ||
| _logger.Warning( | ||
| $"No samples found in SD card file '{logSession.FileName}'. " + | ||
| $"DeviceConfig present: {config != null}"); | ||
| } | ||
|
|
||
| _logger.Information($"Imported {samplesProcessed} samples for session '{session.Name}' (ID={session.ID})"); | ||
| return session; | ||
| } | ||
|
|
||
| private LoggingSession CreateSession(SdCardLogSession logSession, ImportOptions options) | ||
| { | ||
| using var context = _loggingContext.CreateDbContext(); | ||
|
|
||
| var sessionName = options.SessionNameOverride | ||
| ?? $"SD Import - {Path.GetFileNameWithoutExtension(logSession.FileName)}"; | ||
|
|
||
| // Check for existing session with same name | ||
| if (options.OverwriteExistingSession) | ||
| { | ||
| var existing = context.Sessions.FirstOrDefault(s => s.Name == sessionName); | ||
| if (existing != null) | ||
| { | ||
| context.Sessions.Remove(existing); | ||
| context.SaveChanges(); | ||
| } | ||
| } | ||
|
|
||
| // Generate new session ID (same pattern as LoggingManager.OnActiveChanged) | ||
| var ids = context.Sessions.AsNoTracking().Select(s => s.ID).ToList(); | ||
| var newId = ids.Count > 0 ? ids.Max() + 1 : 0; | ||
|
|
||
| var session = new LoggingSession(newId, sessionName) | ||
| { | ||
| SessionStart = logSession.FileCreatedDate ?? DateTime.Now | ||
| }; | ||
|
|
||
| context.Sessions.Add(session); | ||
| context.SaveChanges(); | ||
|
|
||
| return session; | ||
| } | ||
|
|
||
| private async Task FlushBatchAsync(List<DataSample> batch, CancellationToken ct) | ||
| { | ||
| ct.ThrowIfCancellationRequested(); | ||
|
|
||
| using var context = _loggingContext.CreateDbContext(); | ||
| using var transaction = context.Database.BeginTransaction(); | ||
| context.BulkInsert(batch); | ||
| transaction.Commit(); | ||
| } |
There was a problem hiding this comment.
7. Partial import persisted 🐞 Bug ⛯ Reliability
SdCardSessionImporter persists a new LoggingSession before inserting samples and flushes samples in separate transactions without rollback/cleanup, so exceptions or cancellations can leave empty/partial sessions and partial sample sets in SQLite.
Agent Prompt
### Issue description
SD card import can leave the database in a partially-imported state (session row created, some sample batches committed) when parsing/insertion fails or is cancelled.
### Issue Context
`CreateSession()` saves immediately, and each batch insert commits independently with no top-level transaction or compensating cleanup.
### Fix Focus Areas
- Daqifi.Desktop/Loggers/SdCardSessionImporter.cs[156-323]
- Daqifi.Desktop/ViewModels/DeviceLogsViewModel.cs[182-295]
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1188-1240]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
… in cleanup log - Add XML documentation to RefreshSdCardFiles, UpdateSdCardFiles, DownloadSdCardFileAsync, and DeleteSdCardFileAsync - Log full exception (not just Message) when temp file cleanup fails, preserving the stack trace for diagnostics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Always pull Daqifi.Core from NuGet; remove the DaqifiCoreProjectPath conditional switch and local project reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tage scaling - Update Daqifi.Core to v0.19.2 (fixes raw ADC scaling, timestamp reconstruction, and ConfigurationOverride merge behavior) - SdCardSessionImporter.ImportFromDeviceAsync now builds a ConfigurationOverride from the connected device's channel data (calibration, resolution, port range, internal scale) so the parser can convert raw ADC values to real voltages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 18.8%
Daqifi.Desktop.Common - 39.3%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
User description
Summary
SdCardSessionImporterservice that parses SD card.binlog files (via core library'sSdCardFileParser) and bulk-inserts them into the desktop app's SQLite database asLoggingSession+DataSampleentitiesDownloadSdCardFileAsyncandDeleteSdCardFileAsyncto the desktopIStreamingDeviceinterfaceDaqifiCoreProjectPathMSBuild property (needed until core fix: updated menu name with correct capitalizaiton #110/chore: Update build workflow to build MSI installer #111 are published to NuGet)Test plan
.binfile via "Import .bin File" button → session appears in session list → viewable in plot → exportable to CSVdotnet build -p:DaqifiCoreProjectPath="path/to/Daqifi.Core.csproj"Prerequisites
Closes #374
🤖 Generated with Claude Code
PR Type
Enhancement
Description
Add
SdCardSessionImporterservice to parse and bulk-import SD card.binlog files into SQLite databaseImplement three import paths: local file, stream, and USB device download with progress tracking
Add "Import .bin File" button in Application Logs tab for local file imports
Add per-file "Import" and "Import All" buttons in Device Logs tab for USB device imports
Extend
IStreamingDeviceinterface withDownloadSdCardFileAsyncandDeleteSdCardFileAsyncmethodsSupport conditional local core project reference via
DaqifiCoreProjectPathMSBuild propertyDiagram Walkthrough
File Walkthrough
SdCardSessionImporter.cs
Core SD card import service with batch processingDaqifi.Desktop/Loggers/SdCardSessionImporter.cs
ImportFromFileAsync,ImportFromStreamAsync, andImportFromDeviceAsyncSdCardLogEntryobjects to desktopDataSampleentities withper-channel color assignment
samples) for performance
ImportOptionsfor session name override, overwrite behavior,and post-import device file deletion
ImportProgressclass for progress reporting during importoperations
IStreamingDevice.cs
Extend interface with SD card operationsDaqifi.Desktop/Device/IStreamingDevice.cs
DownloadSdCardFileAsyncmethod to download files from device SDcard over USB with progress tracking
DeleteSdCardFileAsyncmethod to delete files from device SD cardAbstractStreamingDevice.cs
Implement SD card operations in device classDaqifi.Desktop/Device/AbstractStreamingDevice.cs
Daqifi.Core.Device.SdCardnamespaceDownloadSdCardFileAsyncmethod delegating to core deviceDeleteSdCardFileAsyncmethod delegating to core deviceGetCoreDeviceForSd()helper for validationDaqifiViewModel.cs
Add local file import command to main view modelDaqifi.Desktop/ViewModels/DaqifiViewModel.cs
ImportSdCardLogFilerelay command for importing local.binfilesvia file dialog
LoggingManagerandshowing success dialog
cancellation support
DeviceLogsViewModel.cs
Add device file import commands with batch supportDaqifi.Desktop/ViewModels/DeviceLogsViewModel.cs
ImportFilerelay command for importing individual files fromconnected USB device
ImportAllFilesrelay command for batch importing all device fileswith per-file error handling
LoggingManagerMainWindow.xaml
Add import button to application logs UIDaqifi.Desktop/MainWindow.xaml
import icon and label
export/delete buttons
ImportSdCardLogFileCommandfrom view modelDeviceLogsView.xaml
Add import buttons to device logs UIDaqifi.Desktop/View/DeviceLogsView.xaml
positioned right of refresh button
Daqifi.Desktop.csproj
Support conditional local core project referenceDaqifi.Desktop/Daqifi.Desktop.csproj
Daqifi.CoreNuGet package reference conditional onDaqifiCoreProjectPathbeing emptyProjectReferenceto local core project whenDaqifiCoreProjectPathMSBuild property is setproject file