fix(gui): tighten GuardedSlider member types, extract EQ formatter + linger constant (#2987)#3012
Merged
Merged
Conversation
…linger constant (#2987) Three polish items from the PR #2944 (DragValuePopup) review. 1. Tighten slider member types where construction is GuardedSlider. TxApplet and PhoneApplet declared m_rfPowerSlider / m_tunePowerSlider / m_amCarrierSlider / m_voxLevelSlider / m_dexpSlider as QSlider* despite constructing them as GuardedSlider, forcing awkward static_cast<GuardedSlider*>(...)->setDragValueFormatter(...) calls at the use site. Member type is now GuardedSlider* so the formatter calls and any other GuardedSlider-specific API resolves directly. Forward declarations in the headers go in the global namespace where GuardedSlider actually lives, not under namespace AetherSDR (this was the build error during the first attempt — silent ODR-incompatible duplicate type). 2. Extract dbWithSignText helper in EqApplet.cpp. The EQ band sliders' drag-popup formatter was an inline lambda; other applets use file-scope free functions (wattsText, percentText, panText) for the same pattern. Pull the lambda out to match. 3. Name the DragValuePopup linger duration. The 450 ms 'memory cue' that lingers after slider release was baked into the default-argument literal in DragValuePopup.h. Promote to kDefaultLingerMs constexpr with a comment recording the testing rationale (250 ms too fleeting, 750 ms felt sluggish) so the value is findable without spelunking PR #2944's body. Closes #2987. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Nice, focused polish PR — the three items are independent, low-risk, and each one improves the code. Thanks @ten9876!
A few observations:
Correctness
- Forward-decl placement looks right:
GuardedSliderlives in the global namespace (src/gui/GuardedSlider.h:32), so theclass GuardedSlider;lines in both headers correctly sit outsidenamespace AetherSDR. Good catch on the first-attempt ODR trap noted in the PR description. dbWithSignTextis byte-identical to the lambda it replaces and matches thewattsText/percentText/panTextshape in sibling files.
Worth a follow-up (not blockers)
RxApplethas the exact same pattern this PR is cleaning up —m_afSliderandm_panSliderare declaredQSlider*inRxApplet.h:241-242but constructed asGuardedSliderand then static-cast forsetDragValueFormatter(RxApplet.cpp:773,:796). Easy to roll into a follow-up for consistency, or extend this PR if you want a clean sweep.m_voxDelaySliderinPhoneAppletis also constructed asGuardedSlider(PhoneApplet.cpp:198) but stays typed asQSlider*. No immediate effect since it has no formatter, but if we're standardizing the member type, this one is conceptually in the same bucket.- Minor: after this change
TxApplet.hno longer referencesQSliderdirectly, so theclass QSlider;forward decl is now unused. Harmless; skip if you prefer not to expand the diff.
LGTM aside from the suggested follow-up on RxApplet. The test-plan checkbox for manual drag-popup verification is still unchecked — worth ticking that off (or flagging a tester) before merge since the formatter/type plumbing is what's being moved around.
🤖 aethersdr-agent · cost: $4.3300 · model: claude-opus-4-7
G6PWY-Chris
pushed a commit
to G6PWY-Chris/AetherSDR
that referenced
this pull request
Jun 22, 2026
…linger constant (aethersdr#2987) (aethersdr#3012) Closes aethersdr#2987. Three polish items from the PR aethersdr#2944 (DragValuePopup) review. ## 1. Tighten slider member types `TxApplet` and `PhoneApplet` declared their sliders as `QSlider*` for historical reasons but constructed them as `GuardedSlider`, forcing `static_cast<GuardedSlider*>(...)->setDragValueFormatter(...)` at every call site. Member type is now `GuardedSlider*` so the formatter calls resolve directly: - `TxApplet.{h,cpp}`: `m_rfPowerSlider`, `m_tunePowerSlider` - `PhoneApplet.{h,cpp}`: `m_amCarrierSlider`, `m_voxLevelSlider`, `m_dexpSlider` Forward declarations in the headers go in the global namespace where `GuardedSlider` actually lives, not under `namespace AetherSDR`. (First attempt had them inside the namespace, which silently created an ODR-incompatible second type — caught by the compiler.) ## 2. Extract `dbWithSignText` helper in EqApplet.cpp The EQ band sliders' drag-popup formatter was an inline lambda; other applets use file-scope free functions for the same pattern. Lifted to a named helper for consistency: ```cpp static QString dbWithSignText(int v) { return QStringLiteral("%1%2 dB") .arg(v > 0 ? QStringLiteral("+") : QString()) .arg(v); } ``` ## 3. Name the DragValuePopup linger duration The 450 ms "memory cue" was baked into the default-argument literal in `DragValuePopup.h`. Promoted to `kDefaultLingerMs` constexpr with a comment recording the testing rationale so the value is findable without spelunking PR aethersdr#2944's body. ## Test plan - [x] Builds clean - [ ] Verify drag-popup readouts still appear correctly on RF Power, Tune Pwr, AM Carrier, VOX Level, DEXP, EQ bands (no behavior change expected — pure type/constant refactor) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2987. Three polish items from the PR #2944 (DragValuePopup) review.
1. Tighten slider member types
TxAppletandPhoneAppletdeclared their sliders asQSlider*for historical reasons but constructed them asGuardedSlider, forcingstatic_cast<GuardedSlider*>(...)->setDragValueFormatter(...)at every call site. Member type is nowGuardedSlider*so the formatter calls resolve directly:TxApplet.{h,cpp}:m_rfPowerSlider,m_tunePowerSliderPhoneApplet.{h,cpp}:m_amCarrierSlider,m_voxLevelSlider,m_dexpSliderForward declarations in the headers go in the global namespace where
GuardedSlideractually lives, not undernamespace AetherSDR. (First attempt had them inside the namespace, which silently created an ODR-incompatible second type — caught by the compiler.)2. Extract
dbWithSignTexthelper in EqApplet.cppThe EQ band sliders' drag-popup formatter was an inline lambda; other applets use file-scope free functions for the same pattern. Lifted to a named helper for consistency:
3. Name the DragValuePopup linger duration
The 450 ms "memory cue" was baked into the default-argument literal in
DragValuePopup.h. Promoted tokDefaultLingerMsconstexpr with a comment recording the testing rationale so the value is findable without spelunking PR #2944's body.Test plan
🤖 Generated with Claude Code