Improve fan mode button label clarity#3651
Conversation
There was a problem hiding this comment.
Thanks for the contribution, @sathvika234567 — this cleanly addresses #3649 (KW1M's confusion about "S" reading as "Standby"). The helper-function approach is the right call: it centralizes the mapping so all three call sites (constructor, click handler, setFanMode) stay in sync, and it correctly falls back to "Fan" for unknown/empty modes. Case handling is consistent too — setFanMode upper-cases input and the click handler uses upper-case kModes, so the == "STANDARD" comparisons all line up. No scope creep, no convention issues (no QSettings, no meter UI), single file. Nicely done.
Two small, non-blocking notes:
-
Stale comment. The comment just above the button is now inaccurate — it still describes the old single-letter scheme:
// Fan speed cycle button — single letter: S (STANDARD), C (CONTEST), B (BROADCAST).Worth updating to reflect the new "Fan: Std / Contest / Bcast" labels so the next reader isn't misled.
-
"Bcast" vs "Broadcast". The issue author suggested the full word ("Fan: Broadcast"). You abbreviated to "Bcast" — reasonable for width, and the button uses
QSizePolicy::Minimumso the full word would fit fine too. Either is acceptable; just flagging in case you'd prefer to match the requester's wording. (If you do, the accessible name and tooltip already spell out the full terms regardless.)
Neither is a blocker. Thanks again!
🤖 aethersdr-agent · cost: $1.3515 · model: claude-opus-4-8
…view aethersdr#3651) Maintainer co-fix on top of the contribution: the cycle-button comment still described the old single-letter labels (S/C/B), which now contradicts the fanModeLabel() output ("Fan: Std" / "Fan: Contest" / "Fan: Bcast"). Updated it and removed a stray double blank line. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ten9876
left a comment
There was a problem hiding this comment.
Approving. Code is correct (fanModeLabel applied at all 3 sites, closes #3649) and CI was green on the prior commit. Pushed a maintainer co-fix (6561158) updating the stale single-letter comment and dropping the extra blank line — the two cleanups from review. Squash-merge will credit @sathvika234567's first contribution. Enabling auto-merge so it lands once CI re-greens on the new commit.
Summary
Constitution principle honored
Test plan
cmake --build build)Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V)
reverse-engineered from a proprietary binary (Principle IV)
MeterSmoother(AGENTS.md convention)Summary
This change makes the amplifier fan mode button more descriptive by replacing the single-letter labels (S, C, B) with clearer text labels.
Changes
Motivation
The previous single-letter labels could be confused with amplifier state indicators, especially "S" being interpreted as "Standby". The new labels make the button purpose clearer while preserving existing behavior.
Fixes #3649