feat(live-graph): show 'Logging to Device' status while SD-card logging#507
Open
tylerkron wants to merge 2 commits into
Open
feat(live-graph): show 'Logging to Device' status while SD-card logging#507tylerkron wants to merge 2 commits into
tylerkron wants to merge 2 commits into
Conversation
The Live Graph showed an empty plot and "LOGGING OFF" while the device was actively recording to its SD card, because samples are written on-device in that mode and never reach the desktop. Replaces the empty plot with a status panel (recording icon, elapsed clock, format) and makes the logging toggle reflect device-reported state instead of only the local toggle flag — so a desktop reconnect to a device that's still SD-logging now displays the real state. - IsLogging getter combines the toggle flag with each connected device's IsLoggingToSdCard, subscribing via PropertyChanged so the UI follows device state without polling. - Subscription tracking uses a HashSet diff against ConnectedDevices so Clear/Reset events don't leak handlers when the device list refreshes. - Adds IsSdCardLoggingActive + a 1Hz DispatcherTimer-driven SdLoggingElapsed, formatted off TotalHours so multi-day sessions don't wrap at 24h. - LiveGraphPane hides the OxyPlot and "no channels streaming" empty state when SD-logging is active, swaps the RATE chip for ELAPSED, and shows a centered status overlay matching the existing in-pane card style. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
Review Summary by QodoShow "Logging to Device" status while SD-card logging active
WalkthroughsDescription• Live Graph now displays "Logging to Device" status panel when SD-card logging is active, replacing the empty plot • IsLogging property reflects device-reported SD-card logging state via PropertyChanged subscriptions, not just local toggle • Header chips swap RATE (Hz) for ELAPSED (HH:mm:ss) when SD-card logging is active, with elapsed time formatted for multi-day sessions • Added comprehensive unit tests for logging state tracking, device subscription lifecycle, and property change notifications Diagramflowchart LR
A["Device SD-Card<br/>Logging State"] -->|PropertyChanged| B["OnDeviceLoggingStateChanged"]
B -->|Updates| C["IsSdCardLoggingActive"]
C -->|Drives| D["Live Graph<br/>Status Panel"]
C -->|Drives| E["ELAPSED Chip<br/>in Header"]
F["DispatcherTimer<br/>1Hz Tick"] -->|Updates| G["SdLoggingElapsed<br/>HH:mm:ss"]
G -->|Binds to| D
H["ConnectedDevices<br/>CollectionChanged"] -->|Diff via HashSet| I["Subscribe/<br/>Unsubscribe<br/>PropertyChanged"]
I -->|Prevents| J["Handler Leaks<br/>on Clear/Reset"]
File Changes1. Daqifi.Desktop.Test/ViewModels/DaqifiViewModelLoggingStateTests.cs
|
Contributor
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
- Convert SdLoggingElapsed to [ObservableProperty] for consistency with the rest of the view model (CommunityToolkit MVVM source generators). Removes the manual property + inline-brace style violation. - Block SelectedStreamingFrequency changes whenever IsLogging is true, not only when LoggingManager.Active is — so the guard also fires when a device is reporting SD logging that the local toggle never started. - Clarify the IsLogging XML comment: streaming state isn't tracked via PropertyChanged because IsStreaming isn't on the interface. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
Author
|
@qodo-code-review re: advisory #4 (misleading |
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 18%
Daqifi.Desktop.Common - 30.8%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Live Graph showed an empty plot and a "LOGGING OFF" label while the device was actively recording to its SD card, because samples in that mode are written on-device and never stream to the desktop. This PR makes the pane reflect what's actually happening.
IsLoggingnow reflects device-reported state, not just the local toggle flag — so a desktop reconnect to a device that's still SD-logging shows "LOGGING ON" instead of misleadingly showing "LOGGING OFF". The view model subscribes to each device'sPropertyChangedforIsLoggingToSdCardand theIsLogginggetter combines the toggle flag with that signal.Implementation notes
ConnectedDevices.CollectionChangedhandler diffs against a trackedHashSet<IStreamingDevice>instead of usinge.OldItems/e.NewItemsdirectly. This is required becauseObservableCollection<T>.Clear()raises aResetevent withOldItems == null—UpdateUicallsClear()on every device-list refresh, so a naive subscribe/unsubscribe handler would leak handlers.SdLoggingElapsedformats offTimeSpan.TotalHours, not theHourscomponent, so multi-day sessions don't wrap at 24h.PropertyChangedbecauseIsStreamingisn't on theIStreamingDeviceinterface; the streaming path already updates state synchronously through the toggle setter, so this isn't a regression.Known gap (out of scope)
When the desktop reconnects to a USB device that's already SD-logging, nothing currently queries the device's actual state —
IsLoggingToSdCardstaysfalseuntil something callsStartSdCardLogging()locally. The infrastructure to react is now in place; closing this loop likely needs a Core/firmware-level state query and is a separate task.Test plan
Daqifi.Desktop.Test/ViewModels/DaqifiViewModelLoggingStateTests.cs.🤖 Generated with Claude Code