Fix ATmega32U4 _writeDutyCycle3PWM signature mismatch with hardware_api.h - #553
Open
94xhn wants to merge 1 commit into
Open
Fix ATmega32U4 _writeDutyCycle3PWM signature mismatch with hardware_api.h#55394xhn wants to merge 1 commit into
94xhn wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
…pi.h The ATmega32U4-specific _writeDutyCycle3PWM had a stray unused `int pinA` parameter, giving it a 5-parameter signature that mismatches the 4-parameter prototype declared in hardware_api.h (and matched by all other 10 MCU implementations, plus the weak fallback in generic_mcu.cpp and the sole call site in BLDCDriver3PWM.cpp). Because C++ overload resolution treats differing parameter lists as different functions, this made the ATmega32U4-specific override permanently unreachable/dead code - any call to _writeDutyCycle3PWM on this MCU silently resolved to the generic weak fallback instead. The unused pinA parameter is removed so the function matches the declared interface and is restored as the actual override used by BLDCDriver3PWM for ATmega32U4 boards.
Copy link
Copy Markdown
Member
|
Thanks for finding this! |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
_writeDutyCycle3PWM in src/drivers/hardware_specific/atmega/atmega32u4_mcu.cpp had an extra, unused int pinA parameter, giving it a 5-parameter signature:
This mismatches the prototype declared in src/drivers/hardware_api.h:
...and the 4-parameter signature used by every other MCU implementation (atmega2560, atmega328, esp32 x2, renesas, rp2040, samd, silabs/efr32, stm32, teensy), the weak fallback in generic_mcu.cpp, and the sole call site in BLDCDriver3PWM.cpp.
Because differing parameter lists make these distinct overloads/symbols in C++, the ATmega32U4-specific _writeDutyCycle3PWM could never actually be called by anything in the codebase — it was permanently dead code, and any call from BLDCDriver3PWM on an ATmega32U4 board silently resolved to the generic weak fallback instead of the MCU-specific override.
pinA is never referenced anywhere in the 5-line function body (just three analogWrite calls), so removing it is a pure signature fix with no behavior change to the body itself — it just makes the function reachable again as the intended per-MCU override.
Fix
Remove the unused int pinA parameter so the function matches hardware_api.h and the pattern used by every other MCU:
Note on impact
Today the ATmega32U4-specific body and the generic weak-fallback body are functionally identical (both just do three analogWrite calls with the same pin mapping), so there's no currently-observable runtime symptom from this bug on existing boards. This fix restores the correct per-MCU override/interface intent and removes unreachable/dead code — it also prevents a silent no-op trap for any future edit to the ATmega32U4-specific body (e.g. to actually use a pinA-style parameter), which today would never take effect due to this signature mismatch.
Testing
Verified by inspecting current dev branch source directly (this repo has no CI build matrix I could run locally for AVR). Confirmed via grep that:
Also built an isolated minimal repro (separate from this repo) transplanting the exact buggy 5-param body and the exact 4-param call site, compiled with g++, and confirmed via nm that the 5-param version's mangled symbol never matches what the call site needs — while the fixed 4-param version's symbol matches exactly and links/runs correctly.