From be178df28f71f6cc4ced72b559c49217d26eb168 Mon Sep 17 00:00:00 2001 From: David Lee <247393336+davelee98@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:46:58 -0400 Subject: [PATCH] fix: correct hz_to_index to firmware quarter-tone scale 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) --- src/opendisplay/models/buzzer_activate.py | 25 +++++++-- tests/unit/test_models_buzzer_activate.py | 62 +++++++++++++++++++---- 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/opendisplay/models/buzzer_activate.py b/src/opendisplay/models/buzzer_activate.py index ff631ac..82f521e 100644 --- a/src/opendisplay/models/buzzer_activate.py +++ b/src/opendisplay/models/buzzer_activate.py @@ -2,18 +2,33 @@ from __future__ import annotations +import math from dataclasses import dataclass -_MIN_HZ = 400 -_MAX_HZ = 12000 +_ANCHOR_HZ = 13.75 +_STEPS_PER_OCTAVE = 24 _DURATION_UNIT_MS = 5 def hz_to_index(hz: int) -> int: - """Convert frequency in Hz to firmware tone index (0-255). 0 Hz → silence.""" + """Convert Hz to firmware quarter-tone index (0-255). 0/negative Hz -> 0 (silence). + + Inverse of the firmware scale Freq(idx) = 13.75 * 2**(idx/24) Hz. + + Examples: + idx 1 -> ~14.15 Hz (nAm1p, the bottom note in the table) + idx 120 -> 440.00 Hz (nA4, concert pitch A) + idx 255 -> ~21714.33 Hz (nE10p, the top note in the table) + + Indices outside the firmware's playable window [117, 234] are octave-folded + by the firmware itself (preserving pitch class) before being driven onto the + speaker -- this protects the buzzer hardware from being driven outside its + safe operating range. This helper only produces a valid 0-255 index; it does + not need to replicate that folding. + """ if hz <= 0: return 0 - idx = round(1 + (hz - _MIN_HZ) * 254 / (_MAX_HZ - _MIN_HZ)) + idx = round(_STEPS_PER_OCTAVE * math.log2(hz / _ANCHOR_HZ)) return max(1, min(255, idx)) @@ -26,7 +41,7 @@ def ms_to_units(ms: int) -> int: class BuzzerStep: """A single tone step: one frequency for one duration.""" - frequency_index: int # 0=silence, 1–255 → 400–12000 Hz + frequency_index: int # 0=silence; 1–255 → quarter-tone note, Freq = 13.75 * 2**(idx/24) Hz duration_units: int # ×5 ms each; range 1–255 diff --git a/tests/unit/test_models_buzzer_activate.py b/tests/unit/test_models_buzzer_activate.py index 59fb459..f39729f 100644 --- a/tests/unit/test_models_buzzer_activate.py +++ b/tests/unit/test_models_buzzer_activate.py @@ -2,6 +2,8 @@ from __future__ import annotations +import pytest + from opendisplay.models.buzzer_activate import ( BuzzerActivateConfig, BuzzerPattern, @@ -18,24 +20,56 @@ def test_silence_at_zero(self): def test_silence_at_negative(self): assert hz_to_index(-100) == 0 - def test_minimum_frequency(self): - assert hz_to_index(400) == 1 + def test_low_landmark_frequency(self): + # 400 Hz maps to index 117 (bottom of the firmware's playable window) + assert hz_to_index(400) == 117 - def test_maximum_frequency(self): - assert hz_to_index(12000) == 255 + def test_high_landmark_frequency(self): + # 12000 Hz maps to index 234 (top of the firmware's playable window) + assert hz_to_index(12000) == 234 - def test_midpoint_frequency(self): - # 6200 Hz is the midpoint → index ~128 - idx = hz_to_index(6200) - assert 126 <= idx <= 130 + def test_6200_hz(self): + assert hz_to_index(6200) == 212 def test_clamps_above_max(self): assert hz_to_index(99999) == 255 - def test_clamps_below_min_non_zero(self): - # Values between 1 and 399 Hz should still produce index 1 (not 0) + def test_low_non_zero(self): + # Any positive Hz below the anchor still produces at least index 1 (not 0) assert hz_to_index(1) == 1 - assert hz_to_index(399) == 1 + # 399 Hz is not "below min" any more; it maps to 117 like 400 Hz + assert hz_to_index(399) == 117 + + def test_concert_a4(self): + # Reference doc §4.2: nA4 = 440 Hz exactly at idx 120 + assert hz_to_index(440) == 120 + + def test_a5_octave(self): + # nA5 = 880 Hz at idx 144; one octave up is exactly +24 quarter-tones + assert hz_to_index(880) == 144 + assert hz_to_index(880) - hz_to_index(440) == 24 + + def test_landmark_1000_hz(self): + assert hz_to_index(1000) == 148 + + def test_landmark_2000_hz(self): + assert hz_to_index(2000) == 172 + + def test_landmark_523_hz(self): + assert hz_to_index(523) == 126 + + def test_landmark_11840_hz(self): + assert hz_to_index(11840) == 234 + + def test_landmark_21714_hz(self): + assert hz_to_index(21714) == 255 + + @pytest.mark.parametrize("idx", [117, 120, 126, 144, 148, 172, 212, 234]) + def test_round_trip(self, idx): + # Feeding the firmware's frequency for an index back through hz_to_index + # recovers that index. + hz = round(13.75 * 2 ** (idx / 24)) + assert hz_to_index(hz) == idx class TestMsToUnits: @@ -86,6 +120,12 @@ def test_single_tone_wire_format(self): assert data[2] == 1 # 1 step assert len(data) == 5 + def test_single_tone_a4_wire_bytes(self): + # Reference doc §7.1 worked example: 440 Hz / 200 ms -> freq_idx 120 (0x78), + # dur_units 40 (0x28). + config = BuzzerActivateConfig.single_tone(frequency_hz=440, duration_ms=200) + assert config.to_bytes() == bytes([1, 1, 1, 120, 40]) + def test_silence_tone(self): config = BuzzerActivateConfig.single_tone(frequency_hz=0, duration_ms=50) data = config.to_bytes()