fix(examples): Migrate buzzer tone() to hardware PWM Timer.#273
Conversation
nedseb
left a comment
nedseb
left a comment
There was a problem hiding this comment.
Review — PR #273
Très bon travail, rien à corriger.
Points positifs
- Migration propre vers Timer PWM, alignée avec metal_detector.py (#251)
- tone() est non-bloquant : le son joue pendant que la boucle continue de poller les boutons et le MENU
- finally: no_tone() garantit le silence à la sortie (Ctrl+C ou MENU)
- Tracking intelligent avec last_freq pour éviter les appels redondants à no_tone()
- Son continu tant qu'on maintient un bouton — comportement naturel pour un piano
Prêt à merger.
There was a problem hiding this comment.
Pull request overview
Migrates the dpad_piano.py example from a blocking, software bit-banged buzzer implementation to a hardware PWM-based approach using Timer 1, improving button responsiveness during note playback.
Changes:
- Replaces GPIO toggling (
sleep_usloop) with TIM1_CH4 PWM on theSPEAKERpin. - Introduces non-blocking
tone()andno_tone()helpers backed bypyb.Timer. - Updates the main loop to start/stop PWM based on current D-PAD input and ensure the buzzer is silenced on exit.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| last_note = None | ||
| last_octave = None | ||
| last_freq = None | ||
|
|
||
| try: | ||
| while True: | ||
| # MENU button is a direct MCU pin, not an MCP23009E pin | ||
| if MENU_BUTTON.value() == 0: | ||
| print("Menu button pressed, exiting piano.") | ||
| wait_menu_release() | ||
| break | ||
|
|
||
| pressed = get_pressed_buttons() | ||
| frequency, note_name, octave = select_frequency(pressed) | ||
|
|
||
| if pressed: | ||
| frequency, note_name, octave = select_frequency(pressed) | ||
|
|
||
| if note_name != last_note or octave != last_octave: | ||
| print( | ||
| "Playing:", | ||
| note_name, | ||
| "-", | ||
| frequency, | ||
| "Hz", | ||
| "(" + octave + " octave)", | ||
| ) | ||
| last_note = note_name | ||
| last_octave = octave | ||
|
|
||
| tone(SPEAKER, frequency, 60) | ||
| else: | ||
| if frequency == 0: | ||
| if last_freq != 0: | ||
| no_tone() | ||
| last_note = None | ||
| last_octave = None | ||
| last_freq = 0 | ||
| sleep_ms(20) |
There was a problem hiding this comment.
last_freq is initialized to None but later compared to 0, which causes an extra no_tone() call on the first loop iteration even before any note is played. Consider initializing last_freq to 0 (or removing last_freq entirely and calling no_tone() unconditionally when frequency == 0, since it is idempotent).
| def tone(freq): | ||
| """Start a tone using hardware PWM.""" | ||
| if freq <= 0: | ||
| buzzer_ch.pulse_width_percent(0) |
There was a problem hiding this comment.
In tone(), the freq <= 0 path directly sets buzzer_ch.pulse_width_percent(0) instead of reusing no_tone(). Calling no_tone() here would keep the silencing logic in one place (and makes future changes less error-prone).
| buzzer_ch.pulse_width_percent(0) | |
| no_tone() |
|
Les deux remarques de Copilot ont été corrigées dans ef28b74 :
C'étaient des corrections rapides et évidentes, elles n'auraient pas dû rester en attente. |
## [0.1.3](v0.1.2...v0.1.3) (2026-03-28) ### Bug Fixes * **examples:** Migrate buzzer tone() to hardware PWM Timer. ([#273](#273)) ([54a46e2](54a46e2))
|
🎉 This PR is included in version 0.1.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Replace the blocking software-based
tone()implementation indpad_piano.pywith a hardware PWM-based approach using Timer 1.Closes #254
Changes
sleep_usloop) with hardware PWM (TIM1_CH4 on PA11 / SPEAKER)tone()andno_tone()functions using Timer PWMmetal_detector.pyexample (already using PWM)Checklist
ruff checkpasses<scope>: <Description.>format