fix: correct hz_to_index to firmware quarter-tone scale#136
Merged
Conversation
hz_to_index() still implemented the pre-PR-98 linear pitch map
(idx = round(1 + (hz-400)*254/11600)), so every Hz-selected pitch played
the wrong note -- e.g. 440 Hz returned index 2, which the firmware folds
to A#4 (~466 Hz), a semitone sharp of the requested A4.
Replace it with the inverse of the firmware's quarter-tone equal-tempered
scale Freq(idx) = 13.75 * 2**(idx/24) Hz:
idx = clamp(round(24 * log2(hz / 13.75)), 1, 255) for hz > 0
idx = 0 for hz <= 0
Now hz_to_index(440) == 120 (exact A4) and the octave delta is 24.
The Hz-based input surface is unchanged; the firmware still octave-folds
out-of-window indices, so the host does not replicate that.
Rewrite TestHzToIndex for the new mapping and add landmark, octave, and
firmware-forward round-trip coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Resolves the conflict with OpenDisplay#135 (melody notation parser), which landed on main and edited the same imports/constants block in buzzer_activate.py. The changes are complementary: - imports: keep math (this PR) + re/Sequence (OpenDisplay#135) - constants: keep _ANCHOR_HZ, dedupe the duplicated _STEPS_PER_OCTAVE - hz_to_index: keep this PR's quarter-tone fix; OpenDisplay#135's melody code unchanged Full suite (852 tests), mypy --strict, and ruff pass on the merge result. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V74DxueL8qykYiobc3kmyJ
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
2 participants
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
hz_to_index()still implemented the pre-firmware-PR-98 linear Hz→index map, so every pitch chosen by Hz (including the Home Assistantactivate_buzzerservice) played the wrong note — e.g.hz_to_index(440)returned 2, which the firmware folds to ~466 Hz (A♯4), a semitone sharp of A4.This replaces it with the inverse of the firmware's quarter-tone scale (
Freq(idx) = 13.75 · 2^(idx/24)):Landmarks verified against
BUZZER_MUSIC_PROTOCOL_REFERENCE.md§4:440→120(A4 exact),880→144(octave = 24 steps),400→117,6200→212,12000→234.Hz-based input surface is unchanged (targeted correctness fix, not an API redesign); no wire-format or folding changes. Fix plan:
opendisplay-protocol/docs/buzzer-hz-index-fix-plan.md.Release note
Intended to ship in the same minor as #135 (melody notation) so downstream consumers bump their pin once.
🤖 Generated with Claude Code