feat: Live Graph redesign + streaming regression fix#495
Merged
Conversation
Extracts the Live Graph tab into a dedicated LiveGraphPane UserControl (matching the Channels/Devices/LoggedData pane pattern) and applies the dark visual system throughout. - Dark Surface background replaces the default light MahApps theme - Session status strip shows MODE / DURATION / RATE chips inline with a green LIVE pulse indicator when logging; hidden when idle - Logging toggle label now uses TextTertiary/StatusGreen instead of the default MahApps white pill - Channel legend sidebar uses SurfaceRaised + left BorderDim divider, a CHANNELS header, and a narrow color stripe (matching channel tile language) instead of a plain rectangle swatch - Plot toolbar replaced FadeButton (white background, scale-on-hover) with PlotIconButton inside a SurfaceRaised bordered group at the top-right of the plot; zoom-X, zoom-Y, reset, save, and settings are all in one row with dim separators - LiveGraphFlyout.xaml restyled with a dark #171A20 background, section labels, and row-label/toggle layout matching the Devices drawer pattern - Removes the debug (OpenLogSummaryCommand) button from the toolbar; the debug button in the title bar serves that purpose Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Review Summary by QodoRedesign Live Graph pane with dark theme and unified component pattern
WalkthroughsDescription• Extracts Live Graph tab into dedicated LiveGraphPane UserControl matching established pane pattern • Applies dark visual system with Surface background, design tokens, and consistent typography • Redesigns session status header with inline MODE/DURATION/RATE chips and green LIVE indicator • Replaces scattered toolbar buttons with unified SurfaceRaised pill containing icon buttons with dim dividers • Refactors channel legend sidebar with SurfaceRaised background, left border, and narrow color stripes • Restyled LiveGraphFlyout with dark background, section labels, and grid-line/precision controls Diagramflowchart LR
A["MainWindow TabItem"] -->|"ContentPresenter"| B["LiveGraphPane UserControl"]
B --> C["Header Bar"]
B --> D["Plot Area"]
B --> E["Channel Sidebar"]
C --> C1["Session Status Chips<br/>MODE/DURATION/RATE"]
C --> C2["Logging Toggle<br/>with Label"]
D --> D1["OxyPlot"]
D --> D2["Plot Toolbar Pill<br/>Zoom/Reset/Save/Settings"]
E --> E1["CHANNELS Header"]
E --> E2["Channel List<br/>with Color Stripes"]
F["LiveGraphFlyout"] --> F1["Grid Lines Section"]
F --> F2["Precision Slider<br/>with Consolas Display"]
File Changes1. Daqifi.Desktop/View/Prototype/LiveGraphPane.xaml.cs
|
Contributor
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Cleanup pass after design-philosophy review: - Drop dead SessionDuration/StorageUsage chip bindings (properties don't exist) - Move plot toolbar pill into the header bar so it no longer overlaps the right Y-axis labels - Restore the log-summary debug button (Bug icon) inside the toolbar pill - Replace hardcoded hex in LiveGraphFlyout with DesignTokens DynamicResource references so the flyout follows the dark theme tokens Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses Qodo rule 244813 (XML docs required on public API members). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Live Graph empty states (no channels streaming): - Centered overlay on the plot area with chart icon and CTA pointing users to the Channels tab - Inline hint in the Channels sidebar so the panel isn't a blank column Window chrome: - Override MahApps Light.Blue title bar with Surface/TextPrimary tokens so the title bar matches the dark panes instead of glowing blue - Swap the hardcoded "Red" notification badge to the StatusRed token Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the user has connected a device and enabled channels but hasn't flipped the LOGGING toggle yet, the plot was just an empty grid with no signal that toggling logging is what makes data appear. Adds a second overlay (green play icon, "Channels ready · Toggle LOGGING ON to start streaming") triggered when IsLogging is false and at least one input channel is active. Backed by a new HasActiveInputChannels derived property on DaqifiViewModel since MultiDataTrigger can only do equality checks and a "Count > 0" condition needs a bool. Mirrors the existing HasLoggingSessions pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The default MahApps ToggleSwitch off-state had near-zero contrast against the dark plot card, making it hard for users to see whether logging was actually toggled. Swap in a custom ToggleButton template: a dim track with a gray thumb when off, StatusGreen with a white thumb when on. The on state now matches the LIVE pulse and "LOGGING ON" label color, so the active state reads at a glance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Toggling logging on, off (with no samples), then on again could throw 'UNIQUE constraint failed: SessionDeviceMetadata.LoggingSessionID, SessionDeviceMetadata.DeviceSerialNo'. The new session ID was computed as max(Sessions.ID) + 1, but if SessionDeviceMetadata or Samples held orphan rows from a prior crash (or a delete that ran without SQLite foreign keys enabled), the chosen ID could already be referenced — and the composite PK on SessionDeviceMetadata rejected the insert. Compute the next ID as max(Sessions, SessionDeviceMetadata, Samples) + 1 so it can't collide with any leftover reference, and sweep orphan metadata at startup so the database self-heals on the next launch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Picks up the protobuf parser fix from daqifi-core#169 — the parser now resyncs past leading garbage instead of stalling after 3 retries, so the desktop Live Graph receives stream messages even when boot-time junk lands at the head of the consumer buffer on serial connect. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The "Channels ready / Toggle LOGGING ON" overlay covered the plot whenever logging was off and channels were active — including right after a session ended, hiding the data the user just captured. The LOGGING toggle is already prominent in the toolbar, so the prompt is unnecessary. The "No channels streaming" overlay (shown when no channels are active) is preserved. Also removes the now-unused HasActiveInputChannels property and its PropertyChanged firing, which only existed for this overlay's binding. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tylerkron
force-pushed
the
claude/sweet-gates-10c324
branch
from
a92062e to
e355560
Compare
Diagnostic-only. Adds counter-based logs at every junction in the
stream data path so one run can localize whether messages are
arriving from Core, where they get gated, and whether samples reach
the plot. Also logs BuildDeviceMetadataForSession output so the EF
duplicate-key error on session restart can be attributed to a
specific serial.
Touchpoints:
- Serial/WiFi OnCoreMessageReceived: raw count from Core
- OnStreamMessageReceived: received / gated / processed counts
- InitializeStreaming: end-of-start snapshot of IsStreaming, Mode,
active analog/digital channel counts
- LoggingManager.OnActiveChanged(true): subscribed channels grouped
by serial, connected-device serials
- BuildDeviceMetadataForSession: returned rows + in-loop duplicate
detection to pinpoint the EF tracker conflict
- HandleDeviceMessage / HandleChannelUpdate: counts + gate state
- PlotLogger.Log: samples reaching the plot
All logs are Interlocked-counted and emit at count 1, 10, and every
100 after — cheap in the hot path, informative at session boundaries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls in the oversized-prefix parser stall fix (daqifi-core#173) on top of the v0.19.6 leading-garbage recovery. Addresses the streaming regression where boot-time serial garbage that happens to varint-encode a large length left the parser waiting indefinitely — device LED blinking, buffer growing, zero MessageReceived events fired. [STREAM_DIAG] instrumentation from b507a97 stays in place until this lands on a physical device and we confirm frames flow end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit b507a97.
tylerkron
changed the title
Contributor
Author
|
/agentic_review |
Contributor
Code Review by QodoLooking for bugs?Check back in a few minutes. An AI review agent is analyzing this pull request.
|
Contributor
Author
|
/agentic_review |
Contributor
|
Persistent review updated to latest commit 3cb00c1 |
…e border on hover Two findings from Qodo review on PR #495: 1. UI blocked by MAX queries (Bug, Performance). The session-ID collision-safe lookup ran three round-trips on the UI thread (one per related table) every time logging started. Folded into a single SQL using UNION ALL so the round-trip count drops to one. Each inner MAX hits an index — Sessions.ID is the PK, the SessionDeviceMetadata composite PK leads with LoggingSessionID, and IX_Samples_SessionTime covers Samples.LoggingSessionID — so SQLite resolves each inner MAX as an index seek (verified via EXPLAIN QUERY PLAN: "SEARCH Samples USING COVERING INDEX"). Behavior is preserved: empty tables yield -1+1=0, orphans in any table push the next ID safely past them. 2. Hover overrides checked border (Bug, Maintainability). The LoggingToggle template had a bare IsMouseOver=true Trigger setting Track.BorderBrush, which overrode the IsChecked=true setter when both held — swapping the green "logging on" outline for TextSecondary while hovered. Replaced with a MultiTrigger gated on IsChecked=False so hover styling only applies in the unchecked state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Triaged the latest Qodo review (3 new findings on top of the streaming-fix commits). Resolved in 96e91a6: 4. UI blocked by MAX queries (Bug, Performance — agreed and fixed) SELECT MAX(id) AS Value FROM (
SELECT MAX(ID) AS id FROM Sessions
UNION ALL SELECT MAX(LoggingSessionID) FROM SessionDeviceMetadata
UNION ALL SELECT MAX(LoggingSessionID) FROM Samples
)Each inner MAX is an index seek — 5. Hover overrides checked border (Bug, Maintainability — agreed and fixed) 3. The principled fix is making the whole load path async ( (The two earlier findings — hardcoded hex colors in the flyout and missing XML doc on |
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 17.6%
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
This branch grew to cover two related-but-separable changes that both affect the streaming experience: a Live Graph pane redesign that matches the rest of the app's dark theme, and a streaming regression fix where no data appeared on the graph despite the device LED indicating active streaming.
Streaming regression fix
USB-Serial streaming would silently produce no
MessageReceivedevents on Windows. Device LED blinked, byte buffer in the Core consumer grew unbounded (66 → 8202 bytes in 2s observed), Live Graph stayed empty.Root cause was in
daqifi-coreProtobufMessageParser: boot-time garbage on the serial port (USB CDC handshake, DTR pulse artifacts) occasionally varint-encoded a length value in the tens of KB. Under the old 1 MBMaxMessageSizeBytescap the parser accepted that declared length, took the "wait for more data" branch, and stayed there indefinitely — buffer kept growing, no frames ever parsed.This PR pulls in that fix via two dependency bumps:
chore(deps): bump Daqifi.Core to 0.19.6— first parser-resync fix (recovery from unparseable bytes via 1-byte advance).chore(deps): bump Daqifi.Core to 0.19.7— oversized-prefix fix: 4 KB cap on declared frame size, gap gate, and a protobuf field-tag plausibility check on the first body byte. Real partial frames spanning reads are preserved; bogus prefixes are rejected and resync continues.Also addresses a downstream UNIQUE-constraint failure that surfaced when re-toggling streaming after a session-with-no-samples:
fix(logging): UNIQUE constraint when restarting a stream session— Sessions.ID was computed asmax(Sessions) + 1, which could collide with orphan rows inSessionDeviceMetadata/Samplesleft behind by prior crashes or pre-foreign-keys deletes. Now picksmaxacross all three related tables and sweeps orphan metadata at startup.Live Graph pane redesign
Extracts the Live Graph tab into a dedicated
LiveGraphPaneUserControl inView/Prototype/, matching the established pattern used by Channels, Devices, and Logged Data panes. Applies the dark visual system throughout —Surfacebackground, design token colors, same typography and spacing language as the exemplar Channels pane.LiveGraphPane.xaml (new)
Surfacebackground fills the entire pane — no more white bleed from the MahApps Light.Blue themeMODE / DURATION / RATEchip strip (collapsed when idle), logging toggle label rendered withTextTertiary/StatusGreenDataTrigger instead of the default MahApps pillSurfaceRaised-bordered pill containingPlotIconButtonicon buttons (zoom-X, zoom-Y, reset, save, settings) separated byBorderDimdividersSurfaceRaisedbackground, leftBorderDimborder,CHANNELSsection header, compact rows with a 3px color stripe (vs. the previous 16×16 square),TextPrimarychannel name,TextTertiarydevice serialLiveGraphFlyout.xaml (updated)
Background="#171A20", dark grid lines section withDrawerSectionLabel/DrawerRowLabelstyles, precision section withConsolasvalue readoutMainWindow.xaml (updated)
TabItembody replaced with<prototype:LiveGraphPane/>viaContentPresenter, cutting ~250 lines from MainWindowTest plan
Streaming regression
HandleChannelUpdateandPlotLogger.Logcounts climb in lockstep with arriving frames)daqifi-core-example-app --mimic-desktop— no regression on the happy pathRecoversFromOversizedPrefixandPreservesPartialFrameAfterGarbageregressions)Live Graph redesign
SurfaceRaisedwith left borderTextTertiary🤖 Generated with Claude Code