feat(spectrum): continuous edge auto-pan while dragging a slice#3581
feat(spectrum): continuous edge auto-pan while dragging a slice#3581svabi79 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Thanks @svabi79 — this is a thorough, well-instrumented change, and the velocity-vs-position-controller framing in the description (plus the env-tunable knobs and the AETHER_NO_DRAG_EDGEPAN A/B escape hatch) makes the UX decision easy to evaluate. CI is green across all six checks. One substantive concern and a couple of minor notes.
1. The new tune path bypasses applyTuneRequest's guards (lock / SWR sweep / diversity)
This is the main thing I'd want addressed before merge. The whole drag now routes through edgePanTuneRequested, and its MainWindow handler tunes the slice with a bare target->setFrequency(sliceFreqMhz):
queueActiveSliceForSpectrumTarget(target->sliceId());
target->setFrequency(sliceFreqMhz);Previously the drag emitted incrementalTuneRequested → applyTuneRequest(target, mhz, IncrementalTune, …). By going around applyTuneRequest you also drop the guards it applies to every other tune source (MainWindow.cpp:5654):
- Locked slice —
applyTuneRequestcallsnotifyTuneBlockedByLock()and restores the VFO readout.mousePressEventstartsm_draggingVforegardless of lock state, so with this PR a user can now drag a locked slice's passband and it will retune. That's a regression in the lock affordance. - SWR sweep —
applyTuneRequestearly-returns whilem_swrSweep.running. The direct path lets a drag retune mid-sweep. - Diversity mirroring —
applyTuneRequestcallsmirrorDiversityChildFrequency(slice, mhz); the direct path skips it, so the diversity child won't follow the parent during a drag.
I understand why you bypassed it — applyTuneRequest(IncrementalTune) ends in panFollowVfo, which is exactly the controller you're trying to keep out of the drag path. But the lock/SWR/diversity checks are orthogonal to pan-follow. Could you preserve them, e.g. by factoring the guard block (lock → notifyTuneBlockedByLock, m_swrSweep.running, mirrorDiversityChildFrequency) into a small helper that both applyTuneRequest and the edgePanTuneRequested handler call, so the edge-pan path tunes-without-follow but still honors them?
2. No upper bound on center (minor)
edgePanVelocityStep clamps the low edge (std::max(newCenter, m_bandwidthMhz/2.0)) and the MainWindow handler repeats that floor, but neither caps the high end. Holding at the right edge advances m_centerMhz (and reprojects the waterfall) unbounded until the radio echo clamps it back, which can show as overshoot-then-snap at the top of the tuning range. A symmetric upper clamp against the pan's max frequency would keep the local view consistent with what the radio will accept.
3. Direct sendCommand vs applyPanRangeRequest (nit, non-blocking)
The handler issues a raw display pan set … center=… rather than going through applyPanRangeRequest, so it doesn't update pan->applyPanStatus() locally or share that path's dedupe. You rely on the widget's local reproject for immediacy instead, which works — just flagging the divergence from the established explicit-pan path for consistency.
Nice work on the root-cause analysis and the symmetric L/R telemetry. The velocity-default sign-off is appropriately flagged as a maintainer call. Addressing #1 would be my one blocker.
🤖 aethersdr-agent · cost: $2.6158 · model: claude-opus-4-8
…Principle XI. Review of aethersdr#3581: the edge-pan tune path bypassed applyTuneRequest to avoid pan-follow, which also dropped the lock / SWR-sweep / diversity guards every other tune source honours -- a dragged *locked* slice would retune, a drag mid-SWR-sweep would tune, and the diversity child would not follow. Extract the lock + SWR-sweep guard into MainWindow::tuneBlockedByGuards() (a behaviour-identical refactor of applyTuneRequest's existing block) and call it from the edgePanTuneRequested handler before panning/tuning; a blocked tune now pans nothing either, matching the pre-existing locked-slice drag. Mirror the diversity child in the handler too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the careful review — addressed in ff79c04. 1. Guard bypass (lock / SWR / diversity) — fixed. You're right, this was a 2. No upper center clamp. Intentionally consistent with the existing pan 3. The velocity defaults remain flagged for your UX sign-off; |
…iple XI.
Dragging a slice to the panadapter edge to tune across the band only
crept and stuttered ("rubber band", worse going down): the edge follow
(revealFrequencyIfNeeded) is a position controller whose nudge is driven
by the cursor's overshoot past the trigger margin, and at the frame
border that overshoot is tiny and self-limiting (~0.1x span/s, measured),
so you had to zoom in first. The flag-extended trigger (aethersdr#2761) also fires
asymmetrically just inside the edge, fighting the follow on the flag side.
Replace the edge behaviour with a velocity controller: while the cursor
sits in the edge zone, a ~30 Hz timer pans at a speed that scales with
edge depth and ramps with hold time (measured 1.2x span/s at full
depth+ramp, monotonic, symmetric L/R), parking the slice just inside the
leading edge so it stays visible while the band scrolls under it. Pan+tune
go through a new edgePanTuneRequested signal that bypasses pan-follow so
the two controllers can't fight; reveal is removed from the drag path
entirely (in-window moves tune-only). aether.perf SliceDrag logging makes
a drag replayable from the support log.
The velocity defaults (top speed / ramp / interval) are a UX decision and
are exposed as env overrides (AETHER_DRAG_EDGEPAN_VMAX / _RAMP /
_INTERVAL) so the feel can be tuned without a rebuild -- they need
maintainer sign-off. AETHER_NO_DRAG_EDGEPAN=1 restores the legacy
reveal-only behaviour for A/B comparison.
Fixes aethersdr#3580
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Principle XI. Review of aethersdr#3581: the edge-pan tune path bypassed applyTuneRequest to avoid pan-follow, which also dropped the lock / SWR-sweep / diversity guards every other tune source honours -- a dragged *locked* slice would retune, a drag mid-SWR-sweep would tune, and the diversity child would not follow. Extract the lock + SWR-sweep guard into MainWindow::tuneBlockedByGuards() (a behaviour-identical refactor of applyTuneRequest's existing block) and call it from the edgePanTuneRequested handler before panning/tuning; a blocked tune now pans nothing either, matching the pre-existing locked-slice drag. Mirror the diversity child in the handler too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ff79c04 to
a5eaf6a
Compare
|
Ready for review when you have a moment, @ten9876. CI is green across all six checks, and the review blocker from @aethersdr-agent (lock / SWR-sweep / diversity guards on the edge-pan tune path) is addressed in the latest commit. The branch is rebased onto current This touches |
Summary
Continuous edge auto-pan while dragging a slice across the panadapter, so you can
sweep the whole band in one gesture without zooming in first. Fixes #3580.
Replaces the edge-follow position controller (
revealFrequencyIfNeeded) —whose nudge is bounded by the cursor's self-limiting overshoot at the frame
border (~0.1× span/s, the "rubber band" creep) — with a velocity controller:
while the cursor sits in the edge zone a ~30 Hz timer pans at a speed that scales
with edge depth and ramps with hold time (~1.2× span/s at full depth+ramp),
parking the slice just inside the leading edge so it stays visible while the band
scrolls under it. Pan+tune go through a new
edgePanTuneRequestedsignal thatbypasses pan-follow so the two controllers can't fight; reveal is removed from
the drag path entirely (in-window moves tune-only). This also removes a
one-sided stutter/jump-back caused by the flag-extended trigger (#2761) firing
asymmetrically just inside the edge on the flag side.
This is a UX behaviour change — per AGENTS.md not an autonomous change. The
velocity defaults (top speed / ramp / interval) are a feel decision and need
your sign-off. They're exposed as env overrides so you can tune them live without
rebuilding:
AETHER_DRAG_EDGEPAN_VMAX— top speed, % of span per second (default 120)AETHER_DRAG_EDGEPAN_RAMP— ms to ramp to top speed (default 600)AETHER_DRAG_EDGEPAN_INTERVAL— timer interval ms (default 33)AETHER_NO_DRAG_EDGEPAN=1— restore legacy reveal-only behaviour (A/B)Test plan / evidence
directions on a live FLEX.
aether.perfSliceDragtelemetry: legacy ≈ 0.0027 MHz/tick regardless of howhard you push (~0.1× span/s); fixed = 0.00182 MHz/tick at full depth+ramp
(1.2× span/s), center monotonic, no jump-back, symmetric L/R, slice parked at
the 5% boundary.
AETHER_NO_DRAG_EDGEPAN=1reproduces the old behaviour on the same build for adirect A/B comparison.
Scope
Independent of #3578 (waterfall perf) and deliberately kept separate from #3444
(mouseMoveEvent overload).
Principle XI.
🤖 Generated with Claude Code