From f370c44f1997ff5adc95f0e6a8ddea21cb6ef014 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 26 Jul 2026 02:01:28 -0500 Subject: [PATCH 1/3] Fix LED output mode falsely flagging sibling timer channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit timerHardwareOverride() applied TIM_USE_LED to every pad sharing a timer set to OUTPUT_MODE_LED, but ws2811LedStripInit() only ever wires up the first matching pad — all channels of one timer share a single period register, so only one can actually be the WS2811 output. This caused sibling pads (e.g. S6 on a timer with S5 as the LED strip) to be reported as "Led" over MSP and shown as such in the configurator Mixer tab while being functionally dead. Confirmed on hardware via bootlog: before the fix, a two-pad shared timer ended with both pads flagged TIM_USE_LED even though only one is ever driven. Track which pad is canonical (first-encountered, matching the scan order ws2811LedStripInit()'s timerGetByUsageFlag() uses) per physical timer; only that pad gets TIM_USE_LED, mirroring the existing BEEPER canonical-pad tracking. Siblings fall back to TIM_USE_PINIO (GPIO on/off), same as BEEPER siblings. A second, subtler bug had to be fixed for this to actually work: pwmAssignOutput()'s MAP_TO_LED_OUTPUT case called pwmClaimTimer(), which propagates the assigned pad's flags to every sibling sharing the same physical timer. That propagation is correct for motor/servo (a multi-channel timer should offer multiple outputs) but was silently re-overwriting the sibling's correct TIM_USE_PINIO back to TIM_USE_LED immediately after the canonical-pad logic had set it correctly. Removed the pwmClaimTimer() call from the LED case; motor/servo keep it. --- src/main/drivers/pwm_mapping.c | 31 +- src/test/unit/pwm_mapping_led_e2e_unittest.cc | 789 ++++++++++++++++++ src/test/unit/pwm_mapping_led_unittest.cc | 705 ++++++++++++++++ 3 files changed, 1520 insertions(+), 5 deletions(-) create mode 100644 src/test/unit/pwm_mapping_led_e2e_unittest.cc create mode 100644 src/test/unit/pwm_mapping_led_unittest.cc diff --git a/src/main/drivers/pwm_mapping.c b/src/main/drivers/pwm_mapping.c index 23b5c596111..44a33220c20 100644 --- a/src/main/drivers/pwm_mapping.c +++ b/src/main/drivers/pwm_mapping.c @@ -211,7 +211,7 @@ static bool checkPwmTimerConflicts(const timerHardware_t *timHw) return false; } -static void timerHardwareOverride(timerHardware_t * timer, bool isCanonicalBeeperPad) { +static void timerHardwareOverride(timerHardware_t * timer, bool isCanonicalBeeperPad, bool isCanonicalLedPad) { switch (timerOverrides(timer2id(timer->tim))->outputMode) { case OUTPUT_MODE_MOTORS: timer->usageFlags &= ~(TIM_USE_SERVO|TIM_USE_LED|TIM_USE_BEEPER); @@ -222,8 +222,17 @@ static void timerHardwareOverride(timerHardware_t * timer, bool isCanonicalBeepe timer->usageFlags |= TIM_USE_SERVO; break; case OUTPUT_MODE_LED: - timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_BEEPER); - timer->usageFlags |= TIM_USE_LED; + // Other channels of this timer share its period register, so they can't + // be repurposed as motor/servo/BEEPER either. Only the first (canonical) + // pad is the one ws2811LedStripInit() actually wires up; siblings can still + // drive a GPIO-only PINIO (binary on/off, no PWM), since that doesn't need + // the timer's fixed WS2811-bitrate period. + timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_BEEPER|TIM_USE_LED); + timer->usageFlags |= TIM_USE_PINIO; + if (isCanonicalLedPad) { + timer->usageFlags &= ~TIM_USE_PINIO; + timer->usageFlags |= TIM_USE_LED; + } break; case OUTPUT_MODE_PINIO: timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_LED|TIM_USE_BEEPER); @@ -292,8 +301,14 @@ static void pwmAssignOutput(timMotorServoHardware_t *timOutputs, timerHardware_t pwmClaimTimer(timHw->tim, timHw->usageFlags); break; case MAP_TO_LED_OUTPUT: + // Unlike motor/servo, do NOT pwmClaimTimer() here: that propagates + // this pad's flags to every sibling pad sharing the physical timer, + // which is correct for motor/servo (a multi-channel timer should + // offer multiple motor/servo outputs) but wrong for LED — only this + // one canonical pad is ever driven, and propagating TIM_USE_LED would + // overwrite siblings' correct TIM_USE_PINIO/BEEPER (from the earlier + // canonical-pad pass in timerHardwareOverride()) back to a false LED flag. timHw->usageFlags &= TIM_USE_LED; - pwmClaimTimer(timHw->tim, timHw->usageFlags); break; default: break; @@ -327,6 +342,7 @@ void pwmBuildTimerOutputList(timMotorServoHardware_t *timOutputs) // Apply all timerOverrides upfront so flag state is stable for both passes bool beeperClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; + bool ledClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; for (int idx = 0; idx < timerHardwareCount; idx++) { timerHardware_t *timHw = &timerHardware[idx]; uint8_t timId = timer2id(timHw->tim); @@ -335,7 +351,12 @@ void pwmBuildTimerOutputList(timMotorServoHardware_t *timOutputs) beeperClaimed[timId] = true; isCanonicalBeeperPad = true; } - timerHardwareOverride(timHw, isCanonicalBeeperPad); + bool isCanonicalLedPad = false; + if (timerOverrides(timId)->outputMode == OUTPUT_MODE_LED && !ledClaimed[timId]) { + ledClaimed[timId] = true; + isCanonicalLedPad = true; + } + timerHardwareOverride(timHw, isCanonicalBeeperPad, isCanonicalLedPad); } // Assign outputs in priority order: dedicated first, then auto. diff --git a/src/test/unit/pwm_mapping_led_e2e_unittest.cc b/src/test/unit/pwm_mapping_led_e2e_unittest.cc new file mode 100644 index 00000000000..56f981ccfac --- /dev/null +++ b/src/test/unit/pwm_mapping_led_e2e_unittest.cc @@ -0,0 +1,789 @@ +/* + * End-to-end unit test: pwmBuildTimerOutputList()'s FULL two-loop pipeline + * (canonical-pad override loop + priority-ordered assignment loop) must not + * let a sibling pad on a shared LED timer end up flagged TIM_USE_LED. + * + * This is a companion to pwm_mapping_led_unittest.cc, which only exercises + * timerHardwareOverride() in isolation (the first loop). That is NOT enough + * to catch the real bug found via hardware bootlog tracing on a SPEDIXF405 + * (TIM4, S5+S6 shared): timerHardwareOverride() correctly set + * S5=TIM_USE_LED, S6=TIM_USE_PINIO after the first (canonical-claim) loop, + * but pwmAssignOutput()'s MAP_TO_LED_OUTPUT case, run later in the SECOND + * (assignment) loop, used to call pwmClaimTimer(timHw->tim, timHw->usageFlags) + * after assigning the canonical pad. pwmClaimTimer() propagates the + * just-assigned pad's usageFlags to EVERY sibling pad sharing the same + * physical timer — correct for MOTOR/SERVO (a multi-channel motor timer + * really should offer multiple motor outputs), but wrong for LED, since it + * silently overwrote the sibling's correct TIM_USE_PINIO back to + * TIM_USE_LED. This reintroduced the "sibling pad falsely reported as LED" + * bug via a different, later code path than the one originally fixed in + * timerHardwareOverride(). + * + * Fix (already applied to src/main/drivers/pwm_mapping.c, NOT modified by + * this test file): pwmAssignOutput()'s MAP_TO_LED_OUTPUT case no longer + * calls pwmClaimTimer() — it only masks timHw->usageFlags &= TIM_USE_LED. + * The MAP_TO_MOTOR_OUTPUT / MAP_TO_SERVO_OUTPUT cases are UNCHANGED and + * still call pwmClaimTimer() to propagate flags to sibling pads. + * + * This file contains: + * 1. Inline, faithful reproductions of ALL THREE pieces working together, + * end to end: + * - timerHardwareOverride() (fixed version, with isCanonicalBeeperPad + * / isCanonicalLedPad canonical-pad tracking) + * - pwmClaimTimer() (exact real logic: propagate-usageFlags-to- + * siblings-sharing-a-timer) + * - pwmAssignOutput() in BOTH its FIXED form (current source: LED + * case does NOT call pwmClaimTimer) and its BUGGY/pre-fix form (LED + * case DOES call pwmClaimTimer, exactly like MOTOR/SERVO), so the + * test can demonstrate the bug reproduces against the buggy form + * and is fixed against the current form. + * pwm_mapping.c is excluded from the SITL/host unit-test build (guarded + * by `#if !defined(SITL_BUILD)`), and depends on PG (parameter-group) + * infrastructure not available in a host unit build, so it cannot be + * linked directly — same constraint as pwm_mapping_led_unittest.cc and + * pwm_mapping_beeper_unittest.cc. + * 2. A minimal simulateBuildTimerOutputList() that reproduces + * pwmBuildTimerOutputList()'s two-loop structure: the canonical-claim + * loop, then the priority-ordered (dedicated pass, then auto pass) + * assignment loop, with checkPwmTimerConflicts() stubbed to always + * return false (matching the real hardware trace where + * FEATURE_LED_STRIP was off). + * 3. TEST SUITE LedSiblingE2E — the primary regression test. Two pads + * share one physical timer set to LED mode. After running the FULL + * simulated pipeline (both loops): + * - with the BUGGY pwmAssignOutput (pre-fix): the sibling pad ends up + * incorrectly flagged TIM_USE_LED too (bug reproduction). + * - with the FIXED pwmAssignOutput (current source): the canonical + * pad keeps TIM_USE_LED and the sibling correctly falls back to + * TIM_USE_PINIO, NOT TIM_USE_LED. + * 4. TEST SUITE MotorSiblingPropagationRegression — the inverse/regression + * guard: pwmClaimTimer()'s propagation-to-siblings behavior must still + * happen for MAP_TO_MOTOR_OUTPUT (this fix must not have been + * accidentally broadened to remove propagation for motor/servo too). + * 5. TEST SUITE SourceSync — reads the ACTUAL LIVE + * src/main/drivers/pwm_mapping.c off disk at test time and asserts that + * pwmAssignOutput()'s MAP_TO_LED_OUTPUT case does NOT contain a + * pwmClaimTimer() call, while MAP_TO_MOTOR_OUTPUT / MAP_TO_SERVO_OUTPUT + * still do. This exists because pwm_mapping_beeper_unittest.cc was + * previously found to have drifted silently for months after upstream + * logic changed (see pwm_mapping_led_unittest.cc's SourceSync suite) — + * an inline reproduction is unavoidable here, but it must not be + * allowed to silently go stale. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "unittest_macros.h" + +/* ------------------------------------------------------------------------- + * Minimal type/flag definitions — mirrors the real firmware headers on this + * branch (src/main/drivers/timer.h, src/main/flight/mixer.h, + * src/main/drivers/pwm_mapping.c's internal MAP_TO_* enum). + * ------------------------------------------------------------------------- */ + +typedef enum { + TIM_USE_ANY = 0, + TIM_USE_PPM = (1 << 0), + TIM_USE_PWM = (1 << 1), + TIM_USE_MOTOR = (1 << 2), + TIM_USE_SERVO = (1 << 3), + TIM_USE_LED = (1 << 24), + TIM_USE_BEEPER = (1 << 25), + TIM_USE_PINIO = (1 << 26), +} timerUsageFlag_e; + +#define TIM_IS_MOTOR(flags) ((flags) & (uint32_t)TIM_USE_MOTOR) +#define TIM_IS_SERVO(flags) ((flags) & (uint32_t)TIM_USE_SERVO) +#define TIM_IS_LED(flags) ((flags) & (uint32_t)TIM_USE_LED) + +typedef enum { + OUTPUT_MODE_AUTO = 0, + OUTPUT_MODE_MOTORS, + OUTPUT_MODE_SERVOS, + OUTPUT_MODE_LED, + OUTPUT_MODE_PINIO, + OUTPUT_MODE_BEEPER +} outputMode_e; + +/* Mirrors the anonymous enum at the top of pwm_mapping.c */ +enum { + MAP_TO_NONE, + MAP_TO_MOTOR_OUTPUT, + MAP_TO_SERVO_OUTPUT, + MAP_TO_LED_OUTPUT +}; + +/* Minimal timer hardware entry. `timId` stands in for the real code's + * timer2id(timer->tim) / the `tim` pointer identity itself — in the real + * driver, multiple timerHardware[] pads sharing the same physical timer + * compare equal on timer2id(tim); here we just store the id directly and use + * it as the "same physical timer" key everywhere the real code would compare + * `->tim` pointers (including inside the simulated pwmClaimTimer()). */ +typedef struct timerHardware_s { + uint32_t usageFlags; + uint8_t timId; +} timerHardware_t; + +static const int MAX_PADS = 16; +static const int MAX_TIMID = 16; + +/* ------------------------------------------------------------------------- + * Piece 1: timerHardwareOverride() — literal translation of the current + * (fixed) src/main/drivers/pwm_mapping.c. Identical to the copy in + * pwm_mapping_led_unittest.cc / pwm_mapping_beeper_unittest.cc; duplicated + * here because each *_unittest.cc becomes its own standalone executable + * (see src/test/unit/CMakeLists.txt's `file(GLOB TEST_PROGRAMS *_unittest.cc)` + * + one add_executable() per file), so statics cannot be shared across files. + * ------------------------------------------------------------------------- */ +static void timerHardwareOverride_fixed(timerHardware_t *timer, + outputMode_e outputMode, + bool isCanonicalBeeperPad, + bool isCanonicalLedPad) +{ + switch (outputMode) { + case OUTPUT_MODE_MOTORS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; + break; + case OUTPUT_MODE_SERVOS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_SERVO; + break; + case OUTPUT_MODE_LED: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_BEEPER | TIM_USE_LED); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalLedPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_LED; + } + break; + case OUTPUT_MODE_PINIO: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + break; + case OUTPUT_MODE_BEEPER: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalBeeperPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_BEEPER; + } + break; + default: + break; + } +} + +/* ------------------------------------------------------------------------- + * Piece 2: pwmClaimTimer() — exact real logic (propagate usageFlags to every + * pad sharing the same physical timer). Real signature is + * pwmClaimTimer(HAL_Timer_t *tim, uint32_t usageFlags) and it iterates the + * GLOBAL timerHardware[]/timerHardwareCount; here `pads`/`count` stand in for + * that global array, passed explicitly since this is a host unit test with + * no global timer table. + * ------------------------------------------------------------------------- */ +static uint8_t pwmClaimTimer_sim(timerHardware_t *pads, int count, uint8_t timId, uint32_t usageFlags) +{ + uint8_t changed = 0; + for (int idx = 0; idx < count; idx++) { + timerHardware_t *timHw = &pads[idx]; + if (timHw->timId == timId && timHw->usageFlags != usageFlags) { + timHw->usageFlags = usageFlags; + changed++; + } + } + return changed; +} + +/* Output-collection bookkeeping — stands in for timMotorServoHardware_t's + * timMotors[]/timServos[] arrays (pointers into timerHardware[] in the real + * code; here indices into `pads[]`). Note: the real timOutputs struct does + * NOT track LED pads at all — the LED strip driver finds its pad later via + * timerGetByUsageFlag(TIM_USE_LED) — so we don't track LED assignments here + * either; LED "assignment" is verified purely via the pad's final usageFlags, + * matching real-world observability. */ +struct SimOutputs { + int motorPadIdx[MAX_PADS]; + int motorCount; + int servoPadIdx[MAX_PADS]; + int servoCount; +}; + +static bool pwmHasMotorOnTimer_sim(const timerHardware_t *pads, const SimOutputs *outputs, uint8_t timId) +{ + for (int i = 0; i < outputs->motorCount; i++) { + if (pads[outputs->motorPadIdx[i]].timId == timId) { + return true; + } + } + return false; +} + +static bool pwmHasServoOnTimer_sim(const timerHardware_t *pads, const SimOutputs *outputs, uint8_t timId) +{ + for (int i = 0; i < outputs->servoCount; i++) { + if (pads[outputs->servoPadIdx[i]].timId == timId) { + return true; + } + } + return false; +} + +/* ------------------------------------------------------------------------- + * Piece 3: pwmAssignOutput() — provided in TWO forms: + * + * pwmAssignOutput_fixed() — literal translation of the CURRENT (fixed) + * src/main/drivers/pwm_mapping.c: MOTOR/SERVO + * cases call pwmClaimTimer_sim() to propagate + * flags to sibling pads; the LED case does NOT. + * + * pwmAssignOutput_buggy() — reproduces the PRE-FIX behavior: identical + * except the LED case ALSO calls + * pwmClaimTimer_sim(), exactly like MOTOR/SERVO + * used to (and still do). This is the exact bug + * found via hardware bootlog tracing. + * ------------------------------------------------------------------------- */ +static void pwmAssignOutput_fixed(SimOutputs *outputs, timerHardware_t *pads, int count, + timerHardware_t *timHw, int type) +{ + int padIdx = (int)(timHw - pads); + switch (type) { + case MAP_TO_MOTOR_OUTPUT: + timHw->usageFlags &= (uint32_t)TIM_USE_MOTOR; + outputs->motorPadIdx[outputs->motorCount++] = padIdx; + pwmClaimTimer_sim(pads, count, timHw->timId, timHw->usageFlags); + break; + case MAP_TO_SERVO_OUTPUT: + timHw->usageFlags &= (uint32_t)TIM_USE_SERVO; + outputs->servoPadIdx[outputs->servoCount++] = padIdx; + pwmClaimTimer_sim(pads, count, timHw->timId, timHw->usageFlags); + break; + case MAP_TO_LED_OUTPUT: + // FIX: no pwmClaimTimer_sim() call here — see file header comment. + timHw->usageFlags &= (uint32_t)TIM_USE_LED; + break; + default: + break; + } +} + +static void pwmAssignOutput_buggy(SimOutputs *outputs, timerHardware_t *pads, int count, + timerHardware_t *timHw, int type) +{ + int padIdx = (int)(timHw - pads); + switch (type) { + case MAP_TO_MOTOR_OUTPUT: + timHw->usageFlags &= (uint32_t)TIM_USE_MOTOR; + outputs->motorPadIdx[outputs->motorCount++] = padIdx; + pwmClaimTimer_sim(pads, count, timHw->timId, timHw->usageFlags); + break; + case MAP_TO_SERVO_OUTPUT: + timHw->usageFlags &= (uint32_t)TIM_USE_SERVO; + outputs->servoPadIdx[outputs->servoCount++] = padIdx; + pwmClaimTimer_sim(pads, count, timHw->timId, timHw->usageFlags); + break; + case MAP_TO_LED_OUTPUT: + // BUG (pre-fix): propagates this pad's masked flags to every + // sibling sharing the timer, overwriting a correctly-computed + // TIM_USE_PINIO sibling back to TIM_USE_LED. + timHw->usageFlags &= (uint32_t)TIM_USE_LED; + pwmClaimTimer_sim(pads, count, timHw->timId, timHw->usageFlags); + break; + default: + break; + } +} + +typedef void (*pwmAssignOutputFn)(SimOutputs *, timerHardware_t *, int, timerHardware_t *, int); + +/* checkPwmTimerConflicts() stub — the task scenario matches the real + * hardware trace where FEATURE_LED_STRIP was off, so this always returns + * false (never skip a pad due to a conflicting port/feature). */ +static bool checkPwmTimerConflicts_stub(const timerHardware_t * /*timHw*/) +{ + return false; +} + +/* ------------------------------------------------------------------------- + * Piece 4: simulateBuildTimerOutputList() — minimal reproduction of + * pwmBuildTimerOutputList()'s two-loop structure: + * Loop 1: canonical-pad tracking (beeperClaimed[]/ledClaimed[]) + + * timerHardwareOverride() for every pad. + * Loop 2: priority-ordered assignment (priority 0 = dedicated pass, + * priority 1 = auto pass), motor branch then servo branch then + * LED branch (auto-pass only), exactly mirroring the real + * function's branch order and conditions. + * `assignFn` selects pwmAssignOutput_fixed or pwmAssignOutput_buggy. + * ------------------------------------------------------------------------- */ +static void simulateBuildTimerOutputList(timerHardware_t *pads, int count, + const outputMode_e *modeByTimId, + int motorCount, int servoCount, + pwmAssignOutputFn assignFn, + SimOutputs *outputs) +{ + outputs->motorCount = 0; + outputs->servoCount = 0; + + // Loop 1: apply all timerOverrides upfront (canonical-claim tracking). + bool beeperClaimed[MAX_TIMID] = { false }; + bool ledClaimed[MAX_TIMID] = { false }; + for (int idx = 0; idx < count; idx++) { + timerHardware_t *timHw = &pads[idx]; + uint8_t timId = timHw->timId; + ASSERT_LT(timId, MAX_TIMID); + outputMode_e mode = modeByTimId[timId]; + + bool isCanonicalBeeperPad = false; + if (mode == OUTPUT_MODE_BEEPER && !beeperClaimed[timId]) { + beeperClaimed[timId] = true; + isCanonicalBeeperPad = true; + } + bool isCanonicalLedPad = false; + if (mode == OUTPUT_MODE_LED && !ledClaimed[timId]) { + ledClaimed[timId] = true; + isCanonicalLedPad = true; + } + timerHardwareOverride_fixed(timHw, mode, isCanonicalBeeperPad, isCanonicalLedPad); + } + + // Loop 2: assign outputs in priority order (dedicated, then auto). + for (int priority = 0; priority < 2; priority++) { + int motorIdx = outputs->motorCount; + + for (int idx = 0; idx < count; idx++) { + timerHardware_t *timHw = &pads[idx]; + outputMode_e mode = modeByTimId[timHw->timId]; + bool isDedicated = (priority == 0); + + if (checkPwmTimerConflicts_stub(timHw)) { + continue; + } + + // Motors: dedicated (OUTPUT_MODE_MOTORS) first, then auto + if (TIM_IS_MOTOR(timHw->usageFlags) && motorIdx < motorCount + && !pwmHasServoOnTimer_sim(pads, outputs, timHw->timId) + && (isDedicated ? mode == OUTPUT_MODE_MOTORS : mode != OUTPUT_MODE_MOTORS)) { + assignFn(outputs, pads, count, timHw, MAP_TO_MOTOR_OUTPUT); + motorIdx++; + continue; + } + + // Servos: dedicated (OUTPUT_MODE_SERVOS) first, then auto + if (TIM_IS_SERVO(timHw->usageFlags) && outputs->servoCount < servoCount + && !pwmHasMotorOnTimer_sim(pads, outputs, timHw->timId) + && (isDedicated ? mode == OUTPUT_MODE_SERVOS : mode != OUTPUT_MODE_SERVOS)) { + assignFn(outputs, pads, count, timHw, MAP_TO_SERVO_OUTPUT); + continue; + } + + // LEDs: only on the auto pass, and only if timer is uncontested + if (!isDedicated && TIM_IS_LED(timHw->usageFlags) + && !pwmHasMotorOnTimer_sim(pads, outputs, timHw->timId) + && !pwmHasServoOnTimer_sim(pads, outputs, timHw->timId)) { + assignFn(outputs, pads, count, timHw, MAP_TO_LED_OUTPUT); + } + } + } +} + +/* ========================================================================= + * TEST SUITE: LedSiblingE2E + * + * Two pads share one physical timer (timId=4), overridden to + * OUTPUT_MODE_LED. No motors/servos in play (motorCount=servoCount=0), so + * only the LED auto-pass branch is exercised in loop 2 — the exact path + * that exposed the real bug (FEATURE_LED_STRIP off -> checkPwmTimerConflicts + * always false -> LED pad reached via the uncontested-timer auto pass). + * ========================================================================= */ + +TEST(LedSiblingE2E, BugReproduction_FullPipeline_SiblingIncorrectlyGetsLed) +{ + timerHardware_t pads[2]; + pads[0].usageFlags = 0; pads[0].timId = 4; + pads[1].usageFlags = 0; pads[1].timId = 4; + + outputMode_e modeByTimId[MAX_TIMID]; + for (int i = 0; i < MAX_TIMID; i++) modeByTimId[i] = OUTPUT_MODE_AUTO; + modeByTimId[4] = OUTPUT_MODE_LED; + + SimOutputs outputs; + simulateBuildTimerOutputList(pads, 2, modeByTimId, /*motorCount=*/0, /*servoCount=*/0, + pwmAssignOutput_buggy, &outputs); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED) + << "Canonical (first-scanned) pad should get TIM_USE_LED"; + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_LED) + << "BUG REPRODUCED: with the pre-fix pwmAssignOutput() (LED case also " + "calling pwmClaimTimer()), the sibling pad — correctly computed as " + "TIM_USE_PINIO by timerHardwareOverride() in loop 1 — gets silently " + "overwritten back to TIM_USE_LED when the canonical pad is assigned " + "in loop 2. This is the real hardware-observed bug (SPEDIXF405 " + "TIM4 S5+S6)."; + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Sibling's correct PINIO flag was clobbered by propagation (bug symptom)"; +} + +TEST(LedSiblingE2E, FixVerification_FullPipeline_SiblingKeepsPinio_NotLed) +{ + timerHardware_t pads[2]; + pads[0].usageFlags = 0; pads[0].timId = 4; + pads[1].usageFlags = 0; pads[1].timId = 4; + + outputMode_e modeByTimId[MAX_TIMID]; + for (int i = 0; i < MAX_TIMID; i++) modeByTimId[i] = OUTPUT_MODE_AUTO; + modeByTimId[4] = OUTPUT_MODE_LED; + + SimOutputs outputs; + simulateBuildTimerOutputList(pads, 2, modeByTimId, /*motorCount=*/0, /*servoCount=*/0, + pwmAssignOutput_fixed, &outputs); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED) + << "Canonical (first-scanned) pad must have TIM_USE_LED after the full pipeline"; + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Canonical pad must not also carry the PINIO fallback flag"; + + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_LED) + << "FIX VERIFIED: sibling pad on the same timer must NOT end up with " + "TIM_USE_LED after the full two-loop pipeline (this is the " + "assertion that would have FAILED against the pre-fix " + "pwmAssignOutput(), which called pwmClaimTimer() for LED too)"; + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Sibling pad must retain TIM_USE_PINIO (GPIO-only fallback) set by " + "timerHardwareOverride() in loop 1, undisturbed by loop 2's " + "assignment of the canonical pad"; +} + +TEST(LedSiblingE2E, FixVerification_ThreeSiblingPads_OnlyCanonicalKeepsLed) +{ + timerHardware_t pads[3]; + for (int i = 0; i < 3; i++) { pads[i].usageFlags = 0; pads[i].timId = 9; } + + outputMode_e modeByTimId[MAX_TIMID]; + for (int i = 0; i < MAX_TIMID; i++) modeByTimId[i] = OUTPUT_MODE_AUTO; + modeByTimId[9] = OUTPUT_MODE_LED; + + SimOutputs outputs; + simulateBuildTimerOutputList(pads, 3, modeByTimId, /*motorCount=*/0, /*servoCount=*/0, + pwmAssignOutput_fixed, &outputs); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED); + for (int i = 1; i < 3; i++) { + EXPECT_FALSE(pads[i].usageFlags & (uint32_t)TIM_USE_LED) + << "pads[" << i << "] must not end up LED-flagged"; + EXPECT_TRUE(pads[i].usageFlags & (uint32_t)TIM_USE_PINIO) + << "pads[" << i << "] must remain PINIO"; + } +} + +/* ========================================================================= + * TEST SUITE: MotorSiblingPropagationRegression + * + * Regression guard: pwmClaimTimer()'s propagate-to-siblings behavior must + * still happen for MAP_TO_MOTOR_OUTPUT (and, by the same code path, for + * MAP_TO_SERVO_OUTPUT). The fix removed the pwmClaimTimer() call ONLY from + * the LED case; it must NOT have been broadened to also drop it from + * motor/servo. + * ========================================================================= */ + +/* + * Direct-call test: proves pwmAssignOutput_fixed()'s MOTOR case still + * propagates via pwmClaimTimer_sim(). This is the test that actually + * *distinguishes* "propagation happened" from "propagation didn't happen" — + * pads[1] never has pwmAssignOutput_fixed() called on it directly; its + * flags can ONLY change as a side effect of pads[0]'s assignment call + * invoking pwmClaimTimer_sim() for the shared timId. Pre-existing + * (non-MOTOR) bits on pads[1] must be overwritten (not merely OR'd/left + * alone), matching pwmClaimTimer()'s real "usageFlags = usageFlags" (full + * overwrite, not `|=`) semantics. + */ +TEST(MotorSiblingPropagationRegression, DirectCall_PropagationOverwritesSiblingFlags) +{ + timerHardware_t pads[2]; + pads[0].usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_PWM); + pads[0].timId = 7; + pads[1].usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_PPM); // different pre-existing bit + pads[1].timId = 7; + + SimOutputs outputs; + outputs.motorCount = 0; + outputs.servoCount = 0; + + // Assign ONLY pads[0] — pads[1] is never passed to pwmAssignOutput_fixed(). + pwmAssignOutput_fixed(&outputs, pads, 2, &pads[0], MAP_TO_MOTOR_OUTPUT); + + EXPECT_EQ(pads[0].usageFlags, (uint32_t)TIM_USE_MOTOR) + << "Assigned pad's own flags must be masked down to TIM_USE_MOTOR only"; + EXPECT_EQ(pads[1].usageFlags, (uint32_t)TIM_USE_MOTOR) + << "REGRESSION CHECK: sibling pad's flags must be overwritten to " + "TIM_USE_MOTOR by pwmClaimTimer() propagation alone, even though " + "pwmAssignOutput() was never called on it directly. If this " + "propagation were accidentally removed for MOTOR too, pads[1] " + "would incorrectly retain TIM_USE_PPM here."; + EXPECT_EQ(outputs.motorCount, 1); +} + +/* + * Full-pipeline sanity check: with the current fixed source, a two-pad + * shared MOTOR timer still ends up with both pads assigned as motor outputs. + * (Note: unlike the direct-call test above, this alone would still pass even + * without propagation, since each pad also gets individually masked by its + * own pwmAssignOutput() call in the auto/dedicated pass — the direct-call + * test above is what actually isolates the propagation behavior. This test + * is included for overall pipeline sanity / non-regression of the motor + * assignment path as a whole.) + */ +TEST(MotorSiblingPropagationRegression, FullPipeline_BothMotorPadsAssigned) +{ + timerHardware_t pads[2]; + pads[0].usageFlags = 0; pads[0].timId = 7; + pads[1].usageFlags = 0; pads[1].timId = 7; + + outputMode_e modeByTimId[MAX_TIMID]; + for (int i = 0; i < MAX_TIMID; i++) modeByTimId[i] = OUTPUT_MODE_AUTO; + modeByTimId[7] = OUTPUT_MODE_MOTORS; + + SimOutputs outputs; + simulateBuildTimerOutputList(pads, 2, modeByTimId, /*motorCount=*/2, /*servoCount=*/0, + pwmAssignOutput_fixed, &outputs); + + EXPECT_EQ(outputs.motorCount, 2) << "Both pads on the shared MOTOR timer should be assigned"; + EXPECT_EQ(pads[0].usageFlags, (uint32_t)TIM_USE_MOTOR); + EXPECT_EQ(pads[1].usageFlags, (uint32_t)TIM_USE_MOTOR); +} + +/* ========================================================================= + * TEST SUITE: SourceSync + * + * Reads the ACTUAL LIVE src/main/drivers/pwm_mapping.c off disk and checks + * that pwmAssignOutput()'s MAP_TO_LED_OUTPUT case does NOT call + * pwmClaimTimer(), while MAP_TO_MOTOR_OUTPUT/MAP_TO_SERVO_OUTPUT still do. + * Mirrors the SourceSync pattern in pwm_mapping_led_unittest.cc / + * pwm_mapping_beeper_unittest.cc. This is a drift-detector, not a substitute + * for the behavioral tests above (see those files' header comments for why + * pwm_mapping.c can't be linked directly into a host unit test). + * ========================================================================= */ + +namespace { + +std::string stripLineComments(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + for (size_t i = 0; i < text.size();) { + if (text[i] == '/' && i + 1 < text.size() && text[i + 1] == '/') { + while (i < text.size() && text[i] != '\n') { + i++; + } + continue; + } + result += text[i]; + i++; + } + return result; +} + +std::string normalizeWhitespace(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + bool lastWasSpace = true; + for (char c : text) { + if (std::isspace(static_cast(c))) { + if (!lastWasSpace) { + result += ' '; + lastWasSpace = true; + } + } else { + result += c; + lastWasSpace = false; + } + } + while (!result.empty() && result.back() == ' ') { + result.pop_back(); + } + return result; +} + +std::string normalizeSource(const std::string &text) +{ + return normalizeWhitespace(stripLineComments(text)); +} + +std::string pwmMappingSourcePath() +{ + std::string thisFile = __FILE__; // .../src/test/unit/pwm_mapping_led_e2e_unittest.cc + size_t lastSlash = thisFile.find_last_of("/\\"); + std::string testUnitDir = (lastSlash == std::string::npos) ? "." : thisFile.substr(0, lastSlash); + return testUnitDir + "/../../main/drivers/pwm_mapping.c"; +} + +::testing::AssertionResult loadNormalizedPwmMappingSource(std::string *out) +{ + const std::string path = pwmMappingSourcePath(); + std::ifstream file(path); + if (!file.is_open()) { + return ::testing::AssertionFailure() + << "Could not open live source file at computed path: " << path + << " -- SourceSync tests cannot verify anything against dead/absent " + "source."; + } + std::ostringstream buffer; + buffer << file.rdbuf(); + if (buffer.str().empty()) { + return ::testing::AssertionFailure() + << "Live source file at " << path << " opened but read as empty."; + } + *out = normalizeSource(buffer.str()); + return ::testing::AssertionSuccess(); +} + +// Extracts the substring of `normalizedSource` starting at the (already +// normalized) needle `funcSignature` through its matching closing brace, +// assuming brace-balanced C code with no braces inside string/char literals +// (true for pwmAssignOutput()). Returns empty string if not found or +// unbalanced. +std::string extractBraceBlock(const std::string &normalizedSource, const std::string &funcSignature) +{ + size_t start = normalizedSource.find(funcSignature); + if (start == std::string::npos) { + return ""; + } + size_t bracePos = normalizedSource.find('{', start); + if (bracePos == std::string::npos) { + return ""; + } + int depth = 0; + size_t i = bracePos; + for (; i < normalizedSource.size(); i++) { + if (normalizedSource[i] == '{') depth++; + else if (normalizedSource[i] == '}') { + depth--; + if (depth == 0) { + return normalizedSource.substr(bracePos, i - bracePos + 1); + } + } + } + return ""; // unbalanced -- shouldn't happen for valid C +} + +} // namespace + +TEST(SourceSync, PwmAssignOutputSignatureMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedSignature = normalizeSource( + "static void pwmAssignOutput(timMotorServoHardware_t *timOutputs, " + "timerHardware_t *timHw, int type)"); + + EXPECT_NE(normalizedSource.find(expectedSignature), std::string::npos) + << "pwmAssignOutput()'s signature in the LIVE src/main/drivers/pwm_mapping.c " + "no longer matches what this test assumes. Update " + "pwm_mapping_led_e2e_unittest.cc's inline reproduction and this " + "expected string to match."; +} + +TEST(SourceSync, LedCaseDoesNotCallPwmClaimTimer) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + std::string body = extractBraceBlock(normalizedSource, + normalizeSource("static void pwmAssignOutput(timMotorServoHardware_t *timOutputs, " + "timerHardware_t *timHw, int type)")); + ASSERT_FALSE(body.empty()) + << "Could not locate/extract pwmAssignOutput()'s body from the live source " + "-- check PwmAssignOutputSignatureMatchesLiveSource for details."; + + size_t ledCasePos = body.find("case MAP_TO_LED_OUTPUT:"); + ASSERT_NE(ledCasePos, std::string::npos) + << "Could not find 'case MAP_TO_LED_OUTPUT:' inside pwmAssignOutput()'s body " + "in the live source."; + + size_t nextCasePos = body.find("case", ledCasePos + 1); + size_t defaultPos = body.find("default:", ledCasePos + 1); + size_t ledCaseEnd = std::min( + nextCasePos == std::string::npos ? body.size() : nextCasePos, + defaultPos == std::string::npos ? body.size() : defaultPos); + std::string ledCaseBody = body.substr(ledCasePos, ledCaseEnd - ledCasePos); + + EXPECT_EQ(ledCaseBody.find("pwmClaimTimer("), std::string::npos) + << "REGRESSION: the live MAP_TO_LED_OUTPUT case in " + "src/main/drivers/pwm_mapping.c now calls pwmClaimTimer() again. " + "This reintroduces the exact bug this test file guards against: " + "propagating this pad's flags to sibling pads on the same timer, " + "overwriting their correct TIM_USE_PINIO back to TIM_USE_LED. " + "LED case body found: " << ledCaseBody; +} + +TEST(SourceSync, MotorAndServoCasesStillCallPwmClaimTimer) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + std::string body = extractBraceBlock(normalizedSource, + normalizeSource("static void pwmAssignOutput(timMotorServoHardware_t *timOutputs, " + "timerHardware_t *timHw, int type)")); + ASSERT_FALSE(body.empty()); + + size_t motorCasePos = body.find("case MAP_TO_MOTOR_OUTPUT:"); + size_t servoCasePos = body.find("case MAP_TO_SERVO_OUTPUT:"); + ASSERT_NE(motorCasePos, std::string::npos); + ASSERT_NE(servoCasePos, std::string::npos); + ASSERT_LT(motorCasePos, servoCasePos) + << "Expected MAP_TO_MOTOR_OUTPUT case to appear before MAP_TO_SERVO_OUTPUT " + "in source order, as in the current file. If this legitimately " + "changed, adjust this test's slicing logic."; + + size_t ledCasePos = body.find("case MAP_TO_LED_OUTPUT:", servoCasePos); + ASSERT_NE(ledCasePos, std::string::npos); + + std::string motorAndServoBody = body.substr(motorCasePos, ledCasePos - motorCasePos); + + // Expect exactly two pwmClaimTimer( calls in the motor+servo span (one each). + int count = 0; + size_t pos = 0; + while ((pos = motorAndServoBody.find("pwmClaimTimer(", pos)) != std::string::npos) { + count++; + pos += std::strlen("pwmClaimTimer("); + } + EXPECT_EQ(count, 2) + << "REGRESSION: expected exactly 2 pwmClaimTimer() calls across the " + "MAP_TO_MOTOR_OUTPUT and MAP_TO_SERVO_OUTPUT cases (one each) in the " + "live source, found " << count << ". This fix must only remove " + "propagation for LED, not for motor/servo. Span checked: " + << motorAndServoBody; +} + +TEST(SourceSync, PwmClaimTimerBodyMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedBody = normalizeSource( + "uint8_t pwmClaimTimer(HAL_Timer_t *tim, uint32_t usageFlags) { " + "uint8_t changed = 0; " + "for (int idx = 0; idx < timerHardwareCount; idx++) { " + "timerHardware_t *timHw = &timerHardware[idx]; " + "if (timHw->tim == tim && timHw->usageFlags != usageFlags) { " + "timHw->usageFlags = usageFlags; " + "changed++; " + "} " + "} " + "return changed; " + "}"); + + EXPECT_NE(normalizedSource.find(expectedBody), std::string::npos) + << "pwmClaimTimer()'s body in the LIVE src/main/drivers/pwm_mapping.c no " + "longer matches pwmClaimTimer_sim()'s reproduction in this test file " + "(propagate-usageFlags-to-siblings-sharing-a-timer). Update " + "pwmClaimTimer_sim() to match, then update this expected string."; +} diff --git a/src/test/unit/pwm_mapping_led_unittest.cc b/src/test/unit/pwm_mapping_led_unittest.cc new file mode 100644 index 00000000000..3428107afa6 --- /dev/null +++ b/src/test/unit/pwm_mapping_led_unittest.cc @@ -0,0 +1,705 @@ +/* + * Unit test: timerHardwareOverride() must not falsely flag sibling timer + * channels as TIM_USE_LED when OUTPUT_MODE_LED is applied to a shared timer. + * + * Bug (pre-fix, same bug class as the BEEPER bug fixed by commits + * 8c16aeed85 / 31bb99ea0b, see pwm_mapping_beeper_unittest.cc): + * timerHardwareOverride() applied TIM_USE_LED to every pad sharing a timer + * set to OUTPUT_MODE_LED, but the WS2811 LED strip driver + * (light_ws2811strip.c) only ever wires up the FIRST matching pad, found + * via timerGetByUsageFlag(TIM_USE_LED). All channels of one timer share a + * single period register (fixed at the WS2811 bit rate), so only one pad + * can actually drive the LED strip. This caused sibling pads on the same + * timer to be falsely reported/claimed as LED outputs while being + * functionally dead (or worse, stealing a pad that could have been used + * for something else). + * + * Fix (mirrors the BEEPER fix exactly): + * timerHardwareOverride() now takes an `isCanonicalLedPad` bool. The + * OUTPUT_MODE_LED case always clears TIM_USE_MOTOR|TIM_USE_SERVO| + * TIM_USE_BEEPER|TIM_USE_LED, and sets TIM_USE_PINIO as a GPIO-only + * fallback (binary on/off, no PWM — doesn't need the timer's fixed + * WS2811-bitrate period). Only if isCanonicalLedPad is true (i.e. this is + * the first timerHardware[] entry, in ascending scan order, seen for this + * physical timer with outputMode == OUTPUT_MODE_LED) does it clear + * TIM_USE_PINIO and set TIM_USE_LED instead. + * + * pwmBuildTimerOutputList() computes isCanonicalLedPad via a + * `ledClaimed[HARDWARE_TIMER_DEFINITION_COUNT]` array, parallel to the + * existing `beeperClaimed[]` array used for OUTPUT_MODE_BEEPER. + * + * This file contains: + * 1. A minimal inline reproduction of both the BUGGY and FIXED versions of + * timerHardwareOverride() (LED case only, plus MOTORS/SERVOS/BEEPER/ + * PINIO for regression coverage) so the test is fully self-contained. + * pwm_mapping.c is excluded from the SITL/unit build by + * `#if !defined(SITL_BUILD)`, and the real function depends on PG + * (parameter-group) infrastructure not available in a host unit build. + * 2. TEST BugReproduction — demonstrates that the old, unconditional-LED + * code gives TIM_USE_LED to every pad sharing an LED-mode timer. + * 3. TEST FixVerification — only the canonical (first) pad on a shared + * timer gets TIM_USE_LED; sibling pads fall back to TIM_USE_PINIO. + * 4. Multi-pad (3 siblings) test — only the first scanned pad is canonical. + * 5. Regression tests for MOTORS/SERVOS/BEEPER/PINIO overrides on the + * fixed function, including timers mixed with LED-mode timers. + * 6. TEST SUITE SourceSync — reads the ACTUAL LIVE + * src/main/drivers/pwm_mapping.c off disk at test time and asserts the + * real timerHardwareOverride() source text still matches the logic the + * inline reproduction above assumes. This exists specifically because + * the sibling pwm_mapping_beeper_unittest.cc was found to have drifted: + * it kept passing for months after commits 8c16aeed85/31bb99ea0b + * changed the real BEEPER logic, because it only ever exercised its own + * frozen, hand-copied logic and never looked at the live file again. + * An inline reproduction is unavoidable here (see point 1), but it + * should not be allowed to silently go stale — if pwm_mapping.c's real + * OUTPUT_MODE_LED case (or the canonical-pad tracking loop in + * pwmBuildTimerOutputList()) changes without this file being updated to + * match, SourceSync must fail loudly instead of the rest of the suite + * quietly passing against dead logic. + */ + +#include +#include +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "unittest_macros.h" + +/* ------------------------------------------------------------------------- + * Minimal type/flag definitions — mirrors the real firmware headers on this + * branch (src/main/drivers/timer.h, src/main/flight/mixer.h). Unlike + * release/9.1, this branch has TIM_USE_PINIO / TIM_USE_BEEPER and + * OUTPUT_MODE_PINIO / OUTPUT_MODE_BEEPER. + * ------------------------------------------------------------------------- */ + +/* timerUsageFlag_e — must match src/main/drivers/timer.h exactly */ +typedef enum { + TIM_USE_ANY = 0, + TIM_USE_PPM = (1 << 0), + TIM_USE_PWM = (1 << 1), + TIM_USE_MOTOR = (1 << 2), + TIM_USE_SERVO = (1 << 3), + TIM_USE_LED = (1 << 24), + TIM_USE_BEEPER = (1 << 25), + TIM_USE_PINIO = (1 << 26), +} timerUsageFlag_e; + +/* outputMode_e — must match src/main/flight/mixer.h exactly */ +typedef enum { + OUTPUT_MODE_AUTO = 0, + OUTPUT_MODE_MOTORS, + OUTPUT_MODE_SERVOS, + OUTPUT_MODE_LED, + OUTPUT_MODE_PINIO, + OUTPUT_MODE_BEEPER +} outputMode_e; + +/* Minimal timer hardware entry — only the fields exercised by the function. + * `timId` stands in for the real code's timer2id(timer->tim) — in the real + * driver, multiple timerHardware[] pads sharing the same physical timer + * compare equal on timer2id(); here we just store the id directly. */ +typedef struct timerHardware_s { + uint32_t usageFlags; + uint8_t timId; +} timerHardware_t; + +/* ------------------------------------------------------------------------- + * Inline reproductions of timerHardwareOverride(). + * + * We inline these rather than linking pwm_mapping.c because: + * a) pwm_mapping.c is wrapped in #if !defined(SITL_BUILD) and the unit-test + * target.h defines SITL_BUILD, so the real file compiles to nothing. + * b) The function depends on timerOverrides() (parameter-group accessor) and + * timer2id(), which are platform / PG infrastructure that is not present + * in a host unit-test build. + * + * The functions below are literal translations of the current C source + * (post BEEPER-fix, post LED-fix) with the PG indirection collapsed into a + * simple `outputMode` parameter, and canonical-pad tracking passed in + * explicitly as bool parameters (mirroring isCanonicalBeeperPad / + * isCanonicalLedPad computed by pwmBuildTimerOutputList()). + * + * See the SourceSync test suite at the bottom of this file for a mechanism + * that catches this reproduction going out of sync with the real source. + * ------------------------------------------------------------------------- */ + +/* + * BUGGY version — reproduces the pre-fix LED behaviour: OUTPUT_MODE_LED + * unconditionally sets TIM_USE_LED on every pad sharing the timer, with no + * canonical-pad tracking at all (same bug class as the pre-fix BEEPER code + * in commit history before 8c16aeed85). MOTORS/SERVOS/PINIO/BEEPER cases are + * included only for completeness / contrast; this reproduction focuses on + * the LED bug. + */ +static void timerHardwareOverride_buggy(timerHardware_t *timer, + outputMode_e outputMode) +{ + switch (outputMode) { + case OUTPUT_MODE_MOTORS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; + break; + case OUTPUT_MODE_SERVOS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_SERVO; + break; + case OUTPUT_MODE_LED: + /* BUG: unconditionally sets TIM_USE_LED on every pad of this + * timer, even though only one pad is actually wired up by + * ws2811LedStripInit(). No canonical-pad tracking whatsoever. */ + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_LED; + break; + case OUTPUT_MODE_PINIO: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + break; + case OUTPUT_MODE_BEEPER: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED); + timer->usageFlags |= (uint32_t)TIM_USE_BEEPER; + break; + default: + break; + } +} + +/* + * FIXED version — literal translation of the current + * src/main/drivers/pwm_mapping.c timerHardwareOverride() on this branch. + * Kept in sync with the real source by the SourceSync tests below. + */ +static void timerHardwareOverride_fixed(timerHardware_t *timer, + outputMode_e outputMode, + bool isCanonicalBeeperPad, + bool isCanonicalLedPad) +{ + switch (outputMode) { + case OUTPUT_MODE_MOTORS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; + break; + case OUTPUT_MODE_SERVOS: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_SERVO; + break; + case OUTPUT_MODE_LED: + // Other channels of this timer share its period register, so they can't + // be repurposed as motor/servo/BEEPER either. Only the first (canonical) + // pad is the one ws2811LedStripInit() actually wires up; siblings can still + // drive a GPIO-only PINIO (binary on/off, no PWM), since that doesn't need + // the timer's fixed WS2811-bitrate period. + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_BEEPER | TIM_USE_LED); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalLedPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_LED; + } + break; + case OUTPUT_MODE_PINIO: + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + break; + case OUTPUT_MODE_BEEPER: + // Other channels of this timer share its period register, so they can't + // be repurposed as motor/servo/LED either. They can still drive a GPIO-only + // PINIO (binary on/off, no PWM), since that doesn't need the timer's period. + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalBeeperPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_BEEPER; + } + break; + default: + break; + } +} + +/* + * Helper — reproduces the canonical-pad-tracking loop in + * pwmBuildTimerOutputList(): scans pads in ascending array order and applies + * timerHardwareOverride_fixed() with isCanonicalBeeperPad / isCanonicalLedPad + * computed exactly as the real code does (first pad seen per physical timer, + * for a timer whose overridden outputMode matches, wins canonical status). + */ +static void applyFixedOverridesToArray(timerHardware_t *pads, int count, + const outputMode_e *outputModeByPad, + int maxTimId) +{ + bool beeperClaimed[32] = { false }; + bool ledClaimed[32] = { false }; + ASSERT_LT(maxTimId, 32); + + for (int idx = 0; idx < count; idx++) { + timerHardware_t *timHw = &pads[idx]; + uint8_t timId = timHw->timId; + outputMode_e mode = outputModeByPad[idx]; + + bool isCanonicalBeeperPad = false; + if (mode == OUTPUT_MODE_BEEPER && !beeperClaimed[timId]) { + beeperClaimed[timId] = true; + isCanonicalBeeperPad = true; + } + bool isCanonicalLedPad = false; + if (mode == OUTPUT_MODE_LED && !ledClaimed[timId]) { + ledClaimed[timId] = true; + isCanonicalLedPad = true; + } + timerHardwareOverride_fixed(timHw, mode, isCanonicalBeeperPad, isCanonicalLedPad); + } +} + +/* ========================================================================= + * TEST SUITE: LedTimerSiblingPads + * ========================================================================= */ + +/* + * TEST 1 — BugReproduction + * + * Two pads share timer id 5, both overridden to OUTPUT_MODE_LED. On the + * BUGGY code path (no canonical-pad tracking at all), BOTH pads get + * TIM_USE_LED, even though only one of them will ever actually be wired up + * by ws2811LedStripInit()/timerGetByUsageFlag(TIM_USE_LED). This test + * PASSES (i.e. the bug reproduces) on the buggy code — confirming the + * observable symptom of the bug. + */ +TEST(LedTimerSiblingPads, BugReproduction_BothSiblingsGetLedFlag) +{ + timerHardware_t padA; padA.usageFlags = 0; padA.timId = 5; + timerHardware_t padB; padB.usageFlags = 0; padB.timId = 5; + + timerHardwareOverride_buggy(&padA, OUTPUT_MODE_LED); + timerHardwareOverride_buggy(&padB, OUTPUT_MODE_LED); + + EXPECT_TRUE(padA.usageFlags & (uint32_t)TIM_USE_LED) + << "First pad should get TIM_USE_LED (expected)"; + EXPECT_TRUE(padB.usageFlags & (uint32_t)TIM_USE_LED) + << "BUG CONFIRMED: sibling pad on the SAME timer also got TIM_USE_LED, " + "but light_ws2811strip.c only wires up the first pad found via " + "timerGetByUsageFlag(TIM_USE_LED). This sibling would be falsely " + "reported as an LED output while being functionally dead."; +} + +/* + * TEST 2 — FixVerification (two sibling pads) + * + * Same scenario as above, but using the FIXED function with proper + * canonical-pad computation (as pwmBuildTimerOutputList() does). Only the + * first-scanned pad on the shared timer should get TIM_USE_LED; the sibling + * must fall back to TIM_USE_PINIO (GPIO-only), NOT TIM_USE_LED, and NOT be + * left flagless (this branch has the PINIO fallback, unlike release/9.1). + */ +TEST(LedTimerSiblingPads, FixVerification_OnlyCanonicalPadGetsLed) +{ + timerHardware_t pads[2]; + pads[0].usageFlags = 0; pads[0].timId = 5; + pads[1].usageFlags = 0; pads[1].timId = 5; + outputMode_e modes[2] = { OUTPUT_MODE_LED, OUTPUT_MODE_LED }; + + applyFixedOverridesToArray(pads, 2, modes, 6); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED) + << "Canonical (first-scanned) pad must get TIM_USE_LED"; + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Canonical pad must not also carry the PINIO fallback flag"; + + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_LED) + << "FIX VERIFIED: sibling pad on the same timer must NOT get TIM_USE_LED"; + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Sibling pad must fall back to TIM_USE_PINIO (GPIO-only), not be left flagless"; +} + +/* + * TEST 3 — Three-pad case: only the first scanned pad is canonical + * + * Confirms the canonical tracking generalizes beyond two pads: with three + * pads sharing a timer, only pads[0] should be canonical; pads[1] and + * pads[2] must both fall back to PINIO. + */ +TEST(LedTimerSiblingPads, ThreePads_OnlyFirstScannedIsCanonical) +{ + timerHardware_t pads[3]; + for (int i = 0; i < 3; i++) { + pads[i].usageFlags = 0; + pads[i].timId = 7; + } + outputMode_e modes[3] = { OUTPUT_MODE_LED, OUTPUT_MODE_LED, OUTPUT_MODE_LED }; + + applyFixedOverridesToArray(pads, 3, modes, 8); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED) + << "pads[0] (first scanned) must be canonical and get TIM_USE_LED"; + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO); + + for (int i = 1; i < 3; i++) { + EXPECT_FALSE(pads[i].usageFlags & (uint32_t)TIM_USE_LED) + << "pads[" << i << "] must NOT be canonical, so must not get TIM_USE_LED"; + EXPECT_TRUE(pads[i].usageFlags & (uint32_t)TIM_USE_PINIO) + << "pads[" << i << "] must fall back to TIM_USE_PINIO"; + } +} + +/* + * TEST 4 — Two independent LED timers each get their own canonical pad + * + * Pads on timer id 5 and pads on timer id 9 are independent: each timer + * should get exactly one canonical (LED) pad, tracked separately via the + * per-timer ledClaimed[] array. + */ +TEST(LedTimerSiblingPads, IndependentTimers_EachGetsOwnCanonicalPad) +{ + timerHardware_t pads[4]; + pads[0].usageFlags = 0; pads[0].timId = 5; // timer A, pad 1 -> canonical + pads[1].usageFlags = 0; pads[1].timId = 5; // timer A, pad 2 -> sibling + pads[2].usageFlags = 0; pads[2].timId = 9; // timer B, pad 1 -> canonical + pads[3].usageFlags = 0; pads[3].timId = 9; // timer B, pad 2 -> sibling + outputMode_e modes[4] = { OUTPUT_MODE_LED, OUTPUT_MODE_LED, OUTPUT_MODE_LED, OUTPUT_MODE_LED }; + + applyFixedOverridesToArray(pads, 4, modes, 10); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO); + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_LED); + + EXPECT_TRUE(pads[2].usageFlags & (uint32_t)TIM_USE_LED) + << "Timer B's first pad must independently become canonical"; + EXPECT_TRUE(pads[3].usageFlags & (uint32_t)TIM_USE_PINIO); + EXPECT_FALSE(pads[3].usageFlags & (uint32_t)TIM_USE_LED); +} + +/* ========================================================================= + * Negative / regression tests — MOTORS/SERVOS/BEEPER overrides must still + * work correctly with the fixed function, including when mixed with an + * LED-mode timer in the same scan. + * ========================================================================= */ + +TEST(LedTimerRegression, MotorOverride_Unaffected) +{ + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 1; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_MOTORS, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); +} + +TEST(LedTimerRegression, ServoOverride_Unaffected) +{ + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 2; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_SERVOS, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); +} + +TEST(LedTimerRegression, BeeperCanonicalPad_StillGetsBeeperFlag) +{ + /* Regression: the BEEPER canonical-pad logic (isCanonicalBeeperPad) must + * be completely unaffected by the addition of isCanonicalLedPad. */ + timerHardware_t timer; + timer.usageFlags = 0; + timer.timId = 3; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_BEEPER, + /*isCanonicalBeeperPad=*/true, + /*isCanonicalLedPad=*/false); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_PINIO); +} + +TEST(LedTimerRegression, BeeperSiblingPad_FallsBackToPinio) +{ + timerHardware_t timer; + timer.usageFlags = 0; + timer.timId = 3; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_BEEPER, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_PINIO) + << "Non-canonical BEEPER pad must fall back to PINIO (regression check " + "for the pre-existing BEEPER fix, unaffected by the LED fix)"; +} + +TEST(LedTimerRegression, PinioOverride_Unaffected) +{ + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED); + timer.timId = 4; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_PINIO, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_PINIO); +} + +TEST(LedTimerRegression, AutoMode_LeavesTimerUnchanged) +{ + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 6; + const uint32_t originalFlags = timer.usageFlags; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_AUTO, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_EQ(timer.usageFlags, originalFlags) + << "OUTPUT_MODE_AUTO (default case) must not change flags"; +} + +/* + * TEST — Mixed scan: MOTOR timer, LED timer (2 pads), BEEPER timer (canonical + * pad only) all processed together via applyFixedOverridesToArray(), matching + * a realistic pwmBuildTimerOutputList() scenario where different timers on + * the board have different output-mode overrides. Confirms none of the + * per-timer canonical tracking (ledClaimed[] vs beeperClaimed[]) interferes + * across timer ids or across output modes. + */ +TEST(LedTimerRegression, MixedScan_MotorLedBeeperTimersDoNotInterfere) +{ + timerHardware_t pads[5]; + for (int i = 0; i < 5; i++) pads[i].usageFlags = 0; + + pads[0].timId = 1; // MOTOR timer + pads[1].timId = 2; // LED timer, canonical + pads[2].timId = 2; // LED timer, sibling + pads[3].timId = 3; // BEEPER timer, canonical + pads[4].timId = 3; // BEEPER timer, sibling + + outputMode_e modes[5] = { + OUTPUT_MODE_MOTORS, + OUTPUT_MODE_LED, + OUTPUT_MODE_LED, + OUTPUT_MODE_BEEPER, + OUTPUT_MODE_BEEPER, + }; + + applyFixedOverridesToArray(pads, 5, modes, 4); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_MOTOR); + + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_FALSE(pads[2].usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(pads[2].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_TRUE(pads[3].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_FALSE(pads[3].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_FALSE(pads[4].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(pads[4].usageFlags & (uint32_t)TIM_USE_PINIO); +} + +/* ========================================================================= + * TEST SUITE: SourceSync + * + * Reads the ACTUAL LIVE src/main/drivers/pwm_mapping.c off disk (not a + * cached copy, not something computed at CMake-configure time) and checks + * that specific, logic-relevant fragments of the real timerHardwareOverride() + * and pwmBuildTimerOutputList() are still present verbatim (modulo + * whitespace and comments). If a future change to pwm_mapping.c alters this + * logic without updating the inline reproduction above, these tests fail — + * loudly, in `make check` / CI — instead of the rest of the file silently + * continuing to pass against a stale copy. + * + * This does not (and structurally cannot, given the SITL_BUILD guard around + * the entire file — see block comment above) substitute for actually linking + * and executing the real function. It is a drift-detector, not a full + * integration test. Comments are stripped and whitespace is collapsed before + * comparison so that pure reformatting/typo-fixes in comments do not cause + * false failures; only the executable-statement tokens are checked. + * ========================================================================= */ + +namespace { + +// Strips "//...\n" line comments. Deliberately does not handle /* */ block +// comments or comments inside string/char literals — pwm_mapping.c does not +// use either within the functions we check, and keeping this simple avoids +// this drift-detector itself becoming a source of drift. +std::string stripLineComments(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + for (size_t i = 0; i < text.size();) { + if (text[i] == '/' && i + 1 < text.size() && text[i + 1] == '/') { + while (i < text.size() && text[i] != '\n') { + i++; + } + continue; + } + result += text[i]; + i++; + } + return result; +} + +// Collapses any run of whitespace (including newlines) to a single space, +// and trims leading/trailing whitespace, so indentation/line-wrap changes +// don't cause false failures. +std::string normalizeWhitespace(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + bool lastWasSpace = true; // trims leading whitespace + for (char c : text) { + if (std::isspace(static_cast(c))) { + if (!lastWasSpace) { + result += ' '; + lastWasSpace = true; + } + } else { + result += c; + lastWasSpace = false; + } + } + while (!result.empty() && result.back() == ' ') { + result.pop_back(); + } + return result; +} + +std::string normalizeSource(const std::string &text) +{ + return normalizeWhitespace(stripLineComments(text)); +} + +// Locates the live pwm_mapping.c relative to this test file's own path. +// __FILE__ is emitted as an absolute path by this project's CMake/Makefiles +// (verified: `c++ ... -c /abs/path/to/pwm_mapping_led_unittest.cc`), so this +// resolves correctly regardless of the build directory or CWD the test is +// invoked from. +std::string pwmMappingSourcePath() +{ + std::string thisFile = __FILE__; // .../src/test/unit/pwm_mapping_led_unittest.cc + size_t lastSlash = thisFile.find_last_of("/\\"); + std::string testUnitDir = (lastSlash == std::string::npos) ? "." : thisFile.substr(0, lastSlash); + return testUnitDir + "/../../main/drivers/pwm_mapping.c"; +} + +// Reads and normalizes pwm_mapping.c. Fails the calling test LOUDLY (not +// silently returning empty / skipping) if the file can't be found, since a +// missing file here means this drift-detector isn't detecting anything. +::testing::AssertionResult loadNormalizedPwmMappingSource(std::string *out) +{ + const std::string path = pwmMappingSourcePath(); + std::ifstream file(path); + if (!file.is_open()) { + return ::testing::AssertionFailure() + << "Could not open live source file at computed path: " << path + << " -- SourceSync tests cannot verify anything against dead/absent " + "source. Check that src/test/unit/ and src/main/drivers/ still " + "have their expected relative layout."; + } + std::ostringstream buffer; + buffer << file.rdbuf(); + if (buffer.str().empty()) { + return ::testing::AssertionFailure() + << "Live source file at " << path << " opened but read as empty."; + } + *out = normalizeSource(buffer.str()); + return ::testing::AssertionSuccess(); +} + +} // namespace + +TEST(SourceSync, TimerHardwareOverrideSignatureMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedSignature = normalizeSource( + "static void timerHardwareOverride(timerHardware_t * timer, " + "bool isCanonicalBeeperPad, bool isCanonicalLedPad) {"); + + EXPECT_NE(normalizedSource.find(expectedSignature), std::string::npos) + << "timerHardwareOverride()'s signature in the LIVE " + "src/main/drivers/pwm_mapping.c no longer matches what this test's " + "inline reproduction assumes. If the signature legitimately " + "changed, update timerHardwareOverride_fixed()/_buggy() in " + "pwm_mapping_led_unittest.cc (and pwm_mapping_beeper_unittest.cc) " + "to match, then update this expected string too."; +} + +TEST(SourceSync, LedCaseMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + // Comments intentionally omitted here (stripLineComments() removes them + // from the live source before comparison too) so that comment wording + // changes don't cause false failures; only the executable statements are + // checked, which is exactly the logic the FixVerification tests above + // depend on. + const std::string expectedLedCase = normalizeSource( + "case OUTPUT_MODE_LED: " + "timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_BEEPER|TIM_USE_LED); " + "timer->usageFlags |= TIM_USE_PINIO; " + "if (isCanonicalLedPad) { " + "timer->usageFlags &= ~TIM_USE_PINIO; " + "timer->usageFlags |= TIM_USE_LED; " + "} " + "break;"); + + EXPECT_NE(normalizedSource.find(expectedLedCase), std::string::npos) + << "The OUTPUT_MODE_LED case in the LIVE src/main/drivers/pwm_mapping.c " + "no longer matches timerHardwareOverride_fixed()'s LED case in this " + "test file. This is exactly the kind of drift that let " + "pwm_mapping_beeper_unittest.cc silently stop verifying real " + "behavior after commits 8c16aeed85/31bb99ea0b. Update the inline " + "reproduction (and the FixVerification/BugReproduction tests, if " + "the fix strategy itself changed) to match the live source."; +} + +TEST(SourceSync, CanonicalPadTrackingLoopMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedLoop = normalizeSource( + "bool beeperClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; " + "bool ledClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; " + "for (int idx = 0; idx < timerHardwareCount; idx++) { " + "timerHardware_t *timHw = &timerHardware[idx]; " + "uint8_t timId = timer2id(timHw->tim); " + "bool isCanonicalBeeperPad = false; " + "if (timerOverrides(timId)->outputMode == OUTPUT_MODE_BEEPER && !beeperClaimed[timId]) { " + "beeperClaimed[timId] = true; " + "isCanonicalBeeperPad = true; " + "} " + "bool isCanonicalLedPad = false; " + "if (timerOverrides(timId)->outputMode == OUTPUT_MODE_LED && !ledClaimed[timId]) { " + "ledClaimed[timId] = true; " + "isCanonicalLedPad = true; " + "}"); + + EXPECT_NE(normalizedSource.find(expectedLoop), std::string::npos) + << "pwmBuildTimerOutputList()'s canonical-pad-tracking loop in the LIVE " + "src/main/drivers/pwm_mapping.c no longer matches what " + "applyFixedOverridesToArray() in this test file reproduces. Update " + "applyFixedOverridesToArray() to match the live source (e.g. if the " + "ledClaimed[]/beeperClaimed[] scanning strategy changes)."; +} From 535a2a15ddf79f312c789e6e275146dab33bf1b9 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 26 Jul 2026 02:01:41 -0500 Subject: [PATCH 2/3] test: refresh stale BEEPER pwm_mapping unit test to match current source pwm_mapping_beeper_unittest.cc still hand-reproduced an older, since-superseded timerHardwareOverride() (a simple early-return guard, no canonical-pad tracking, no PINIO fallback) rather than the version that actually landed in commits 8c16aeed85/31bb99ea0b. It kept passing because it's fully self-contained and only ever exercised its own stale inline copy, so it had stopped verifying real BEEPER behavior. Rewritten to mirror the current source, with a SourceSync suite that reads pwm_mapping.c at test time and asserts the inline reproduction's key logic tokens still match the live file, so this can't silently drift again. --- src/test/unit/pwm_mapping_beeper_unittest.cc | 779 ++++++++++++++----- 1 file changed, 566 insertions(+), 213 deletions(-) diff --git a/src/test/unit/pwm_mapping_beeper_unittest.cc b/src/test/unit/pwm_mapping_beeper_unittest.cc index 1ce0754a618..e71eb691253 100644 --- a/src/test/unit/pwm_mapping_beeper_unittest.cc +++ b/src/test/unit/pwm_mapping_beeper_unittest.cc @@ -1,39 +1,87 @@ /* - * Unit test: timerHardwareOverride() must not corrupt TIM_USE_BEEPER flags. + * Unit test: timerHardwareOverride() must not corrupt/misassign TIM_USE_BEEPER + * flags on shared timers. * - * Bug (pre-fix): - * When a user applies OUTPUT_MODE_SERVOS to a timer that has TIM_USE_BEEPER - * set (e.g. MATEKH743 TIM2/PA15), timerHardwareOverride() would apply the - * servo-mode mask without first checking for the beeper flag. The subsequent - * pwmAssignOutput(servo) call strips everything except TIM_USE_SERVO, leaving - * beeperPwmInit() unable to find its timer. + * History (this file previously tested an intermediate, now-superseded fix — + * see below; it was found to be STALE and was rewritten to match current + * source while adding pwm_mapping_led_unittest.cc): * - * Fix (commit 551bce85d6): - * Guard added at the top of timerHardwareOverride(): - * if (timer->usageFlags & TIM_USE_BEEPER) { return; } + * 1. Bug (original, issue #11492): if a user applied a timer_output_mode + * override (e.g. OUTPUT_MODE_SERVOS) to a timer that also had + * TIM_USE_BEEPER set (e.g. MATEKH743 TIM2/PA15), timerHardwareOverride() + * would apply the servo-mode mask without protecting the beeper flag. + * The subsequent pwmAssignOutput(servo) call then strips everything + * except TIM_USE_SERVO, leaving beeperPwmInit() unable to find its + * timer. + * + * 2. Intermediate fix (commit 7eebdf6345, "pwm_mapping: guard beeper timer + * from timerHardwareOverride()"): guard added at the top of + * timerHardwareOverride() — + * if (timer->usageFlags & TIM_USE_BEEPER) { return; } + * This made ANY timer carrying TIM_USE_BEEPER completely immune to all + * output-mode overrides, regardless of the override applied to it. This + * is what the ORIGINAL version of this test file verified. + * + * 3. CURRENT fix (commits 8c16aeed85 "Fix BEEPER output mode falsely + * flagging sibling timer channels" and 31bb99ea0b "Expose BEEPER-timer + * sibling pads as GPIO-only PINIO outputs"): the step-2 early-return + * guard was replaced entirely. BEEPER is now its own explicit + * OUTPUT_MODE_BEEPER (rather than being inferred from a pre-existing + * TIM_USE_BEEPER flag), and timerHardwareOverride() takes an + * `isCanonicalBeeperPad` bool: only the first (canonical) pad on a + * physical timer set to OUTPUT_MODE_BEEPER gets TIM_USE_BEEPER; sibling + * pads on that same timer fall back to TIM_USE_PINIO (GPIO-only — + * binary on/off, no PWM, since that doesn't need the timer's fixed + * buzzer-tone period). This is the exact same fix pattern later applied + * to LED (see pwm_mapping_led_unittest.cc) via a parallel + * `isCanonicalLedPad` parameter. + * + * NOTE ON DISCOVERED STALENESS: this file previously (incorrectly) still + * tested the step-2 guard-based logic long after the real source had moved + * to step-3 — it kept passing the whole time because it only exercised its + * own frozen, hand-copied logic and never re-checked the live file. It has + * been rewritten here to match the CURRENT source in + * src/main/drivers/pwm_mapping.c, and a SourceSync test suite (see bottom + * of this file) has been added specifically to catch this class of drift + * automatically in the future. * * This file contains: - * 1. A minimal inline reproduction of both the BUGGY and FIXED versions of - * timerHardwareOverride() so that the test is fully self-contained - * (pwm_mapping.c is excluded from the SITL/unit build by its own guard). - * 2. TEST BugReproduction — demonstrates that the bug corrupts TIM_USE_BEEPER. - * This test would FAIL on pre-fix code and PASSES on the fixed code. - * 3. TEST FixVerification — positive assertion that TIM_USE_BEEPER survives - * after a servo-mode override on a beeper timer. - * 4. Negative / regression tests covering normal (non-beeper) timers to - * confirm that the guard does not break the existing override behaviour. + * 1. A minimal inline reproduction of both a BUGGY version (unconditional + * TIM_USE_BEEPER on every pad of a BEEPER-mode timer, no canonical-pad + * tracking — the same bug class fixed by commit 8c16aeed85) and the + * CURRENT FIXED version of timerHardwareOverride(), since pwm_mapping.c + * is excluded from the SITL/unit build by its own + * `#if !defined(SITL_BUILD)` guard and depends on PG (parameter-group) + * infrastructure not available in a host unit build. + * 2. TEST BugReproduction — demonstrates that the old, unconditional-BEEPER + * code gives TIM_USE_BEEPER to every pad sharing a BEEPER-mode timer. + * 3. TEST FixVerification — only the canonical (first) pad on a shared + * timer gets TIM_USE_BEEPER; sibling pads fall back to TIM_USE_PINIO. + * 4. Multi-pad / multi-timer canonical-tracking tests. + * 5. Regression tests for MOTORS/SERVOS/LED/PINIO overrides on the fixed + * function, including timers mixed with a BEEPER-mode timer. + * 6. TEST SUITE SourceSync — reads the ACTUAL LIVE + * src/main/drivers/pwm_mapping.c off disk at test time and asserts the + * real timerHardwareOverride() BEEPER-case source text still matches + * the logic the inline reproduction above assumes, so this file cannot + * silently go stale again the way it did before. */ #include #include +#include +#include +#include +#include #include "gtest/gtest.h" #include "unittest_macros.h" /* ------------------------------------------------------------------------- - * Minimal type/flag definitions — mirrors the real firmware headers. - * We avoid pulling in the real headers because they drag in hundreds of - * platform-specific includes that cannot be satisfied in a host build. + * Minimal type/flag definitions — mirrors the real firmware headers on this + * branch (src/main/drivers/timer.h, src/main/flight/mixer.h). This branch + * has TIM_USE_PINIO / TIM_USE_BEEPER and OUTPUT_MODE_PINIO / OUTPUT_MODE_BEEPER + * (unlike release/9.1). * ------------------------------------------------------------------------- */ /* timerUsageFlag_e — must match src/main/drivers/timer.h exactly */ @@ -45,6 +93,7 @@ typedef enum { TIM_USE_SERVO = (1 << 3), TIM_USE_LED = (1 << 24), TIM_USE_BEEPER = (1 << 25), + TIM_USE_PINIO = (1 << 26), } timerUsageFlag_e; /* outputMode_e — must match src/main/flight/mixer.h exactly */ @@ -53,13 +102,17 @@ typedef enum { OUTPUT_MODE_MOTORS, OUTPUT_MODE_SERVOS, OUTPUT_MODE_LED, - OUTPUT_MODE_PINIO + OUTPUT_MODE_PINIO, + OUTPUT_MODE_BEEPER } outputMode_e; -/* Minimal timer hardware entry — only the fields exercised by the function. */ +/* Minimal timer hardware entry — only the fields exercised by the function. + * `timId` stands in for the real code's timer2id(timer->tim) — in the real + * driver, multiple timerHardware[] pads sharing the same physical timer + * compare equal on timer2id(); here we just store the id directly. */ typedef struct timerHardware_s { uint32_t usageFlags; - /* All other fields (tim, tag, channel, …) are unused in this test. */ + uint8_t timId; } timerHardware_t; /* ------------------------------------------------------------------------- @@ -69,40 +122,52 @@ typedef struct timerHardware_s { * a) pwm_mapping.c is wrapped in #if !defined(SITL_BUILD) and the unit-test * target.h defines SITL_BUILD, so the real file compiles to nothing. * b) The function depends on timerOverrides() (parameter-group accessor) and - * timer2id(), which are platform / PG infrastructure that is not present in - * a host unit-test build. + * timer2id(), which are platform / PG infrastructure that is not present + * in a host unit-test build. * - * The functions below are literal translations of the C source with the PG - * indirection collapsed into a simple `outputMode` parameter. + * The functions below are literal translations of the current C source + * (post BEEPER-fix, post LED-fix) with the PG indirection collapsed into a + * simple `outputMode` parameter, and canonical-pad tracking passed in + * explicitly as bool parameters (mirroring isCanonicalBeeperPad / + * isCanonicalLedPad computed by pwmBuildTimerOutputList()). + * + * See the SourceSync test suite at the bottom of this file for a mechanism + * that catches this reproduction going out of sync with the real source. * ------------------------------------------------------------------------- */ /* - * BUGGY version (pre-fix, equivalent to the code BEFORE commit 551bce85d6). - * - * The OUTPUT_MODE_SERVOS case clears TIM_USE_MOTOR and TIM_USE_LED but does - * NOT clear TIM_USE_BEEPER. The resulting combined flags - * (TIM_USE_SERVO | TIM_USE_BEEPER) will later be stripped by pwmAssignOutput() - * to just TIM_USE_SERVO, silently destroying the beeper timer entry. + * BUGGY version — reproduces the pre-8c16aeed85 BEEPER behaviour: + * OUTPUT_MODE_BEEPER unconditionally sets TIM_USE_BEEPER on every pad + * sharing the timer, with no canonical-pad tracking at all. MOTORS/SERVOS/ + * LED/PINIO cases are included only for contrast; this reproduction focuses + * on the BEEPER bug. */ static void timerHardwareOverride_buggy(timerHardware_t *timer, - outputMode_e outputMode) + outputMode_e outputMode) { - /* NOTE: no beeper guard here — this is intentionally the buggy version */ switch (outputMode) { case OUTPUT_MODE_MOTORS: - timer->usageFlags &= ~((uint32_t)(TIM_USE_SERVO | TIM_USE_LED)); - timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; + timer->usageFlags &= ~(uint32_t)(TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; break; case OUTPUT_MODE_SERVOS: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_LED)); - timer->usageFlags |= (uint32_t)TIM_USE_SERVO; + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_SERVO; break; case OUTPUT_MODE_LED: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO)); - timer->usageFlags |= (uint32_t)TIM_USE_LED; + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_LED; break; case OUTPUT_MODE_PINIO: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED)); + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + break; + case OUTPUT_MODE_BEEPER: + /* BUG: unconditionally sets TIM_USE_BEEPER on every pad of this + * timer, even though only one pad is actually wired up by + * beeperPwmInit(). No canonical-pad tracking whatsoever. */ + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED); + timer->usageFlags |= (uint32_t)TIM_USE_BEEPER; break; default: break; @@ -110,266 +175,554 @@ static void timerHardwareOverride_buggy(timerHardware_t *timer, } /* - * FIXED version (post-fix, matches commit 551bce85d6 exactly). - * - * The beeper guard causes the function to return immediately when - * TIM_USE_BEEPER is set, leaving the flags completely untouched. + * FIXED version — literal translation of the current + * src/main/drivers/pwm_mapping.c timerHardwareOverride() on this branch. + * Kept in sync with the real source by the SourceSync tests below. */ static void timerHardwareOverride_fixed(timerHardware_t *timer, - outputMode_e outputMode) + outputMode_e outputMode, + bool isCanonicalBeeperPad, + bool isCanonicalLedPad) { - /* Guard added by the fix: never modify a beeper timer. */ - if (timer->usageFlags & (uint32_t)TIM_USE_BEEPER) { - return; - } switch (outputMode) { case OUTPUT_MODE_MOTORS: - timer->usageFlags &= ~((uint32_t)(TIM_USE_SERVO | TIM_USE_LED)); - timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; + timer->usageFlags &= ~(uint32_t)(TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_MOTOR; break; case OUTPUT_MODE_SERVOS: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_LED)); - timer->usageFlags |= (uint32_t)TIM_USE_SERVO; + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_SERVO; break; case OUTPUT_MODE_LED: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO)); - timer->usageFlags |= (uint32_t)TIM_USE_LED; + // Other channels of this timer share its period register, so they can't + // be repurposed as motor/servo/BEEPER either. Only the first (canonical) + // pad is the one ws2811LedStripInit() actually wires up; siblings can still + // drive a GPIO-only PINIO (binary on/off, no PWM), since that doesn't need + // the timer's fixed WS2811-bitrate period. + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_BEEPER | TIM_USE_LED); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalLedPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_LED; + } break; case OUTPUT_MODE_PINIO: - timer->usageFlags &= ~((uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED)); + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + break; + case OUTPUT_MODE_BEEPER: + // Other channels of this timer share its period register, so they can't + // be repurposed as motor/servo/LED either. They can still drive a GPIO-only + // PINIO (binary on/off, no PWM), since that doesn't need the timer's period. + timer->usageFlags &= ~(uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED | TIM_USE_BEEPER); + timer->usageFlags |= (uint32_t)TIM_USE_PINIO; + if (isCanonicalBeeperPad) { + timer->usageFlags &= ~(uint32_t)TIM_USE_PINIO; + timer->usageFlags |= (uint32_t)TIM_USE_BEEPER; + } break; default: break; } } -/* ========================================================================= - * Helper — simulate what pwmAssignOutput(MAP_TO_SERVO_OUTPUT) does to flags. - * In real code: timHw->usageFlags &= TIM_USE_SERVO; - * We need this to show the full two-step corruption path. - * ========================================================================= */ -static void pwmAssignServo_stripFlags(timerHardware_t *timHw) +/* + * Helper — reproduces the canonical-pad-tracking loop in + * pwmBuildTimerOutputList(): scans pads in ascending array order and applies + * timerHardwareOverride_fixed() with isCanonicalBeeperPad / isCanonicalLedPad + * computed exactly as the real code does (first pad seen per physical timer, + * for a timer whose overridden outputMode matches, wins canonical status). + */ +static void applyFixedOverridesToArray(timerHardware_t *pads, int count, + const outputMode_e *outputModeByPad, + int maxTimId) { - timHw->usageFlags &= (uint32_t)TIM_USE_SERVO; + bool beeperClaimed[32] = { false }; + bool ledClaimed[32] = { false }; + ASSERT_LT(maxTimId, 32); + + for (int idx = 0; idx < count; idx++) { + timerHardware_t *timHw = &pads[idx]; + uint8_t timId = timHw->timId; + outputMode_e mode = outputModeByPad[idx]; + + bool isCanonicalBeeperPad = false; + if (mode == OUTPUT_MODE_BEEPER && !beeperClaimed[timId]) { + beeperClaimed[timId] = true; + isCanonicalBeeperPad = true; + } + bool isCanonicalLedPad = false; + if (mode == OUTPUT_MODE_LED && !ledClaimed[timId]) { + ledClaimed[timId] = true; + isCanonicalLedPad = true; + } + timerHardwareOverride_fixed(timHw, mode, isCanonicalBeeperPad, isCanonicalLedPad); + } } /* ========================================================================= - * TEST SUITE: BeeperTimerProtection + * TEST SUITE: BeeperTimerSiblingPads * ========================================================================= */ -class BeeperTimerProtectionTest : public ::testing::Test { -protected: - timerHardware_t timer; - - void SetUp() override { - /* Simulate MATEKH743 TIM2/PA15: beeper shares a servo-capable timer */ - timer.usageFlags = (uint32_t)(TIM_USE_BEEPER | TIM_USE_SERVO); - } -}; - /* * TEST 1 — BugReproduction * - * This test demonstrates the pre-fix bug. On the BUGGY code path: - * 1. timerHardwareOverride_buggy() applies OUTPUT_MODE_SERVOS, which does - * NOT clear TIM_USE_BEEPER → flags become TIM_USE_SERVO | TIM_USE_BEEPER. - * 2. pwmAssignOutput(servo) strips everything except TIM_USE_SERVO - * → TIM_USE_BEEPER is lost. - * - * The final assertion checks that TIM_USE_BEEPER was lost, which is the - * observable symptom of the bug. This test PASSES (i.e. the bug reproduces), - * which is what we want from the reproduction test. + * Two pads share timer id 5, both overridden to OUTPUT_MODE_BEEPER. On the + * BUGGY code path (no canonical-pad tracking at all), BOTH pads get + * TIM_USE_BEEPER, even though only one of them will ever actually be wired + * up by beeperPwmInit(). This test PASSES (i.e. the bug reproduces) on the + * buggy code — confirming the observable symptom of the bug that commit + * 8c16aeed85 fixed. */ -TEST_F(BeeperTimerProtectionTest, BugReproduction_BeeperFlagLostAfterOverride) +TEST(BeeperTimerSiblingPads, BugReproduction_BothSiblingsGetBeeperFlag) { - /* Step 1: buggy override — does not protect beeper */ - timerHardwareOverride_buggy(&timer, OUTPUT_MODE_SERVOS); - - /* After the buggy override: both SERVO and BEEPER flags should be set - * because OUTPUT_MODE_SERVOS only clears MOTOR and LED, not BEEPER. */ - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_SERVO) - << "SERVO flag should be set by the OUTPUT_MODE_SERVOS override"; - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER) - << "BEEPER flag should still be present at this point (intermediate state)"; - - /* Step 2: simulate pwmAssignOutput(servo) which strips everything - * except TIM_USE_SERVO — this is the second half of the corruption. */ - pwmAssignServo_stripFlags(&timer); - - /* The bug: TIM_USE_BEEPER is now gone. beeperPwmInit() will fail silently. */ - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER) - << "BUG CONFIRMED: TIM_USE_BEEPER was stripped by pwmAssignOutput " - "because timerHardwareOverride() did not protect it. " - "beeperPwmInit() would fail to find its timer."; + timerHardware_t padA; padA.usageFlags = 0; padA.timId = 5; + timerHardware_t padB; padB.usageFlags = 0; padB.timId = 5; + + timerHardwareOverride_buggy(&padA, OUTPUT_MODE_BEEPER); + timerHardwareOverride_buggy(&padB, OUTPUT_MODE_BEEPER); + + EXPECT_TRUE(padA.usageFlags & (uint32_t)TIM_USE_BEEPER) + << "First pad should get TIM_USE_BEEPER (expected)"; + EXPECT_TRUE(padB.usageFlags & (uint32_t)TIM_USE_BEEPER) + << "BUG CONFIRMED: sibling pad on the SAME timer also got TIM_USE_BEEPER, " + "but beeperPwmInit() only wires up the first pad found for the buzzer. " + "This sibling would be falsely reported as \"Buzzer\" over MSP/in the " + "configurator while being functionally dead (oscilloscope showed a real " + "waveform only on the first pad of the timer)."; } /* - * TEST 2 — FixVerification - * - * The fixed timerHardwareOverride() returns immediately when TIM_USE_BEEPER is - * set, so the servo override is a no-op for beeper timers. TIM_USE_BEEPER must - * survive, allowing beeperPwmInit() to find the timer later. + * TEST 2 — FixVerification (two sibling pads) * - * This test PASSES on the fixed code and would FAIL on the buggy code. + * Same scenario as above, but using the FIXED function with proper + * canonical-pad computation (as pwmBuildTimerOutputList() does). Only the + * first-scanned pad on the shared timer should get TIM_USE_BEEPER; the + * sibling must fall back to TIM_USE_PINIO (GPIO-only), NOT TIM_USE_BEEPER, + * and NOT be left flagless (commit 31bb99ea0b added the PINIO fallback). */ -TEST_F(BeeperTimerProtectionTest, FixVerification_BeeperFlagSurvivesOverride) +TEST(BeeperTimerSiblingPads, FixVerification_OnlyCanonicalPadGetsBeeper) { - const uint32_t originalFlags = timer.usageFlags; - - /* Fixed override: should be a no-op because TIM_USE_BEEPER is set */ - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_SERVOS); - - /* Flags must be completely unchanged — the guard must have returned early. */ - EXPECT_EQ(timer.usageFlags, originalFlags) - << "FIX VERIFIED: timerHardwareOverride() must not modify a timer " - "that has TIM_USE_BEEPER set."; - - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER) - << "TIM_USE_BEEPER must still be set after the override attempt"; + timerHardware_t pads[2]; + pads[0].usageFlags = 0; pads[0].timId = 5; + pads[1].usageFlags = 0; pads[1].timId = 5; + outputMode_e modes[2] = { OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER }; + + applyFixedOverridesToArray(pads, 2, modes, 6); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_BEEPER) + << "Canonical (first-scanned) pad must get TIM_USE_BEEPER"; + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Canonical pad must not also carry the PINIO fallback flag"; + + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_BEEPER) + << "FIX VERIFIED: sibling pad on the same timer must NOT get TIM_USE_BEEPER"; + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO) + << "Sibling pad must fall back to TIM_USE_PINIO (GPIO-only), not be left flagless"; } /* - * TEST 3 — FixVerification for OUTPUT_MODE_MOTORS - * - * Confirm the beeper guard works for all output-mode variants, not just SERVOS. + * TEST 3 — Three-pad case: only the first scanned pad is canonical */ -TEST_F(BeeperTimerProtectionTest, FixVerification_BeeperProtectedFromMotorOverride) +TEST(BeeperTimerSiblingPads, ThreePads_OnlyFirstScannedIsCanonical) { - const uint32_t originalFlags = timer.usageFlags; + timerHardware_t pads[3]; + for (int i = 0; i < 3; i++) { + pads[i].usageFlags = 0; + pads[i].timId = 7; + } + outputMode_e modes[3] = { OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER }; - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_MOTORS); + applyFixedOverridesToArray(pads, 3, modes, 8); - EXPECT_EQ(timer.usageFlags, originalFlags) - << "OUTPUT_MODE_MOTORS must not modify a beeper timer"; - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_BEEPER) + << "pads[0] (first scanned) must be canonical and get TIM_USE_BEEPER"; + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO); + + for (int i = 1; i < 3; i++) { + EXPECT_FALSE(pads[i].usageFlags & (uint32_t)TIM_USE_BEEPER) + << "pads[" << i << "] must NOT be canonical, so must not get TIM_USE_BEEPER"; + EXPECT_TRUE(pads[i].usageFlags & (uint32_t)TIM_USE_PINIO) + << "pads[" << i << "] must fall back to TIM_USE_PINIO"; + } } /* - * TEST 4 — FixVerification for OUTPUT_MODE_LED + * TEST 4 — Two independent BEEPER timers each get their own canonical pad */ -TEST_F(BeeperTimerProtectionTest, FixVerification_BeeperProtectedFromLedOverride) +TEST(BeeperTimerSiblingPads, IndependentTimers_EachGetsOwnCanonicalPad) { - const uint32_t originalFlags = timer.usageFlags; - - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_LED); - - EXPECT_EQ(timer.usageFlags, originalFlags) - << "OUTPUT_MODE_LED must not modify a beeper timer"; - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); + timerHardware_t pads[4]; + pads[0].usageFlags = 0; pads[0].timId = 5; // timer A, pad 1 -> canonical + pads[1].usageFlags = 0; pads[1].timId = 5; // timer A, pad 2 -> sibling + pads[2].usageFlags = 0; pads[2].timId = 9; // timer B, pad 1 -> canonical + pads[3].usageFlags = 0; pads[3].timId = 9; // timer B, pad 2 -> sibling + outputMode_e modes[4] = { OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER, OUTPUT_MODE_BEEPER }; + + applyFixedOverridesToArray(pads, 4, modes, 10); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO); + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_BEEPER); + + EXPECT_TRUE(pads[2].usageFlags & (uint32_t)TIM_USE_BEEPER) + << "Timer B's first pad must independently become canonical"; + EXPECT_TRUE(pads[3].usageFlags & (uint32_t)TIM_USE_PINIO); + EXPECT_FALSE(pads[3].usageFlags & (uint32_t)TIM_USE_BEEPER); } /* - * TEST 5 — FixVerification for OUTPUT_MODE_PINIO + * TEST 5 — Edge case: a pure BEEPER-only timer (single pad) still gets + * TIM_USE_BEEPER — the canonical-pad logic must not accidentally suppress + * the common single-pad case. */ -TEST_F(BeeperTimerProtectionTest, FixVerification_BeeperProtectedFromPinioOverride) +TEST(BeeperTimerSiblingPads, SinglePadTimer_StillGetsBeeperFlag) { - const uint32_t originalFlags = timer.usageFlags; + timerHardware_t pads[1]; + pads[0].usageFlags = 0; + pads[0].timId = 2; + outputMode_e modes[1] = { OUTPUT_MODE_BEEPER }; - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_PINIO); + applyFixedOverridesToArray(pads, 1, modes, 3); - EXPECT_EQ(timer.usageFlags, originalFlags) - << "OUTPUT_MODE_PINIO must not modify a beeper timer"; - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_FALSE(pads[0].usageFlags & (uint32_t)TIM_USE_PINIO); } /* ========================================================================= - * Negative / regression tests — normal (non-beeper) timers must still be - * overridden correctly. The guard must NOT prevent normal overrides. + * Negative / regression tests — MOTORS/SERVOS/LED/PINIO overrides must still + * work correctly with the fixed function, including when mixed with a + * BEEPER-mode timer in the same scan. * ========================================================================= */ -class NormalTimerOverrideTest : public ::testing::Test { -protected: +TEST(BeeperTimerRegression, MotorOverride_Unaffected) +{ timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 1; - /* No TIM_USE_BEEPER: both motor and servo flags set (auto-mode timer). */ - void SetUp() override { - timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); - } -}; + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_MOTORS, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); +} -TEST_F(NormalTimerOverrideTest, MotorOverride_SetsMotorClearsServoAndLed) +TEST(BeeperTimerRegression, ServoOverride_Unaffected) { - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_MOTORS); - - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR) - << "TIM_USE_MOTOR must be set after OUTPUT_MODE_MOTORS override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO) - << "TIM_USE_SERVO must be cleared by OUTPUT_MODE_MOTORS override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED) - << "TIM_USE_LED must be cleared by OUTPUT_MODE_MOTORS override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER) - << "TIM_USE_BEEPER must remain clear on a non-beeper timer"; + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 2; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_SERVOS, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_BEEPER); } -TEST_F(NormalTimerOverrideTest, ServoOverride_SetsServoClearsMotorAndLed) +TEST(BeeperTimerRegression, LedCanonicalPad_StillGetsLedFlag) { - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_SERVOS); - - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_SERVO) - << "TIM_USE_SERVO must be set after OUTPUT_MODE_SERVOS override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR) - << "TIM_USE_MOTOR must be cleared by OUTPUT_MODE_SERVOS override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED) - << "TIM_USE_LED must be cleared by OUTPUT_MODE_SERVOS override"; + /* Regression: the LED canonical-pad logic (isCanonicalLedPad) must be + * completely unaffected by the BEEPER-specific test coverage here. */ + timerHardware_t timer; + timer.usageFlags = 0; + timer.timId = 3; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_LED, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/true); + + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_PINIO); } -TEST_F(NormalTimerOverrideTest, LedOverride_SetsLedClearsMotorAndServo) +TEST(BeeperTimerRegression, LedSiblingPad_FallsBackToPinio) { - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_LED); - - EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_LED) - << "TIM_USE_LED must be set after OUTPUT_MODE_LED override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR) - << "TIM_USE_MOTOR must be cleared by OUTPUT_MODE_LED override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO) - << "TIM_USE_SERVO must be cleared by OUTPUT_MODE_LED override"; + timerHardware_t timer; + timer.usageFlags = 0; + timer.timId = 3; + + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_LED, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); + + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_PINIO) + << "Non-canonical LED pad must fall back to PINIO"; } -TEST_F(NormalTimerOverrideTest, PinioOverride_ClearsAllOutputFlags) +TEST(BeeperTimerRegression, PinioOverride_Unaffected) { + timerHardware_t timer; timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO | TIM_USE_LED); + timer.timId = 4; - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_PINIO); + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_PINIO, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR) - << "TIM_USE_MOTOR must be cleared by OUTPUT_MODE_PINIO override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO) - << "TIM_USE_SERVO must be cleared by OUTPUT_MODE_PINIO override"; - EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED) - << "TIM_USE_LED must be cleared by OUTPUT_MODE_PINIO override"; + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_MOTOR); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_SERVO); + EXPECT_FALSE(timer.usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(timer.usageFlags & (uint32_t)TIM_USE_PINIO); } -TEST_F(NormalTimerOverrideTest, AutoMode_LeavesTimerUnchanged) +TEST(BeeperTimerRegression, AutoMode_LeavesTimerUnchanged) { + timerHardware_t timer; + timer.usageFlags = (uint32_t)(TIM_USE_MOTOR | TIM_USE_SERVO); + timer.timId = 6; const uint32_t originalFlags = timer.usageFlags; - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_AUTO); + timerHardwareOverride_fixed(&timer, OUTPUT_MODE_AUTO, + /*isCanonicalBeeperPad=*/false, + /*isCanonicalLedPad=*/false); EXPECT_EQ(timer.usageFlags, originalFlags) << "OUTPUT_MODE_AUTO (default case) must not change flags"; } +/* + * TEST — Mixed scan: MOTOR timer, BEEPER timer (2 pads), LED timer + * (canonical pad only) all processed together via + * applyFixedOverridesToArray(), matching a realistic + * pwmBuildTimerOutputList() scenario where different timers on the board + * have different output-mode overrides. Confirms none of the per-timer + * canonical tracking (beeperClaimed[] vs ledClaimed[]) interferes across + * timer ids or across output modes — the mirror image of the equivalent test + * in pwm_mapping_led_unittest.cc. + */ +TEST(BeeperTimerRegression, MixedScan_MotorBeeperLedTimersDoNotInterfere) +{ + timerHardware_t pads[5]; + for (int i = 0; i < 5; i++) pads[i].usageFlags = 0; + + pads[0].timId = 1; // MOTOR timer + pads[1].timId = 2; // BEEPER timer, canonical + pads[2].timId = 2; // BEEPER timer, sibling + pads[3].timId = 3; // LED timer, canonical + pads[4].timId = 3; // LED timer, sibling + + outputMode_e modes[5] = { + OUTPUT_MODE_MOTORS, + OUTPUT_MODE_BEEPER, + OUTPUT_MODE_BEEPER, + OUTPUT_MODE_LED, + OUTPUT_MODE_LED, + }; + + applyFixedOverridesToArray(pads, 5, modes, 4); + + EXPECT_TRUE(pads[0].usageFlags & (uint32_t)TIM_USE_MOTOR); + + EXPECT_TRUE(pads[1].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_FALSE(pads[1].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_FALSE(pads[2].usageFlags & (uint32_t)TIM_USE_BEEPER); + EXPECT_TRUE(pads[2].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_TRUE(pads[3].usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_FALSE(pads[3].usageFlags & (uint32_t)TIM_USE_PINIO); + + EXPECT_FALSE(pads[4].usageFlags & (uint32_t)TIM_USE_LED); + EXPECT_TRUE(pads[4].usageFlags & (uint32_t)TIM_USE_PINIO); +} + /* ========================================================================= - * Edge case: timer has ONLY TIM_USE_BEEPER (no motor/servo capability) + * TEST SUITE: SourceSync + * + * Reads the ACTUAL LIVE src/main/drivers/pwm_mapping.c off disk (not a + * cached copy, not something computed at CMake-configure time) and checks + * that specific, logic-relevant fragments of the real timerHardwareOverride() + * and pwmBuildTimerOutputList() are still present verbatim (modulo + * whitespace and comments). If a future change to pwm_mapping.c alters this + * logic without updating the inline reproduction above, these tests fail — + * loudly, in `make check` / CI — instead of the rest of the file silently + * continuing to pass against a stale copy, which is exactly what happened to + * this file for months after commits 8c16aeed85/31bb99ea0b. + * + * This does not (and structurally cannot, given the SITL_BUILD guard around + * the entire file — see block comment above) substitute for actually linking + * and executing the real function. It is a drift-detector, not a full + * integration test. Comments are stripped and whitespace is collapsed before + * comparison so that pure reformatting/typo-fixes in comments do not cause + * false failures; only the executable-statement tokens are checked. * ========================================================================= */ -TEST(BeeperOnlyTimer, OverrideIgnoredForPureBeeper) +namespace { + +// Strips "//...\n" line comments. Deliberately does not handle /* */ block +// comments or comments inside string/char literals — pwm_mapping.c does not +// use either within the functions we check, and keeping this simple avoids +// this drift-detector itself becoming a source of drift. +std::string stripLineComments(const std::string &text) { - timerHardware_t timer; - timer.usageFlags = (uint32_t)TIM_USE_BEEPER; + std::string result; + result.reserve(text.size()); + for (size_t i = 0; i < text.size();) { + if (text[i] == '/' && i + 1 < text.size() && text[i + 1] == '/') { + while (i < text.size() && text[i] != '\n') { + i++; + } + continue; + } + result += text[i]; + i++; + } + return result; +} - const uint32_t originalFlags = timer.usageFlags; +// Collapses any run of whitespace (including newlines) to a single space, +// and trims leading/trailing whitespace, so indentation/line-wrap changes +// don't cause false failures. +std::string normalizeWhitespace(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + bool lastWasSpace = true; // trims leading whitespace + for (char c : text) { + if (std::isspace(static_cast(c))) { + if (!lastWasSpace) { + result += ' '; + lastWasSpace = true; + } + } else { + result += c; + lastWasSpace = false; + } + } + while (!result.empty() && result.back() == ' ') { + result.pop_back(); + } + return result; +} - /* All override modes must be no-ops for a pure beeper timer. */ - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_MOTORS); - EXPECT_EQ(timer.usageFlags, originalFlags); +std::string normalizeSource(const std::string &text) +{ + return normalizeWhitespace(stripLineComments(text)); +} - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_SERVOS); - EXPECT_EQ(timer.usageFlags, originalFlags); +// Locates the live pwm_mapping.c relative to this test file's own path. +// __FILE__ is emitted as an absolute path by this project's CMake/Makefiles +// (verified: `c++ ... -c /abs/path/to/pwm_mapping_beeper_unittest.cc`), so +// this resolves correctly regardless of the build directory or CWD the test +// is invoked from. +std::string pwmMappingSourcePath() +{ + std::string thisFile = __FILE__; // .../src/test/unit/pwm_mapping_beeper_unittest.cc + size_t lastSlash = thisFile.find_last_of("/\\"); + std::string testUnitDir = (lastSlash == std::string::npos) ? "." : thisFile.substr(0, lastSlash); + return testUnitDir + "/../../main/drivers/pwm_mapping.c"; +} - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_LED); - EXPECT_EQ(timer.usageFlags, originalFlags); +// Reads and normalizes pwm_mapping.c. Fails the calling test LOUDLY (not +// silently returning empty / skipping) if the file can't be found, since a +// missing file here means this drift-detector isn't detecting anything. +::testing::AssertionResult loadNormalizedPwmMappingSource(std::string *out) +{ + const std::string path = pwmMappingSourcePath(); + std::ifstream file(path); + if (!file.is_open()) { + return ::testing::AssertionFailure() + << "Could not open live source file at computed path: " << path + << " -- SourceSync tests cannot verify anything against dead/absent " + "source. Check that src/test/unit/ and src/main/drivers/ still " + "have their expected relative layout."; + } + std::ostringstream buffer; + buffer << file.rdbuf(); + if (buffer.str().empty()) { + return ::testing::AssertionFailure() + << "Live source file at " << path << " opened but read as empty."; + } + *out = normalizeSource(buffer.str()); + return ::testing::AssertionSuccess(); +} - timerHardwareOverride_fixed(&timer, OUTPUT_MODE_PINIO); - EXPECT_EQ(timer.usageFlags, originalFlags); +} // namespace + +TEST(SourceSync, TimerHardwareOverrideSignatureMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedSignature = normalizeSource( + "static void timerHardwareOverride(timerHardware_t * timer, " + "bool isCanonicalBeeperPad, bool isCanonicalLedPad) {"); + + EXPECT_NE(normalizedSource.find(expectedSignature), std::string::npos) + << "timerHardwareOverride()'s signature in the LIVE " + "src/main/drivers/pwm_mapping.c no longer matches what this test's " + "inline reproduction assumes. If the signature legitimately " + "changed (e.g. the step-2 guard-based approach was reinstated, or " + "a new parameter was added), update timerHardwareOverride_fixed()/" + "_buggy() in this file (and pwm_mapping_led_unittest.cc) to match, " + "then update this expected string too. THIS is the exact class of " + "drift that made this file stale for months previously — do not " + "let it happen again."; +} + +TEST(SourceSync, BeeperCaseMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + // Comments intentionally omitted here (stripLineComments() removes them + // from the live source before comparison too) so that comment wording + // changes don't cause false failures; only the executable statements are + // checked, which is exactly the logic the FixVerification tests above + // depend on. + const std::string expectedBeeperCase = normalizeSource( + "case OUTPUT_MODE_BEEPER: " + "timer->usageFlags &= ~(TIM_USE_MOTOR|TIM_USE_SERVO|TIM_USE_LED|TIM_USE_BEEPER); " + "timer->usageFlags |= TIM_USE_PINIO; " + "if (isCanonicalBeeperPad) { " + "timer->usageFlags &= ~TIM_USE_PINIO; " + "timer->usageFlags |= TIM_USE_BEEPER; " + "} " + "break;"); + + EXPECT_NE(normalizedSource.find(expectedBeeperCase), std::string::npos) + << "The OUTPUT_MODE_BEEPER case in the LIVE " + "src/main/drivers/pwm_mapping.c no longer matches " + "timerHardwareOverride_fixed()'s BEEPER case in this test file. " + "Update the inline reproduction (and the FixVerification/" + "BugReproduction tests, if the fix strategy itself changed) to " + "match the live source."; +} + +TEST(SourceSync, CanonicalPadTrackingLoopMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPwmMappingSource(&normalizedSource)); + + const std::string expectedLoop = normalizeSource( + "bool beeperClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; " + "bool ledClaimed[HARDWARE_TIMER_DEFINITION_COUNT] = { false }; " + "for (int idx = 0; idx < timerHardwareCount; idx++) { " + "timerHardware_t *timHw = &timerHardware[idx]; " + "uint8_t timId = timer2id(timHw->tim); " + "bool isCanonicalBeeperPad = false; " + "if (timerOverrides(timId)->outputMode == OUTPUT_MODE_BEEPER && !beeperClaimed[timId]) { " + "beeperClaimed[timId] = true; " + "isCanonicalBeeperPad = true; " + "} " + "bool isCanonicalLedPad = false; " + "if (timerOverrides(timId)->outputMode == OUTPUT_MODE_LED && !ledClaimed[timId]) { " + "ledClaimed[timId] = true; " + "isCanonicalLedPad = true; " + "}"); + + EXPECT_NE(normalizedSource.find(expectedLoop), std::string::npos) + << "pwmBuildTimerOutputList()'s canonical-pad-tracking loop in the LIVE " + "src/main/drivers/pwm_mapping.c no longer matches what " + "applyFixedOverridesToArray() in this test file reproduces. Update " + "applyFixedOverridesToArray() to match the live source (e.g. if the " + "beeperClaimed[]/ledClaimed[] scanning strategy changes)."; } From 1475a6cc52d9aa6f71f0bb78d0eacdd518add27d Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Sun, 26 Jul 2026 11:59:29 -0500 Subject: [PATCH 3/3] pinio: don't PWM-configure a PINIO pad sharing a timer with LED/BEEPER pinioInit() would happily configure a TIM_USE_PINIO-flagged pad as timer-PWM via pinioInitTimerPWM(), which calls timerConfigBase() to set the timer's period/prescaler. That register is shared by every channel on the physical timer, so if the pad's sibling channel is dedicated to a WS2811 LED strip or BEEPER tone, this silently reprograms the shared period. beeperInit() runs before pinioInit() with nothing to re-assert it afterward, so a BEEPER sibling getting PWM-configured as PINIO can permanently corrupt the tone frequency. Add timerHasFixedPeriodSibling() and gate both pinioInitTimerPWM() call sites in pinioInit() on it, so any pad sharing a timer with an LED or BEEPER output falls through to the existing GPIO-only fallback instead. Found via code review of the LED sibling-pad-flags fix in this branch, which relies on siblings correctly falling back to GPIO-only PINIO. --- src/main/drivers/pinio.c | 23 +- src/test/unit/pinio_timer_sibling_unittest.cc | 432 ++++++++++++++++++ 2 files changed, 452 insertions(+), 3 deletions(-) create mode 100644 src/test/unit/pinio_timer_sibling_unittest.cc diff --git a/src/main/drivers/pinio.c b/src/main/drivers/pinio.c index 700a329979d..9c9c9caafe4 100644 --- a/src/main/drivers/pinio.c +++ b/src/main/drivers/pinio.c @@ -110,6 +110,23 @@ static bool pinioInitTimerPWM(int slot, IO_t io, const timerHardware_t *timHw, b return true; } +// True if another channel on the same physical timer is dedicated to a fixed-period +// output (WS2811 LED strip or BEEPER tone). pinioInitTimerPWM() has no per-timer +// exclusivity check of its own -- timerGetTCH() will happily hand out a TCH for any +// channel regardless of what else is running on that timer -- so without this guard, +// configuring such a pad as timer-PWM would call timerConfigBase() and reprogram the +// period/prescaler shared by every channel on the timer, corrupting the fixed-period +// output it belongs to. +static bool timerHasFixedPeriodSibling(const timerHardware_t *timHw) +{ + for (int i = 0; i < timerHardwareCount; i++) { + if (timerHardware[i].tim == timHw->tim && (timerHardware[i].usageFlags & (TIM_USE_LED | TIM_USE_BEEPER))) { + return true; + } + } + return false; +} + void pinioInit(void) { int runtimeCount = 0; @@ -126,7 +143,7 @@ void pinioInit(void) bool inverted = (pinioHardware[i].flags & PINIO_FLAGS_INVERTED) != 0; const timerHardware_t *timHw = timerGetByTag(pinioHardware[i].ioTag, TIM_USE_ANY); - if (timHw && IOGetOwner(io) == OWNER_FREE && pinioInitTimerPWM(runtimeCount, io, timHw, inverted)) { + if (timHw && IOGetOwner(io) == OWNER_FREE && !timerHasFixedPeriodSibling(timHw) && pinioInitTimerPWM(runtimeCount, io, timHw, inverted)) { runtimeCount++; continue; } @@ -152,13 +169,13 @@ void pinioInit(void) if (!io || IOGetOwner(io) != OWNER_FREE) { continue; } - if (pinioInitTimerPWM(runtimeCount, io, timHw, false)) { + if (!timerHasFixedPeriodSibling(timHw) && pinioInitTimerPWM(runtimeCount, io, timHw, false)) { runtimeCount++; continue; } // GPIO fallback: pad's timer is already running at a fixed frequency - // for another purpose (e.g. BEEPER), so PWM duty control isn't available. + // for another purpose (e.g. BEEPER or LED), so PWM duty control isn't available. IOInit(io, OWNER_PINIO, RESOURCE_OUTPUT, RESOURCE_INDEX(runtimeCount)); IOConfigGPIO(io, IOCFG_OUT_PP); pinioRuntime[runtimeCount].inverted = false; diff --git a/src/test/unit/pinio_timer_sibling_unittest.cc b/src/test/unit/pinio_timer_sibling_unittest.cc new file mode 100644 index 00000000000..94187e95108 --- /dev/null +++ b/src/test/unit/pinio_timer_sibling_unittest.cc @@ -0,0 +1,432 @@ +/* + * Unit test: pinioInit()'s guard against configuring a PINIO pad as + * timer-PWM when it shares its physical timer with a fixed-period output + * (WS2811 LED strip or BEEPER tone). + * + * Bug context (follow-on issue surfaced by the sibling-pad fix in + * src/main/drivers/pwm_mapping.c, see pwm_mapping_led_unittest.cc / + * pwm_mapping_beeper_unittest.cc): + * + * Once pwm_mapping.c correctly falls sibling pads on an LED/BEEPER timer + * back to TIM_USE_PINIO (instead of falsely claiming TIM_USE_LED), those + * pads reach pinioInit(). pinioInit() calls pinioInitTimerPWM() on any + * TIM_USE_PINIO-flagged pad it finds a TCH for, and pinioInitTimerPWM() + * calls timerConfigBase(tch, PINIO_PWM_PERIOD, PINIO_PWM_BASE_HZ) — but the + * period/prescaler register that timerConfigBase() programs is SHARED by + * every channel on that physical timer. timerGetTCH() has no per-timer + * exclusivity check of its own: it will happily hand back a TCH for the + * PINIO pad regardless of what else (LED strip / BEEPER) is already + * running on that timer. Without a guard, configuring the PINIO sibling as + * PWM would silently reprogram the LED-strip's WS2811 bit-rate period (or + * the BEEPER's tone period), corrupting that output. + * + * Fix: src/main/drivers/pinio.c adds timerHasFixedPeriodSibling(), which + * scans timerHardware[] for any OTHER entry on the SAME physical timer + * (`.tim ==`) carrying TIM_USE_LED or TIM_USE_BEEPER, and gates both + * pinioInitTimerPWM() call sites in pinioInit() on + * `!timerHasFixedPeriodSibling(timHw) && ...`, so such a pad falls back to + * the plain-GPIO path instead. + * + * This file contains: + * 1. A minimal inline reproduction of timerHasFixedPeriodSibling() (and the + * minimal timerHardware_t / usage-flag definitions it needs), since + * pinio.c cannot be linked into the host unit-test binary: it is gated + * by `#ifdef USE_PINIO` and depends on live hardware/timer/IO driver + * infrastructure (IOInit, timerGetTCH, timerConfigBase, etc.) that is + * not mocked for unit tests. + * 2. TEST SUITE TimerHasFixedPeriodSibling — covers: no sibling on the + * timer, LED sibling, BEEPER sibling, a sibling with a matching flag + * but on a DIFFERENT physical timer (verifies the `.tim ==` comparison + * itself is doing real work, not just the flag test), and the pad's own + * entry carrying TIM_USE_PINIO while scanning past it. + * 3. TEST SUITE SourceSync — reads the ACTUAL LIVE + * src/main/drivers/pinio.c off disk at test time and verifies (a) the + * real timerHasFixedPeriodSibling() body still matches this file's + * inline reproduction, and (b) both call sites inside pinioInit() still + * gate on `timerHasFixedPeriodSibling(...)`, so this test cannot + * silently drift from the real implementation and cannot stay green if + * the guard is later removed. This is the same drift-detection pattern + * used by pwm_mapping_led_unittest.cc / pwm_mapping_beeper_unittest.cc + * after pwm_mapping_beeper_unittest.cc was found to have gone stale. + */ + +#include +#include +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "unittest_macros.h" + +/* ------------------------------------------------------------------------- + * Minimal type/flag definitions — mirrors src/main/drivers/timer.h exactly + * for the bits this function cares about. `tim` stands in for the real + * code's `tmr_type *tim` / `TIM_TypeDef *tim` pointer — in the real driver, + * multiple timerHardware[] pads sharing the same physical timer compare + * equal on pointer identity of `.tim`; here we just use a plain int id and + * compare that, which exercises identical logic. + * ------------------------------------------------------------------------- */ + +typedef enum { + TIM_USE_ANY = 0, + TIM_USE_PPM = (1 << 0), + TIM_USE_PWM = (1 << 1), + TIM_USE_MOTOR = (1 << 2), + TIM_USE_SERVO = (1 << 3), + TIM_USE_LED = (1 << 24), + TIM_USE_BEEPER = (1 << 25), + TIM_USE_PINIO = (1 << 26), +} timerUsageFlag_e; + +/* Minimal timer hardware entry — only the fields timerHasFixedPeriodSibling() + * actually touches (`.tim`, `.usageFlags`). */ +typedef struct timerHardware_s { + int tim; // stands in for the real tmr_type*/TIM_TypeDef* pointer identity + uint32_t usageFlags; +} timerHardware_t; + +/* ------------------------------------------------------------------------- + * Inline reproduction of timerHasFixedPeriodSibling(). + * + * We inline this rather than linking pinio.c because pinio.c is gated by + * `#ifdef USE_PINIO` and depends on IOInit/IOConfigGPIOAF/timerGetTCH/ + * timerConfigBase/timerPWMConfigChannel/timerPWMStart/timerEnable — none of + * which exist in the host unit-test build. + * + * This is a literal translation of the current C source. It operates against + * test-local `timerHardware[]` / `timerHardwareCount` globals (declared + * below, mirroring the real extern globals from timer.h) rather than the + * real firmware ones, so each test can set up its own hardware table. + * + * See the SourceSync test suite at the bottom of this file for a mechanism + * that catches this reproduction going out of sync with the real source. + * ------------------------------------------------------------------------- */ + +static timerHardware_t *g_timerHardware = nullptr; +static int g_timerHardwareCount = 0; + +static bool timerHasFixedPeriodSibling(const timerHardware_t *timHw) +{ + for (int i = 0; i < g_timerHardwareCount; i++) { + if (g_timerHardware[i].tim == timHw->tim && (g_timerHardware[i].usageFlags & (TIM_USE_LED | TIM_USE_BEEPER))) { + return true; + } + } + return false; +} + +/* ========================================================================= + * TEST SUITE: TimerHasFixedPeriodSibling + * ========================================================================= */ + +/* + * TEST 1 — No other pad on the timer at all: must return false. + */ +TEST(TimerHasFixedPeriodSibling, NoSibling_ReturnsFalse) +{ + timerHardware_t pads[1]; + pads[0].tim = 1; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; + + g_timerHardware = pads; + g_timerHardwareCount = 1; + + EXPECT_FALSE(timerHasFixedPeriodSibling(&pads[0])); +} + +/* + * TEST 2 — A sibling pad on the SAME physical timer carries TIM_USE_LED: + * must return true. + */ +TEST(TimerHasFixedPeriodSibling, LedSiblingOnSameTimer_ReturnsTrue) +{ + timerHardware_t pads[2]; + pads[0].tim = 5; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; // pad under test + pads[1].tim = 5; pads[1].usageFlags = (uint32_t)TIM_USE_LED; // sibling: LED strip + + g_timerHardware = pads; + g_timerHardwareCount = 2; + + EXPECT_TRUE(timerHasFixedPeriodSibling(&pads[0])) + << "A PINIO pad sharing its timer with an LED-strip pad must be " + "reported as having a fixed-period sibling, so pinioInit() does " + "not reprogram the LED strip's WS2811 period."; +} + +/* + * TEST 3 — A sibling pad on the SAME physical timer carries TIM_USE_BEEPER: + * must return true. + */ +TEST(TimerHasFixedPeriodSibling, BeeperSiblingOnSameTimer_ReturnsTrue) +{ + timerHardware_t pads[2]; + pads[0].tim = 7; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; // pad under test + pads[1].tim = 7; pads[1].usageFlags = (uint32_t)TIM_USE_BEEPER; // sibling: buzzer + + g_timerHardware = pads; + g_timerHardwareCount = 2; + + EXPECT_TRUE(timerHasFixedPeriodSibling(&pads[0])) + << "A PINIO pad sharing its timer with a BEEPER pad must be reported " + "as having a fixed-period sibling, so pinioInit() does not " + "reprogram the buzzer's tone period."; +} + +/* + * TEST 4 — A DIFFERENT physical timer has an LED pad, but the timer under + * test does not. Must return false. This specifically verifies the `.tim ==` + * comparison is doing real work (i.e. the function is not just checking + * "does ANY entry anywhere have TIM_USE_LED|TIM_USE_BEEPER", which would be + * a much easier, but wrong, bug to accidentally introduce). + */ +TEST(TimerHasFixedPeriodSibling, LedOnDifferentTimer_ReturnsFalse) +{ + timerHardware_t pads[2]; + pads[0].tim = 1; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; // pad under test, timer 1 + pads[1].tim = 2; pads[1].usageFlags = (uint32_t)TIM_USE_LED; // LED pad, timer 2 (different!) + + g_timerHardware = pads; + g_timerHardwareCount = 2; + + EXPECT_FALSE(timerHasFixedPeriodSibling(&pads[0])) + << "An LED/BEEPER pad on a DIFFERENT physical timer must not cause a " + "false positive -- the comparison must be scoped to the SAME " + "timer (`.tim ==`), not just a global flag scan."; +} + +/* + * TEST 5 — Same as TEST 4 but with BEEPER instead of LED, for symmetry. + */ +TEST(TimerHasFixedPeriodSibling, BeeperOnDifferentTimer_ReturnsFalse) +{ + timerHardware_t pads[2]; + pads[0].tim = 3; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; + pads[1].tim = 9; pads[1].usageFlags = (uint32_t)TIM_USE_BEEPER; + + g_timerHardware = pads; + g_timerHardwareCount = 2; + + EXPECT_FALSE(timerHasFixedPeriodSibling(&pads[0])); +} + +/* + * TEST 6 — The pad's own entry (in timerHardware[]) carries TIM_USE_PINIO; + * scanning must correctly pass over it (a pad with only TIM_USE_PINIO set + * must not itself be mistaken for a fixed-period sibling) while still + * finding the real LED sibling elsewhere on the same timer. Also exercises + * a 3-pad table so the loop must not stop early. + */ +TEST(TimerHasFixedPeriodSibling, OwnPinioEntryIgnored_SiblingStillFound) +{ + timerHardware_t pads[3]; + pads[0].tim = 4; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; // pad under test (itself in the table) + pads[1].tim = 4; pads[1].usageFlags = (uint32_t)TIM_USE_PINIO; // another PINIO pad, same timer, not fixed-period + pads[2].tim = 4; pads[2].usageFlags = (uint32_t)TIM_USE_LED; // the real fixed-period sibling + + g_timerHardware = pads; + g_timerHardwareCount = 3; + + EXPECT_TRUE(timerHasFixedPeriodSibling(&pads[0])) + << "Own TIM_USE_PINIO entry and a plain-PINIO sibling must not mask " + "the presence of the real LED sibling further down the table."; + EXPECT_TRUE(timerHasFixedPeriodSibling(&pads[1])) + << "Same check from the second PINIO pad's perspective."; +} + +/* + * TEST 7 — Multiple pads on the timer, none of them fixed-period (all + * MOTOR/SERVO/PINIO): must return false. Guards against a trivial "returns + * true if timer has >1 pad" mis-implementation. + */ +TEST(TimerHasFixedPeriodSibling, MultipleNonFixedPeriodSiblings_ReturnsFalse) +{ + timerHardware_t pads[3]; + pads[0].tim = 6; pads[0].usageFlags = (uint32_t)TIM_USE_PINIO; + pads[1].tim = 6; pads[1].usageFlags = (uint32_t)TIM_USE_MOTOR; + pads[2].tim = 6; pads[2].usageFlags = (uint32_t)TIM_USE_SERVO; + + g_timerHardware = pads; + g_timerHardwareCount = 3; + + EXPECT_FALSE(timerHasFixedPeriodSibling(&pads[0])); +} + +/* ========================================================================= + * TEST SUITE: SourceSync + * + * Reads the ACTUAL LIVE src/main/drivers/pinio.c off disk (not a cached + * copy, not something computed at CMake-configure time) and checks that: + * (a) timerHasFixedPeriodSibling()'s body still matches this file's inline + * reproduction, and + * (b) both pinioInitTimerPWM() call sites inside pinioInit() still gate on + * `timerHasFixedPeriodSibling(...)`. + * + * If a future change to pinio.c alters this logic (or removes the guard) + * without updating this test file, these checks fail loudly in `make check` + * / CI instead of this file silently continuing to pass against dead logic + * -- exactly the class of drift that was previously found in + * pwm_mapping_beeper_unittest.cc (see its header comment for history). + * + * This does not (and structurally cannot, given the USE_PINIO / hardware- + * driver dependencies described above) substitute for actually linking and + * executing the real function. It is a drift-detector, not a full + * integration test. Comments are stripped and whitespace is collapsed + * before comparison so that pure reformatting/typo-fixes in comments do not + * cause false failures; only the executable-statement tokens are checked. + * ========================================================================= */ + +namespace { + +// Strips "//...\n" line comments. Deliberately does not handle /* */ block +// comments or comments inside string/char literals -- the functions we +// check in pinio.c don't use either, and keeping this simple avoids this +// drift-detector itself becoming a source of drift. +std::string stripLineComments(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + for (size_t i = 0; i < text.size();) { + if (text[i] == '/' && i + 1 < text.size() && text[i + 1] == '/') { + while (i < text.size() && text[i] != '\n') { + i++; + } + continue; + } + result += text[i]; + i++; + } + return result; +} + +// Collapses any run of whitespace (including newlines) to a single space, +// and trims leading/trailing whitespace, so indentation/line-wrap changes +// don't cause false failures. +std::string normalizeWhitespace(const std::string &text) +{ + std::string result; + result.reserve(text.size()); + bool lastWasSpace = true; // trims leading whitespace + for (char c : text) { + if (std::isspace(static_cast(c))) { + if (!lastWasSpace) { + result += ' '; + lastWasSpace = true; + } + } else { + result += c; + lastWasSpace = false; + } + } + while (!result.empty() && result.back() == ' ') { + result.pop_back(); + } + return result; +} + +std::string normalizeSource(const std::string &text) +{ + return normalizeWhitespace(stripLineComments(text)); +} + +// Locates the live pinio.c relative to this test file's own path. __FILE__ +// is emitted as an absolute path by this project's CMake/Makefiles +// (verified by the equivalent helper in pwm_mapping_led_unittest.cc / +// pwm_mapping_beeper_unittest.cc), so this resolves correctly regardless of +// the build directory or CWD the test is invoked from. +std::string pinioSourcePath() +{ + std::string thisFile = __FILE__; // .../src/test/unit/pinio_timer_sibling_unittest.cc + size_t lastSlash = thisFile.find_last_of("/\\"); + std::string testUnitDir = (lastSlash == std::string::npos) ? "." : thisFile.substr(0, lastSlash); + return testUnitDir + "/../../main/drivers/pinio.c"; +} + +// Reads and normalizes pinio.c. Fails the calling test LOUDLY (not silently +// returning empty / skipping) if the file can't be found, since a missing +// file here means this drift-detector isn't detecting anything. +::testing::AssertionResult loadNormalizedPinioSource(std::string *out) +{ + const std::string path = pinioSourcePath(); + std::ifstream file(path); + if (!file.is_open()) { + return ::testing::AssertionFailure() + << "Could not open live source file at computed path: " << path + << " -- SourceSync tests cannot verify anything against dead/absent " + "source. Check that src/test/unit/ and src/main/drivers/ still " + "have their expected relative layout."; + } + std::ostringstream buffer; + buffer << file.rdbuf(); + if (buffer.str().empty()) { + return ::testing::AssertionFailure() + << "Live source file at " << path << " opened but read as empty."; + } + *out = normalizeSource(buffer.str()); + return ::testing::AssertionSuccess(); +} + +} // namespace + +TEST(SourceSync, TimerHasFixedPeriodSiblingBodyMatchesLiveSource) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPinioSource(&normalizedSource)); + + const std::string expectedFunction = normalizeSource( + "static bool timerHasFixedPeriodSibling(const timerHardware_t *timHw) " + "{ " + "for (int i = 0; i < timerHardwareCount; i++) { " + "if (timerHardware[i].tim == timHw->tim && (timerHardware[i].usageFlags & (TIM_USE_LED | TIM_USE_BEEPER))) { " + "return true; " + "} " + "} " + "return false; " + "}"); + + EXPECT_NE(normalizedSource.find(expectedFunction), std::string::npos) + << "timerHasFixedPeriodSibling() in the LIVE src/main/drivers/pinio.c " + "no longer matches this test's inline reproduction. If the " + "function legitimately changed, update the inline " + "timerHasFixedPeriodSibling() in this test file to match, then " + "update this expected string too. THIS is the exact class of " + "drift that made pwm_mapping_beeper_unittest.cc stale for months " + "-- do not let it happen again."; +} + +TEST(SourceSync, Pass1CallSiteGatesOnFixedPeriodSiblingGuard) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPinioSource(&normalizedSource)); + + // Pass 1 (pinioHardware[] loop): the condition guarding pinioInitTimerPWM(). + const std::string expectedGuard = normalizeSource( + "if (timHw && IOGetOwner(io) == OWNER_FREE && !timerHasFixedPeriodSibling(timHw) && pinioInitTimerPWM(runtimeCount, io, timHw, inverted)) {"); + + EXPECT_NE(normalizedSource.find(expectedGuard), std::string::npos) + << "pinioInit()'s Pass 1 (pinioHardware[] target-defined pins) call " + "site no longer gates pinioInitTimerPWM() on " + "!timerHasFixedPeriodSibling(timHw). If someone removes this " + "guard, a PINIO pad sharing a timer with an LED strip or BEEPER " + "would silently corrupt that output's period again -- this test " + "must fail loudly if that happens."; +} + +TEST(SourceSync, Pass2CallSiteGatesOnFixedPeriodSiblingGuard) +{ + std::string normalizedSource; + ASSERT_TRUE(loadNormalizedPinioSource(&normalizedSource)); + + // Pass 2 (timerHardware[] loop, TIM_USE_PINIO pads assigned via mixer): + // the condition guarding pinioInitTimerPWM(). + const std::string expectedGuard = normalizeSource( + "if (!timerHasFixedPeriodSibling(timHw) && pinioInitTimerPWM(runtimeCount, io, timHw, false)) {"); + + EXPECT_NE(normalizedSource.find(expectedGuard), std::string::npos) + << "pinioInit()'s Pass 2 (timerHardware[] TIM_USE_PINIO pads assigned " + "via the mixer) call site no longer gates pinioInitTimerPWM() on " + "!timerHasFixedPeriodSibling(timHw). If someone removes this " + "guard, a mixer-assigned PINIO pad sharing a timer with an LED " + "strip or BEEPER would silently corrupt that output's period " + "again -- this test must fail loudly if that happens."; +}