Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/main/drivers/pwm_mapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,27 +212,27 @@ static bool checkPwmTimerConflicts(const timerHardware_t *timHw)
}

static void timerHardwareOverride(timerHardware_t * timer) {
// Never modify a beeper timer — beeperPwmInit() must find TIM_USE_BEEPER intact
if (timer->usageFlags & TIM_USE_BEEPER) {
return;
}
switch (timerOverrides(timer2id(timer->tim))->outputMode) {
case OUTPUT_MODE_MOTORS:
timer->usageFlags &= ~(TIM_USE_SERVO|TIM_USE_LED);
timer->usageFlags &= ~(TIM_USE_SERVO|TIM_USE_LED|TIM_USE_BEEPER);
timer->usageFlags |= TIM_USE_MOTOR;
break;
case OUTPUT_MODE_SERVOS:
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_LED);
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_LED|TIM_USE_BEEPER);
timer->usageFlags |= TIM_USE_SERVO;
break;
case OUTPUT_MODE_LED:
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO);
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_BEEPER);
timer->usageFlags |= TIM_USE_LED;
break;
case OUTPUT_MODE_PINIO:
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_LED);
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_LED|TIM_USE_BEEPER);
timer->usageFlags |= TIM_USE_PINIO;
break;
case OUTPUT_MODE_BEEPER:
timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_LED|TIM_USE_PINIO);
timer->usageFlags |= TIM_USE_BEEPER;
break;
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/main/drivers/pwm_mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ typedef struct {
const timerHardware_t * timServos[MAX_PWM_OUTPUTS];
} timMotorServoHardware_t;

// Output assignment types for MSP2_INAV_OUTPUT_ASSIGNMENT response
// LED outputs are not reported here; they are already identified by TIM_USE_LED
// in the MSP2_INAV_OUTPUT_MAPPING_EXT2 usageFlags response.
#define OUTPUT_ASSIGNMENT_TYPE_MOTOR 1
#define OUTPUT_ASSIGNMENT_TYPE_SERVO 2
// MSP2_INAV_OUTPUT_ASSIGNMENT type byte: bit index of the TIM_USE_* flag.
// Matches the JavaScript TIM_USE_* constants in outputMapping.js (which are bit indices).
// Use __builtin_ctz(TIM_USE_x) to derive these from the bit-mask definitions in timer.h.
#endif // SITL_BUILD

bool pwmMotorAndServoInit(void);
Expand Down
23 changes: 23 additions & 0 deletions src/main/drivers/sound_beeper.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,29 @@ void beeperInit(const beeperDevConfig_t *config)
#if !defined(BEEPER)
UNUSED(config);
#else
// Runtime output assignment: scan for any pad explicitly set to OUTPUT_MODE_BEEPER.
// pwmBuildTimerOutputList() runs before beeperInit(), so TIM_USE_BEEPER is already
// set on the runtime-assigned pad by the time we get here.
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;
}
Comment on lines +84 to +91

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

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

}

// Skip compile-time beeper pad if the user has overridden it to another function.
for (int idx = 0; idx < timerHardwareCount; idx++) {
if (timerHardware[idx].tag == config->ioTag) {
if (!(timerHardware[idx].usageFlags & TIM_USE_BEEPER)) {
return;
}
break;
}
}

beeperIO = IOGetByTag(config->ioTag);
beeperInverted = config->isInverted;

Expand Down
5 changes: 4 additions & 1 deletion src/main/fc/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ static const char * outputModeNames[] = {
"SERVOS",
"LED",
"PINIO",
"BEEPER",
NULL
};

