Skip to content

[controllers] Route FlexControl volume to master volume#2925

Open
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/issue-2921-flexcontrol-volume
Open

[controllers] Route FlexControl volume to master volume#2925
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/issue-2921-flexcontrol-volume

Conversation

@jensenpat
Copy link
Copy Markdown
Collaborator

Summary

  • Route FlexControl wheel Volume mode through master volume instead of active slice audio gain.
  • Route FlexControl VolumeUp/VolumeDown actions through the same master-volume path.

Validation

  • cmake -S . -B build-ninja -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_PREFIX_PATH=/opt/homebrew/opt/qt
  • cmake --build build-ninja --parallel 22
  • ctest --test-dir build-ninja --output-on-failure -j22 (15/15 passed)

Closes #2921

👨🏼‍💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat

Co-authored-by: Codex <noreply@openai.com>
@jensenpat jensenpat changed the title Route FlexControl volume to master volume [controls] Route FlexControl volume to master volume May 21, 2026
@jensenpat jensenpat marked this pull request as ready for review May 21, 2026 16:06
@jensenpat jensenpat requested a review from ten9876 as a code owner May 21, 2026 16:06
@jensenpat jensenpat changed the title [controls] Route FlexControl volume to master volume [controllers] Route FlexControl volume to master volume May 21, 2026
Copy link
Copy Markdown
Contributor

@aethersdr-agent aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jensenpat — change is tight and fixes a real ergonomic issue (wheel-volume only affecting one slice).

The new adjustMasterVolume lambda mirrors the existing TCI masterVolumeRequested path at MainWindow.cpp:4293-4297 (m_titleBar->setMasterVolume(pct) + applyMasterVolume(pct)), so the title bar, audio path, persistence, and TCI broadcast all stay in sync — nice.

A couple of small observations, none blocking:

  1. Read source for current value. Reading cur from AppSettings each step relies on applyMasterVolume having just written "MasterVolume" to settings (it does, at line 9074). That works, but the title bar slider's current value is arguably the closer canonical source and avoids a settings round-trip per wheel tick. Either is fine — just noting that this couples wheel adjustment to the settings-write behavior in applyMasterVolume.

  2. Wheel step size (steps * 2). This preserves the prior wheel sensitivity (was steps * 2.0f on slice gain), so muscle memory carries over. Worth confirming 2 % per detent still feels right when you're now adjusting a 0–100 master rather than a per-slice float gain — they're the same domain in practice, so likely fine.

  3. One tiny redundancy. applyMasterVolume already clamps to [0,100] internally (lines 9066-9067), so the std::clamp(cur + deltaPct, 0, 100) in the lambda is belt-and-suspenders. Not worth changing — clamping at the caller before setMasterVolume on the title bar is good practice.

Code looks good to me. AppSettings used correctly, no new resource concerns, scope matches the PR description (single file, both Volume wheel mode and VolumeUp/Down button actions). Closes #2921 as advertised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flex control volume wheel control master volume instead of slice volume

1 participant