Add BUZZER as a runtime-assignable timer output mode#11675
Open
sensei-hacker wants to merge 3 commits into
Open
Add BUZZER as a runtime-assignable timer output mode#11675sensei-hacker wants to merge 3 commits into
sensei-hacker wants to merge 3 commits into
Conversation
Extends the unified PWM/PINIO output system so any timer-capable pad can be assigned as a PWM buzzer at runtime via the output assignment UI, instead of being fixed to the compile-time BEEPER pin in target.h. - Add OUTPUT_MODE_BEEPER to outputMode_e enum (mixer.h) - timerHardwareOverride(): set TIM_USE_BEEPER and clear conflicting flags when OUTPUT_MODE_BEEPER is assigned, preventing double-allocation - beeperInit(): scan timerOverrides for OUTPUT_MODE_BEEPER first; if found, call beeperPwmInit() on that pad and return — falls back to compile-time BEEPER pin when no runtime assignment is present - MSP2_INAV_OUTPUT_ASSIGNMENT / MSP2_INAV_QUERY_OUTPUT_ASSIGNMENT: report the runtime-assigned buzzer pad as OUTPUT_ASSIGNMENT_TYPE_BUZZER (3) - CLI timer_output_mode: add BEEPER as a valid mode name
…NT type byte Replace the separate OUTPUT_ASSIGNMENT_TYPE_* numbering system with __builtin_ctz(TIM_USE_*) so the type byte in the 3-tuple response matches the TIM_USE_* bit-index constants already defined in the JS configurator. This aligns firmware and configurator on one consistent system rather than two parallel sets of constants.
Remove the early-return guard that permanently locked TIM_USE_BEEPER pads out of timerHardwareOverride(). Add TIM_USE_BEEPER to the clear masks of all non-beeper cases so a reassigned pad cannot carry both flags at once. In beeperInit(), skip the compile-time fallback when the pad's post-override usageFlags no longer have TIM_USE_BEEPER set. Compile-time beeper pads now default to beeper (AUTO mode = no-op) but can be overridden to motor/servo/etc. like any other timer-capable pad.
Contributor
PR Summary by QodoAdd BUZZER as a runtime-assignable timer output mode
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Contributor
Code Review by Qodo
Context used 1. Wrong beeper pad chosen
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+84
to
+91
| for (int idx = 0; idx < timerHardwareCount; idx++) { | ||
| const timerHardware_t *timHw = &timerHardware[idx]; | ||
| if (timerOverrides(timer2id(timHw->tim))->outputMode == OUTPUT_MODE_BEEPER) { | ||
| beeperPwmInit(timHw->tag, BEEPER_PWM_FREQUENCY); | ||
| beeperConfigMutable()->pwmMode = true; | ||
| systemBeep(false); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
1. Wrong beeper pad chosen 🐞 Bug ≡ Correctness
beeperInit() selects the first timerHardware entry whose *timer* override is OUTPUT_MODE_BEEPER and initializes PWM beeper on that entry’s tag, so on timers with multiple exposed channels it can initialize/report the buzzer on the wrong pad and the buzzer may not sound on the user-intended pin. MSP2_INAV_OUTPUT_ASSIGNMENT mirrors this by also reporting only the first matching timerHardware index, potentially mislabeling the buzzer output in the UI.
Agent Prompt
## Issue description
Runtime buzzer selection is currently keyed off `timerOverrides(timer2id(timHw->tim))->outputMode == OUTPUT_MODE_BEEPER`, but `timerOverrides` is per **timer instance** (TIMx), not per **timer channel/pad**. On timers with multiple pads, this makes the chosen buzzer pad dependent on `timerHardware[]` order, which can differ from the pad the user expects.
## Issue Context
- `timer2id()` maps only by `tim` pointer, so all channels on the same timer share one override.
- `beeperInit()` breaks at the first `timerHardware[]` entry whose timer is in `OUTPUT_MODE_BEEPER`.
- MSP2 output assignment similarly reports only the first matching hardware index.
## Fix Focus Areas
- src/main/drivers/sound_beeper.c[81-103]
- src/main/fc/fc_msp.c[1823-1830]
### Implementation direction
- Introduce a way to select the beeper **pad** (timerHardware index or ioTag) rather than only selecting the timer:
- Option A (best): add a small config field (PG) like `ioTag_t runtimeBeeperTag` set by MSP/CLI/UI, and use that tag for `beeperPwmInit()`.
- Option B (minimal guard): if `OUTPUT_MODE_BEEPER` is set on a timer with multiple `timerHardware[]` entries, either:
- refuse it and log an error, or
- pick a deterministic channel (e.g., prefer the one that was compile-time BEEPER tag if present; otherwise require explicit tag).
- Update MSP2 output-assignment reporting to match the selected beeper tag/index (and avoid reporting a misleading first entry).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+86
to
+91
| if (timerOverrides(timer2id(timHw->tim))->outputMode == OUTPUT_MODE_BEEPER) { | ||
| beeperPwmInit(timHw->tag, BEEPER_PWM_FREQUENCY); | ||
| beeperConfigMutable()->pwmMode = true; | ||
| systemBeep(false); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
2. Pwm beeper init unchecked 🐞 Bug ☼ Reliability
In the runtime-assigned beeper path, beeperInit() sets pwmMode=true and returns without verifying that beeperPwmInit() succeeded; if PWM allocation fails, pwmWriteBeeper() becomes a no-op and the compile-time beeper fallback is skipped, disabling beeps silently. This creates a brittle failure mode when timer channel allocation fails (e.g., no TCH available).
Agent Prompt
## Issue description
The runtime `OUTPUT_MODE_BEEPER` path unconditionally commits to PWM mode (`beeperConfigMutable()->pwmMode = true`) and returns after calling `beeperPwmInit()`, but `beeperPwmInit()` can fail and leave the beeper backend uninitialized. In that case `pwmWriteBeeper()` no-ops and there is no fallback.
## Issue Context
- `beeperPwmInit()` can return early when `timerGetTCH()` fails.
- `pwmWriteBeeper()` returns immediately when `beeperPwm == NULL`.
## Fix Focus Areas
- src/main/drivers/sound_beeper.c[81-91]
### Implementation direction
- Make `beeperPwmInit()` report success (e.g., return `bool`), or add a small getter like `bool beeperPwmIsInitialized(void)`.
- In the runtime assignment path:
- only set `pwmMode=true` and `return` if initialization succeeded;
- otherwise continue into the existing compile-time beeper initialization path (or log an error and keep beeper in GPIO mode if possible).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Test firmware build ready — commit Download firmware for PR #11675 238 targets built. Find your board's
|
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
1 participant
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
Extends the unified PWM/PINIO output system (#11375, #11564) so any timer-capable pad can be assigned as a PWM buzzer at runtime via the output assignment UI, instead of being fixed to the compile-time
BEEPERpin intarget.h. Companion firmware PR to inav-configurator#2654.Changes
flight/mixer.h— addOUTPUT_MODE_BEEPERtooutputMode_edrivers/pwm_mapping.c/pwm_mapping.h—timerHardwareOverride()setsTIM_USE_BEEPERand clears conflicting flags whenOUTPUT_MODE_BEEPERis assigned, preventing double-allocation; also removes the guard that permanently locked compile-timeTIM_USE_BEEPERpads out of reassignment (clearingTIM_USE_BEEPERfrom all non-beeper cases so a reassigned pad can't carry both flags)drivers/sound_beeper.c—beeperInit()scanstimerOverridesforOUTPUT_MODE_BEEPERfirst; if found, initializes that pad as PWM beeper and returns. Falls back to compile-time#define BEEPER+ existing behavior when no runtime assignment is present. Skips the compile-time fallback when a compile-time beeper pad has been reassigned away fromTIM_USE_BEEPER.fc/fc_msp.c—MSP2_INAV_OUTPUT_ASSIGNMENT/MSP2_INAV_QUERY_OUTPUT_ASSIGNMENT: replace the literalOUTPUT_ASSIGNMENT_TYPE_MOTOR/SERVOnumbering with__builtin_ctz(TIM_USE_*)bit-index values, and extend the handler to also report LED/BEEPER/PINIO direct assignments (previously motor/servo only). This aligns the wire type byte with theTIM_USE_*bit-index constants already used in the JS configurator.fc/cli.c—timer_output_modeCLI command: addBEEPERas a valid mode nameBackward compatibility
Targets with existing
#define BEEPER+ old-style compile-time PWM beeper config continue to work unchanged when no runtime BUZZER assignment exists — runtime assignment only takes precedence when explicitly set.Note on the MSP type-byte change
This changes the type byte encoding for the existing MOTOR/SERVO entries too (from literal
1/2to bit-index2/3), not just adding new types. inav-configurator#2654 was written against this new encoding. Both targetmaintenance-10.x/INAV 10.0 (unreleased), so there's no production population affected — but the two PRs should land close together to avoid a stale Output Mapping table for anyone testingmaintenance-10.xconfigurator againstmaintenance-10.xfirmware in the gap between merges.Testing
test-engineerthen verified live MSP/DOM state against the running FC — Output Mapping table correctly reports the buzzer pad asS14/Buzzer, timer dropdown pre-selectsBEEPER, function row showsBuzzer, and Motor 1–4 assignments are unaffected. This confirms the runtime assignment mechanism and MSP reporting are correct end-to-end on real hardware.Related
claude/projects/active/feature-buzzer-unified-output/