Expand Down Expand Up @@ -3225,6 +3226,8 @@ static void cliTimerOutputMode(char *cmdline)
mode = OUTPUT_MODE_LED;
} else if(!sl_strcasecmp("PINIO", tok)) {
mode = OUTPUT_MODE_PINIO;
} else if(!sl_strcasecmp("BEEPER", tok)) {
mode = OUTPUT_MODE_BEEPER;
} else {
cliShowParseError();
return;
Expand Down Expand Up @@ -5037,7 +5040,7 @@ const clicmd_t cmdTable[] = {
#ifdef USE_OSD
CLI_COMMAND_DEF("osd_layout", "get or set the layout of OSD items", "[<layout> [<item> [<col> <row> [<visible>]]]]", cliOsdLayout),
#endif
CLI_COMMAND_DEF("timer_output_mode", "get or set the outputmode for a given timer.", "[<timer> [<AUTO|MOTORS|SERVOS|LED|PINIO>]]", cliTimerOutputMode),
CLI_COMMAND_DEF("timer_output_mode", "get or set the outputmode for a given timer.", "[<timer> [<AUTO|MOTORS|SERVOS|LED|PINIO|BEEPER>]]", cliTimerOutputMode),
};

static void cliHelp(char *cmdline)
Expand Down
24 changes: 20 additions & 4 deletions src/main/fc/fc_msp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1812,14 +1812,22 @@ static bool mspFcProcessOutCommand(uint16_t cmdMSP, sbuf_t *dst, mspPostProcessF
const timMotorServoHardware_t *hw = pwmGetOutputAssignment();
for (int m = 0; m < hw->maxTimMotorCount; m++) {
sbufWriteU8(dst, (uint8_t)(hw->timMotors[m] - timerHardware));
sbufWriteU8(dst, OUTPUT_ASSIGNMENT_TYPE_MOTOR);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_MOTOR));
sbufWriteU8(dst, (uint8_t)(m + 1));
}
for (int s = 0; s < hw->maxTimServoCount; s++) {
sbufWriteU8(dst, (uint8_t)(hw->timServos[s] - timerHardware));
sbufWriteU8(dst, OUTPUT_ASSIGNMENT_TYPE_SERVO);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_SERVO));
sbufWriteU8(dst, (uint8_t)(s + 1));
}
for (int idx = 0; idx < timerHardwareCount; idx++) {
if (timerOverrides(timer2id(timerHardware[idx].tim))->outputMode == OUTPUT_MODE_BEEPER) {
sbufWriteU8(dst, (uint8_t)idx);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_BEEPER));
sbufWriteU8(dst, 1);
break;
}
}
}
break;
#endif
Expand Down Expand Up @@ -4911,14 +4919,22 @@ bool mspFCProcessInOutCommand(uint16_t cmdMSP, sbuf_t *dst, sbuf_t *src, mspResu

for (int m = 0; m < tempOut.maxTimMotorCount; m++) {
sbufWriteU8(dst, (uint8_t)(tempOut.timMotors[m] - timerHardware));
sbufWriteU8(dst, OUTPUT_ASSIGNMENT_TYPE_MOTOR);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_MOTOR));
sbufWriteU8(dst, (uint8_t)(m + 1));
}
for (int s = 0; s < tempOut.maxTimServoCount; s++) {
sbufWriteU8(dst, (uint8_t)(tempOut.timServos[s] - timerHardware));
sbufWriteU8(dst, OUTPUT_ASSIGNMENT_TYPE_SERVO);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_SERVO));
sbufWriteU8(dst, (uint8_t)(s + 1));
}
for (int idx = 0; idx < timerHardwareCount; idx++) {
if (proposedModes[timer2id(timerHardware[idx].tim)] == OUTPUT_MODE_BEEPER) {
sbufWriteU8(dst, (uint8_t)idx);
sbufWriteU8(dst, __builtin_ctz(TIM_USE_BEEPER));
sbufWriteU8(dst, 1);
break;
}
}
*ret = MSP_RESULT_ACK;
}
break;
Expand Down
3 changes: 2 additions & 1 deletion src/main/flight/mixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ typedef enum {
OUTPUT_MODE_MOTORS,
OUTPUT_MODE_SERVOS,
OUTPUT_MODE_LED,
OUTPUT_MODE_PINIO
OUTPUT_MODE_PINIO,
OUTPUT_MODE_BEEPER
} outputMode_e;

typedef struct motorAxisCorrectionLimits_s {
Expand Down
Loading