fix: pace TX waterfall rows to line_duration to match RX scroll rate#3686
Conversation
There was a problem hiding this comment.
Thanks for this, @mvanhorn — clean, well-targeted fix with a clear write-up. The approach is correct: gating the TX pushWaterfallRow behind the same m_nextFallbackWaterfallRowMs / waterfallFallbackIntervalMs() due-time logic that pushRxWaterfallFallbackIfDue() already uses makes both paths follow the rate slider, which is exactly what #3641 needs. Reusing the single member is safe since TX and RX are mutually exclusive, and seeding m_nextFallbackWaterfallRowMs = 0 on the RX→TX edge in setTransmitting() correctly forces the first TX row to render immediately. The comment update at pushWaterfallRow keeps the cadence-ownership contract accurate.
All six CI checks (build, check-macos, check-windows, CodeQL, analyze, Qt a11y) are green.
One minor, non-blocking observation — the reverse edge (TX→RX) isn't symmetrically reset. When TX ends, m_nextFallbackWaterfallRowMs is left at a future value (now + interval from the last TX push). On resume, updateNativeWaterfallFallbackState() only re-seeds it to now when it's <= 0, so in the !m_waterfallFallbackActive path the leftover future value is kept. In practice this is masked by the ~400 ms post-TX blanking window (#2117) and is bounded by one row interval, so at typical rates the leftover is already in the past and the first RX row pushes immediately — only at very slow rates could the first RX fallback row be delayed by up to (interval − blanking). Worth a glance to confirm that's acceptable, but I don't see it as a correctness problem.
Nice work — behavior matches the issue and the #3019 "no fixed cap" guarantee is preserved.
🤖 aethersdr-agent · cost: $1.0635 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Approving — clean, correct, symmetric fix. The TX branch now gates pushWaterfallRow behind the same due-time mechanism RX uses (now >= m_nextFallbackWaterfallRowMs, advance by waterfallFallbackIntervalMs()), so both scroll at the rate-slider cadence; the setTransmitting reset makes the first TX row immediate. No fixed cap reintroduced (#3019 intact), Principle XIII honored. Verified by inspection (TX path mirrors RX); CI green.
Optional non-blocking follow-up: it resets the timer on RX→TX but not TX→RX, so the first RX row after TX can be up to one interval late — resetting on TX→RX too would make it immediate (and may soften the post-TX waterfall gap). Nice work @mvanhorn.
|
Thank you @ten9876. Pacing the TX waterfall to line_duration so it matches the RX scroll rate is a satisfying consistency fix. |
Summary
Fixes #3641. On TX the waterfall scrolled at a different cadence than on RX because the two paths used different pacing sources. The TX branch in
updateSpectrum()pushed one waterfall row per FFT frame (tracking the FFT FPS slider), while RX paces FFT-derived rows toline_durationthroughm_nextFallbackWaterfallRowMsandwaterfallFallbackIntervalMs()(tracking the waterfall rate slider). This change gates the TXpushWaterfallRowcall behind the same due-time mechanism RX already uses, so TX and RX scroll at the same rate-slider-driven cadence.m_nextFallbackWaterfallRowMsis reset on the RX to TX transition insetTransmitting()so the first TX row renders immediately. No fixed rows/sec cap is reintroduced; cadence still follows the live rate slider, keeping the #3019 guarantee intact.Constitution principle honored
Principle XIII (The Operator Outranks Every Agent): the visible scroll rate now follows the operator's rate slider on both TX and RX, instead of silently diverging based on which slider was last touched.
Test plan
cmake --build build)Reproduction: set the FFT FPS slider and the waterfall rate slider to non-aligned values, then transmit. Before this change the TX waterfall scrolls at the FFT FPS rate while RX scrolls at the rate-slider rate; after this change both scroll at the rate-slider rate. RX to TX transition pushes the first TX row immediately (no stall from an un-seeded timer). Could not run a local build: Qt6 is not installed in this environment, so CI verifies the build.
Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V)
reverse-engineered from a proprietary binary (Principle IV)
MeterSmoother(AGENTS.md convention)AI was used for assistance.