[codex] Fix VFO flag-side pan following#3866
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes pan-follow behavior during incremental VFO tuning near pan edges by predicting the VFO flag side after the next tune step (rather than using the stale current side), preventing unnecessary pan recentering when the flag would flip inward and remain visible. It also adds deterministic VFO flag placement helpers + a focused test, and extends the automation bridge with pan-visible grabs and VFO targeting/wheel events for UI verification workflows.
Changes:
- Predict flag-side placement for the new frequency in
panFollowVfo(), using new Spectrum/VFO helper logic. - Add deterministic flag placement helpers (single + multi-VFO) and a dedicated
vfo_flag_placement_test. - Extend the automation bridge + probe script + docs with
grab pan-visible, VFO selectors, andinvoke ... wheel.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/automation_probe.py | Adds CLI support for grab pan-visible <index> / pan-composite selectors. |
| tests/vfo_flag_placement_test.cpp | New deterministic test covering mode defaults + edge/hysteresis and multi-VFO placement decisions. |
| src/gui/VfoWidget.h | Introduces reusable, deterministic helpers for default flag side and auto-direction decisions. |
| src/gui/VfoWidget.cpp | Reuses the new mode-default helper; surfaces sliceId as a widget property for automation targeting. |
| src/gui/SpectrumWidget.h | Adds vfoFlagOnLeftForSlice(...) API to predict flag side for a given slice/frequency. |
| src/gui/SpectrumWidget.cpp | Implements predicted-flag-side logic and refactors multi-VFO direction logic into helpers used by paint/reposition paths. |
| src/gui/MainWindow_Wiring.cpp | Switches pan-follow offset selection to predicted flag side for the new frequency. |
| src/core/AutomationServer.h | Documents new grab pan-visible verb in the bridge header docs. |
| src/core/AutomationServer.cpp | Adds grab pan-visible, VFO selectors (vfo <panIndex>, vfo slice <id>), and invoke ... wheel. |
| docs/automation-bridge.md | Documents grab pan-visible, VFO selector resolution order update, and wheel invoke action. |
| CMakeLists.txt | Registers vfo_flag_placement_test with CTest. |
| QWidget* vfoWidgetForPanIndex(int index) | ||
| { | ||
| const QList<QWidget*> vfos = findWidgetsByClass(QStringLiteral("VfoWidget")); | ||
| for (QWidget* vfo : vfos) { | ||
| for (QWidget* a = vfo; a; a = a->parentWidget()) { | ||
| if (shortClassName(a) != QLatin1String("SpectrumWidget")) { | ||
| continue; | ||
| } | ||
| const QVariant pi = a->property("panIndex"); | ||
| if (pi.isValid() && pi.toInt() == index) { | ||
| return vfo; | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Thanks for this, @rfoust — nicely done. The core fix is exactly right: predicting the flag side for the new frequency via vfoFlagOnLeftForSlice(sliceId, mhz, ...) instead of reading the stale vfo->onLeft() is the correct way to stop wheel tuning from forcing a spurious recenter when the flag can flip inward. Pulling the placement decision into the static autoDirectionForSingleFlag / autoDirectionForDeconflictedFlag / defaultFlagOnLeftForMode helpers and reusing them across repositionVfoFlags, paintEvent, and the pan-follow predictor is a clean de-duplication of what was previously three copies of the same branchy logic — and it makes the behavior unit-testable, which the new vfo_flag_placement_test covers well (edge guards + hysteresis hold/return on both sides). CI is green across build/macos/windows/CodeQL/a11y.
A couple of minor, non-blocking notes:
1. vfoWidgetForPanIndex() doesn't prefer a visible match (confirming Copilot's finding). This one is valid. panSpectrumWidgetForIndex() and vfoWidgetForSliceId() both keep scanning and prefer a visible widget when duplicate indices/ids transiently coexist (mid float/dock reparent), but vfoWidgetForPanIndex() returns on the first panIndex match regardless of visibility:
if (pi.isValid() && pi.toInt() == index) {
return vfo; // first match wins, even if hidden
}For consistency with its siblings — and to avoid invoke vfo <panIndex> wheel silently no-op'ing against a hidden flag during a reparent — consider tracking a candidate and preferring isVisible(), same shape as the other two helpers. Low severity since it's only the transient-duplicate window, but the sibling helpers already established the pattern.
2. Scope. The flag-side fix itself is small; the bulk of the diff is the automation-bridge surface (grab pan-visible/pan-composite, the wheel invoke verb, the vfo slice N / vfo N selector shortcuts, sliceId property exposure, docs, and automation_probe.py). The PR body explains these were the tooling used to verify the fix on a real radio, which is reasonable and they're internally coherent and well-documented — just flagging that a reviewer skimming the title may not expect a feature bundle. Not asking you to split it; just noting it.
Both points are minor — the fix and refactor are solid as-is.
🤖 aethersdr-agent · cost: $2.9976 · model: claude-opus-4-8
576d2b8 to
73963e7
Compare
ten9876
left a comment
There was a problem hiding this comment.
Approved. Reviewed the fix + verified the bridge additions are TX-safe.
- Core fix correct — the pan-follow reveal guard now predicts the flag side for the new frequency via
vfoFlagOnLeftForSlice(sliceId, mhz)instead of the stalevfo->onLeft(), so wheel-tuning near a pan edge no longer forces a spurious recenter when the flag flips inward. Clean de-dup into static, unit-tested helpers (vfo_flag_placement_test: edge guards + hysteresis). Complements #3786's drag-recenter fix. - Bridge additions TX-safe — the
wheelverb dispatches a QWheelEvent (RX tuning) and is gated by the sameisTransmitControlguard that runs first indoInvoke;grab pan-visible/pan-compositeare read-only screenshots;vfoselectors are targeting only. Not a keying path (Principle VI N/A). Live-verified on a FLEX-6700 with TX false throughout.
One minor follow-up filed: vfoWidgetForPanIndex() should prefer a visible match like its siblings. CI green on all 6. Thanks @rfoust.
Summary
Fixes #3840. Pan Follows VFO now predicts the side a slice flag will occupy after the next tune step instead of using the stale current flag side. That prevents normal wheel tuning near a pan edge from forcing a pan recenter when the flag can flip inward and remain fully visible.
This also adds deterministic multi-VFO flag placement helpers, keeps mode-specific split and RTTY defaults, and extends the agent automation bridge with VFO targeting, wheel events, and
grab pan-visible <index>for operator-visible pan screenshots that include VFO flag overlays.Root Cause
The pan-follow reveal guard used the current
VfoWidget::onLeft()value beforeVfoWidget::updatePosition()had a chance to recompute the flag side for the new frequency. When the next frequency would make the flag flip inward, the guard still treated the old outward side as authoritative and recentered the pan unnecessarily.Constitution principle honored
Principle VIII - Evidence Over Assertion. The placement logic has a focused helper test, and the behavior was verified with the visible agent automation bridge against a real FLEX-6700 without enabling TX automation.
Test plan
cmake --build build --target AetherSDR vfo_flag_placement_test -j16ctest --test-dir build -R ^vfo_flag_placement_test$ --output-on-failuregit diff --checkpython3 -m py_compile tools/automation_probe.pypython3 tools/check_a11y.pyexits 0 with existing warning-only findingsBridge evidence:
110x40000001[1, 2]14.2196 MHz14.2184 MHz14.236363 MHzfalse / false / falsegrab pan-visible 1returnedclass: PanadapterApplet,surfaceClass: SpectrumWidget,762x724grab pan 1still returnedclass: SpectrumWidget,762x708Checklist
AppSettingscallsMeterSmootheror is not meter UIGenerated with OpenAI Codex.