feat(voice): make beep notification volume configurable in config.yaml - #56116
Closed
Kewe63 wants to merge 1 commit into
Closed
feat(voice): make beep notification volume configurable in config.yaml#56116Kewe63 wants to merge 1 commit into
Kewe63 wants to merge 1 commit into
Conversation
Closes NousResearch#55908. The CLI voice-mode beep amplitude is hardcoded at 0.3 inside tools.voice_mode:play_beep(), which makes the record start/stop cues too quiet on low-volume systems and headphones. Users couldn't adjust it without editing source. Move the literal into a configurable voice.beep_volume setting (clamped to 0.0-1.0, default 0.3 to preserve prior behaviour). The new _get_beep_volume() helper reads via the same load_config() pattern used by cli.py's _voice_beeps_enabled() and hermes_cli/voice.py's _beeps_enabled(), keeps bools / out-of-range / non-numeric / NaN values safely on the default, and falls back silently if config can't load so the audio cue never breaks the voice loop on a degenerate config.yaml. Covered by tests/tools/test_voice_mode.py: - TestGetBeepVolume (12 cases: missing key, custom value, boundary 0.0/1.0, out-of-range clamp, type coercion, bool guard, NaN guard, exception guard, dict-typed voice section) - TestPlayBeepVolumeWiring (guards against re-introducing a hardcoded 0.3 literal in play_beep) Docs: website/docs/user-guide/configuration.md mentions the new key. Other locale translations (zh-Hans etc.) intentionally untouched — handled by the regular i18n sync pipeline as a separate change. No change in default behaviour: existing users hear exactly the same beep.
Open
5 tasks
Contributor
|
Thanks for the focused configuration improvement. The underlying issue is present on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
Contributor
|
Merged into main via consolidated salvage PR #73520 (merge Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage. |
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
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.
Summary
tools.voice_mode:play_beep()hardcoded the tone amplitude at0.3(#55908), so the CLI voice-mode record start/stop beeps were too quiet on low-volume systems / headphones. Users couldn't adjust it without editing source. This PR addsvoice.beep_volumetoconfig.yamlwith a0.3default, preserving existing behavior for unconfigured installs.Changes
tools/voice_mode.pyplay_beep()— replaces the literal0.3with a dynamic value read from config._get_beep_volume()next toplay_beep()(same module, audio-cue section):0.3when the key is absent — byte-for-byte identical to current behavior.hermes_cli.config.load_config(), the same pattern already used bycli.py:_voice_beeps_enabled()(L10986-10995) andhermes_cli/voice.py:_beeps_enabled()(L247-257).load_configinside the function (parity with the two existing call sites) so a broken~/.hermes/config.yamlcan never break module import.0.0–1.0; falls back to default on out-of-range, NaN, non-numeric, bool, orload_configexception. The bool guard mirrors the long-standingisinstance(_threshold, (int, float)) and not isinstance(_threshold, bool)pattern atcli.py:10764-10765forsilence_threshold.hermes_cli/config.py—_DEFAULT_CONFIG_SCHEMAvoice:section default, adjacent tobeep_enabled(L2024):tests/tools/test_voice_mode.py— 2 new test classes:TestGetBeepVolume(12 cases): default-when-missing, default-when-section-missing, custom value, boundary0.0/1.0, out-of-range high/low, numeric string coercion, non-numeric fallback, bool fallback, NaN fallback, exception fallback, wrong-type-voice-section fallback.TestPlayBeepVolumeWiring(1 case):inspect.getsourceguard against re-introducing a hardcoded0.3literal at the amplitude line — regression guard for the original symptom.website/docs/user-guide/configuration.mdvoice:reference block (L1580) with an inline comment matching the other entries' style. Locale translations (zh-Hans, etc.) are intentionally untouched — handled by the regular i18n sync pipeline.How to Test
User Migration
No action required. Users who haven't configured
voice.beep_volumekeep the existing0.3amplitude byte-for-byte. To make the beep louder:Out-of-range values silently use the default — a typo cannot brick the voice loop.
Checklist
ruff check— PASS, 0 warningsRisk & Impact
None. No behavior change without an opt-in
voice.beep_volumekey in config. All fallback paths degrade to the existing0.3default. A bad config value can't break the voice loop.Type: ✨ New feature
Closes: #55908