Add AetherControl virtual FlexControl controller#2888
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new non-modal “AetherControl” (virtual FlexControl) dialog and integrates it with the existing FlexControl plumbing so the virtual controller and physical device share settings, actions, and indicator/LED behavior. It also adjusts SpectrumWidget waterfall reprojection logic to reduce visual jumps during rapid FlexControl-driven edge scrolling.
Changes:
- Introduce
FlexControlDialog(virtual wheel with inertial tuning, compact mode, mouse capture + ESC release, and physical detect/disconnect controls) and expose it via Settings → AetherControl… - Refactor FlexControl handling in
MainWindowto unify action dispatch for physical + virtual inputs, sync button mappings, and drive hardware LED state. - Extend
FlexControlManagerto open the serial port read/write and send Aux LED commands; tweak waterfall reprojection during animated pan retargets.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/gui/SpectrumWidget.cpp | Adjusts waterfall reprojection source center during pan animation/retargets to reduce jumpiness. |
| src/gui/RadioSetupDialog.h | Adds helpers/state for syncing FlexControl action combos + connection status UI. |
| src/gui/RadioSetupDialog.cpp | Wires FlexControl action mapping changes to emit serialSettingsChanged and adds refresh/status helpers. |
| src/gui/MainWindow.h | Expands wheel-mode enum and adds slots/state for virtual FlexControl dialog + LED/indicator tracking. |
| src/gui/MainWindow.cpp | Adds AetherControl dialog launch + synchronization; centralizes FlexControl action dispatch; syncs hardware connection + LED. |
| src/gui/FlexControlDialog.h | New persistent dialog API for the virtual FlexControl controller. |
| src/gui/FlexControlDialog.cpp | Implements the virtual wheel UI/behavior, action mapping UI, and settings integration. |
| src/core/FlexControlManager.h | Adds API/state for writing Aux LED state to the physical device. |
| src/core/FlexControlManager.cpp | Opens serial port read/write and implements LED write commands + shutdown LED clear. |
| CMakeLists.txt | Adds FlexControlDialog.cpp to GUI sources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto& fcs = AppSettings::instance(); | ||
| const bool fcOpen = fcs.value("FlexControlOpen", "False").toString() == "True"; | ||
| const QString fcPort = fcs.value("FlexControlPort").toString(); | ||
| const bool fcInvert = fcs.value("FlexControlInvertDir", "False").toString() == "True"; | ||
| QMetaObject::invokeMethod(m_flexControl, [this, fcOpen, fcPort, fcInvert] { | ||
| if (fcOpen) { | ||
| if (!m_flexControl->isOpen() && !fcPort.isEmpty()) | ||
| m_flexControl->open(fcPort); | ||
| } else if (m_flexControl->isOpen()) { | ||
| m_flexControl->close(); | ||
| } | ||
| m_flexControl->setInvertDirection(fcInvert); | ||
| }); |
There was a problem hiding this comment.
Addressed in fa48740. The FlexControl settings reload path now closes the current device when the configured port is empty, and closes/reopens when FlexControlPort differs from the currently open port. I applied the same reopen logic in both the serialSettingsChanged handler and the Radio Setup finished handler.
| const QString primaryAction = flexControlButtonAction(button, 0); | ||
| if (action == 1) { | ||
| if (primaryAction == QLatin1String("WheelRit")) { | ||
| if (auto* s = activeSlice()) | ||
| s->setRit(s->ritOn(), 0); | ||
| syncFlexControlDialog(); | ||
| return; | ||
| } | ||
| if (primaryAction == QLatin1String("WheelXit")) { | ||
| if (auto* s = activeSlice()) | ||
| s->setXit(s->xitOn(), 0); | ||
| syncFlexControlDialog(); | ||
| return; | ||
| } | ||
| if (primaryAction == QLatin1String("WheelApf")) { | ||
| if (auto* s = activeSlice()) | ||
| s->setApf(!s->apfOn()); | ||
| syncFlexControlDialog(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Addressed in fa48740. Double tap now routes through the configured Action1 mapping instead of being intercepted by the RIT/XIT/APF primary action. I added explicit ClearRit, ClearXit, and ToggleApf actions to both AetherControl and Radio Setup so those behaviors remain selectable without shadowing user-configured double-tap actions.
There was a problem hiding this comment.
Nice work — the virtual controller is well-scoped, AppSettings is used consistently, the FlexControlManager LED-write path is RAII-clean, and binding the dialog's connection state through RadioSetupDialog keeps the existing Serial tab in sync. Thanks for the contribution, @rfoust.
A few things worth addressing before merge — both Copilot findings are valid:
1. serialSettingsChanged ignores port changes when already open (MainWindow.cpp:5022-5030)
if (fcOpen) {
if (!m_flexControl->isOpen() && !fcPort.isEmpty())
m_flexControl->open(fcPort);
} else if (m_flexControl->isOpen()) {
m_flexControl->close();
}If the user re-runs Detect (or edits FlexControlPort directly) while the device is already open, the new port is persisted to AppSettings but the manager keeps the old serial connection. Suggest reopening when the port differs:
if (fcOpen) {
if (fcPort.isEmpty()) {
if (m_flexControl->isOpen()) m_flexControl->close();
} else if (!m_flexControl->isOpen() || m_flexControl->portName() != fcPort) {
if (m_flexControl->isOpen()) m_flexControl->close();
m_flexControl->open(fcPort);
}
} else if (m_flexControl->isOpen()) {
m_flexControl->close();
}2. Hard-coded double-tap shadows the user-configurable Action1 (MainWindow.cpp:5705-5722)
When the configured Action0 is WheelRit / WheelXit / WheelApf, handleFlexControlButton() runs the reset-to-zero / APF-toggle path on action == 1 and returns before looking up FlexControlBtn%NAction1. But FlexControlDialog exposes a Double Tap combo per Aux and writes that key — meaning the UI lets you pick a Double Tap action that the runtime will never honor when the Single Tap is one of those three wheel modes.
Either route action == 1 through flexControlButtonAction(button, 1) first and only fall back to the hard-coded reset when that action is None (or some "default"), or disable/grey-out the Double Tap combo in the dialog for those Single Tap selections so the UI matches behavior.
3. Minor — setActiveLedButton clamps invalid values to 0 silently
FlexControlManager.cpp:67-77 rewrites button to 0 for any out-of-range value before the early-return check, so callers passing a stale-but-equal m_activeLedButton get treated as "already off, no-op." Probably fine in practice, but a qCWarning on clamp would help diagnose the case where something thinks it set Aux2 but didn't.
Otherwise the SpectrumWidget waterfall reprojection fix looks correct — reprojecting from m_panCenterTarget rather than the mid-animation visual center during rapid retargets is the right anchor.
There was a problem hiding this comment.
Thanks @rfoust — this is a substantial, well-organized addition. The virtual controller mirrors the physical FlexControl settings rather than introducing a parallel config path, the LED-state push to the device is clean, and moving the coalesce timer + wheel-mode state out of the HAVE_SERIALPORT block so virtual operation works without the device built in is the right call. The SpectrumWidget reproject change (using m_panCenterTarget as the waterfall source frame when an animation is already in flight) is a real fix that stands on its own merits.
Both Copilot findings look valid and worth addressing:
1. Port-change reopen (MainWindow.cpp serialSettingsChanged handler) — confirmed. The lambda only opens when !isOpen(), so a re-detect that resolves to a different /dev/tty* while the device is currently open will leave the manager bound to the stale port. Suggest something like:
if (fcOpen && !fcPort.isEmpty()) {
if (!m_flexControl->isOpen() || m_flexControl->portName() != fcPort) {
if (m_flexControl->isOpen()) m_flexControl->close();
m_flexControl->open(fcPort);
}
} else if (m_flexControl->isOpen()) {
m_flexControl->close();
}2. Hardcoded double-tap for Wheel{Rit,Xit,Apf} (handleFlexControlButton) — confirmed. The action == 1 early-return block runs before flexControlButtonAction(button, 1) is consulted, so any user-set FlexControlBtn%NAction1 for a button whose Action0 is one of those three is silently unreachable. Two options:
- Gate the special-case behind
flexControlButtonAction(button, 1) == "None"so the convenience reset only triggers when the user hasn't bound double-tap to something else, or - Expose the reset behavior as a first-class action id (e.g.
ClearRit,ClearXit,ToggleApf) in the dropdown so it can be selected explicitly and the implicit hardcoding goes away.
I'd lean toward the second option since the action lists in RadioSetupDialog.cpp and FlexControlDialog.cpp already include a None entry the user can pick if they want the old reset semantics.
Smaller note (non-blocking):
In handleFlexControlButton, the BandZoom / SegmentZoom branches both early-return after sending the command and skip the syncFlexControlDialog() at the bottom of the function — minor inconsistency vs. the other handlers, but probably benign since neither directly changes wheel mode or active aux.
Otherwise the structure looks good: AppSettings is used throughout (no QSettings creep), the worker-thread crossings go via QMetaObject::invokeMethod, RAII is preserved (QSerialPort member, signal connections tied to dialog lifetime via QPointer), and the I000;/Ixyz; LED command encoding matches the documented protocol comment in the header.
d146cab to
fa48740
Compare
fa48740 to
93730eb
Compare
# Conflicts: # src/gui/MainWindow.cpp # src/gui/MainWindow.h # src/gui/RadioSetupDialog.cpp
|
Claude here — pushed two cleanup commits on top:
Local Release build clean. CI re-running. @rfoust — this is a substantial feature done thoughtfully. The Will admin-merge once CI lands green. 73, |
|
Claude here — merged. Thanks @rfoust, this is the largest single What stood out on review: The slot-based refactor of `handleFlexControlTuneSteps` and The physical+virtual LED sync via `setActiveLedButton()` → The `SpectrumWidget` waterfall reprojection fix during rapid CI fully green including check-windows (rare in this batch). Ships in v26.5.3. 73, |
…3014) Closes #2986. PR #2925 made `FlexWheelMode::Volume` route to master volume (matching SmartSDR per #2921). PR #2888 had previously added a separate `FlexWheelMode::MasterAf` for master volume. After #2925 the two modes were byte-identical — same dispatcher case, same display entry, same code path. This PR collapses them. ## Changes - **Drop `FlexWheelMode::MasterAf` enum value** and the corresponding switch case in `handleFlexControlTuneSteps` + `flexWheelModeName` mapping. - **Fold `WheelMasterAf` into `WheelVolume`** branch in `applyFlexControlWheelAction` via short-circuit OR — body lives once. - **Keep accepting `"WheelMasterAf"` as a string alias** in: - `flexWheelModeForAction` — maps to `FlexWheelMode::Volume` - `FlexControlDialog::isWheelAction` predicate — same back-compat This preserves saved FlexControl bindings that picked "WheelMasterAf" before the consolidation. - **Remove `WheelMasterAf` from the Radio Setup dropdown** so new bindings only see the canonical `WheelVolume`. - **Relabel `WheelVolume` from "Slice AF" to "Master Volume"** in FlexControlDialog's display map — "Slice AF" was the pre-#2925 description and is now misleading (the action no longer touches slice gain). ## Behavior - Users with existing "WheelMasterAf" bindings: continue working unchanged (back-compat alias path). - New users: see one option instead of two identical-behavior choices. - No-binding behavior unchanged. ## Test plan - [x] Builds clean - [ ] Run with a FlexController hardware setup, confirm wheel-volume mode still adjusts master volume - [ ] Verify a previously-saved "WheelMasterAf" binding still works (back-compat alias) - [ ] Verify the Radio Setup dropdown no longer lists WheelMasterAf 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ESC asymmetry) (#3103) ## Summary Changes AetherControl's virtual FlexControl knob from a click-to-latch / Escape-to-unlatch UX to a **double-click for both directions**, with Escape kept as a secondary safety release. ## Why Operators report losing track of the release path. First-time users miss the small "Press ESC to release" hint at the bottom of the dialog and feel stuck with the cursor captured. ## What changed | Action | Before | After | |---|---|---| | **Latch** | Single left-click on wheel | **Double-click** on wheel | | **Unlatch** | ESC key | **Double-click** on wheel (primary) or ESC (secondary safety) | | Single click on wheel | Latched | No-op (event accepted to suppress accidental window drag) | Updated user-visible strings: - Resting hint: `"Double-click the knob to capture circular tuning."` - Armed hint: `"Mouse locked to FlexControl. Double-click the knob to release (or press ESC)."` - Accessible description mirrors the new gesture. ESC stays as a backup for the edge case where the cursor ended up offscreen and can't get back over the knob to double-click. ## Attribution AetherControl original implementation by [@rfoust](https://github.com/rfoust) ([#2888](#2888)); FlexControl wheel UX overhaul also by @rfoust ([#3029](#3029)). This change preserves the existing wheel-feel work and only touches the capture/release gesture. ## Test plan - [x] Linux build clean - [ ] CI: build / check-paths / check-windows / CodeQL - [ ] Manual: open AetherControl (Settings → FlexControl), single-click on wheel does nothing, double-click captures, double-click again releases, ESC still releases as backup 73, Jeremy KK7GWY & Claude (AI dev partner) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Adds a non-modal AetherControl popout that emulates the FlexControl device, including the tuning wheel, Aux buttons, Push button, indicator lights, compact mode, and physical-device Detect/Connected controls.
Validation
cmake --build build --parallel 8git merge-tree; no merge conflicts were reported.