Add loop antenna controls#2863
Conversation
There was a problem hiding this comment.
Pull request overview
Adds LoopA/LoopB awareness to panadapter state and the spectrum ANT overlay, using model capabilities to expose loop controls only on supported radios.
Changes:
- Extends
ModelCapabilitiesand tests with LoopA/LoopB flags. - Tracks panadapter RX antenna and loop state from status updates.
- Adds loop controls to the spectrum overlay and wires commands through
MainWindow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/model_capabilities_test.cpp |
Adds LoopA/LoopB expectations to capability tests. |
src/models/PanadapterModel.h |
Adds RX antenna and loop state accessors/signals/storage. |
src/models/PanadapterModel.cpp |
Parses rxant, loopa, and loopb from panadapter status. |
src/models/ModelCapabilities.h |
Adds loop capability fields. |
src/models/ModelCapabilities.cpp |
Populates loop capability table per model. |
src/gui/SpectrumOverlayMenu.h |
Adds loop UI API, signals, and panadapter wiring state. |
src/gui/SpectrumOverlayMenu.cpp |
Adds loop buttons, visibility gating, and panadapter RX antenna synchronization. |
src/gui/MainWindow.cpp |
Wires overlay pan/model/capabilities and sends loop commands. |
Comments suppressed due to low confidence (1)
src/gui/SpectrumOverlayMenu.cpp:384
- This mirrors the LoopA switching path: unchecking LoopA under
QSignalBlockersuppressesloopAToggled(false), so switching from LoopA to LoopB only sendsloopb=1and does not explicitly clear LoopA on the radio. Emit/send the complete state transition so the radio and UI stay mutually exclusive.
if (on && m_loopABtn && m_loopABtn->isChecked()) {
QSignalBlocker blocker(m_loopABtn);
m_loopABtn->setChecked(false);
}
emit loopBToggled(on);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (m_panStack->panadapter(pan->panId())) { | ||
| if (auto* sw = m_panStack->spectrum(pan->panId())) { | ||
| sw->overlayMenu()->setPanId(pan->panId()); | ||
| sw->overlayMenu()->setRadioModel(&m_radioModel); |
There was a problem hiding this comment.
Fixed in f281fc7 by mirroring the normal wirePanadapter initialization in the duplicate-pan path: the overlay now gets setPanId, setRadioModel, and setRadioCapabilities before that path returns. Verified with cmake --build build --parallel 8 and ctest --test-dir build -R model_capabilities_test --output-on-failure.
| if (on && m_loopBBtn && m_loopBBtn->isChecked()) { | ||
| QSignalBlocker blocker(m_loopBBtn); | ||
| m_loopBBtn->setChecked(false); | ||
| } | ||
| emit loopAToggled(on); |
There was a problem hiding this comment.
Fixed in f281fc7. When switching LoopA/LoopB, the UI still blocks the opposite button signal while updating the check state, but now manually emits the opposite loop false signal so MainWindow sends loopb=0 or loopa=0 before the selected loop command. Verified with cmake --build build --parallel 8 and ctest --test-dir build -R model_capabilities_test --output-on-failure.
There was a problem hiding this comment.
Looks good — nicely scoped feature with capability gating, RAII connection management via QPointer<PanadapterModel>, and the test table extended to cover the two new flags. Both Copilot findings are addressed in f281fc7:
- The duplicate-panadapter path in
MainWindow.cppnow mirrorswirePanadapter()by callingsetPanId/setRadioModel/setRadioCapabilitiesbefore the early return, so loop button visibility is seeded correctly. - The loop-toggle mutex in
SpectrumOverlayMenu.cppnow manually emitsloopBToggled(false)/loopAToggled(false)when unchecking the opposite button underQSignalBlocker, so the radio receives the complete desired state.
A couple of minor notes (non-blocking):
-
Implicit mutex in
PanadapterModel::applyPanStatus(PanadapterModel.cpp +97): when the radio reportsloopa=1, the parser proactively clearsm_loopB(and vice versa) even if the status frame doesn't containloopb. In practice FlexLib emits both keys, so this is defensive and harmless — but if a future firmware ever reportedloopa=1standalone withloopbunchanged-but-true, the client would silently drop the LoopB state without a radio echo. Worth a brief comment noting this is a UI-mirror assumption, not a protocol guarantee. -
Scope overlap with #2862: this PR's
rxantparsing inPanadapterModeland the "send via pan set when panadapter-authoritative" change in the RX ANT combo overlap with PR #2862's stated changes. You called out the merge-tree check in the description, so this is just a flag for the maintainer's attention on landing order.
Thanks for the FlexLib-mirroring discipline (capability table + matching test row) and for citing the FLEX-6500 LoopA / FLEX-6700 LoopA+LoopB hardware specifics in the model table comments. Appreciate the contribution! 🎯
f281fc7 to
a4e343a
Compare
Summary
Verification