Harden PWM initialization failure handling - #326
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens board initialization paths to respect PWM / pull configuration failure reporting (especially to avoid retained PM1 / M5IOE1 hardware states persisting across ESP resets), and improves PM1 PWM API documentation so callers correctly route PWM to the intended pins.
Changes:
- ToughC5: retries the GPIO-mode fallback when PM1 PWM ch1 shutdown fails, and logs whether GPIO4 was actually switched to GPIO mode (or if silence cannot be guaranteed).
- ToughC5: logs a failure to release the CHG_PROG pull state on M5IOE1.
- StopWatch: turns off the motor PWM duty first and only enables the motor output pin after that write succeeds.
- PM1 PWM API: documents channel-to-pin mapping and the need to separately mux GPIO function to
special.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/utility/power/M5PM1_Class.hpp | Adds PWM channel mapping + mux requirement notes to prevent “successful calls but no waveform” confusion. |
| src/utility/Power_Class.cpp | Ensures init-time PWM-off / pull writes are checked and failure handling avoids enabling outputs when safety state can’t be confirmed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Problem
#318, #320 and #322 gave the PM1 / M5IOE1 PWM and pull APIs failure
reporting, but the board initialization paths still discard the results in a
few places where the whole point of the write is to rule out a retained
hardware state:
that reverts PM1 GPIO4 to plain GPIO mode is attempted once with its result
ignored, while the error log claims the pin was left as a low output. If the
fallback write also failed, a retained PWM state keeps the buzzer sounding
despite the log saying otherwise.
PWM duty-off write, and neither result is checked. If the duty-off write
fails, a duty retained from the previous firmware drives the motor as soon
as the pin is enabled.
setPullMode(..., pull_none)) ignores thereported failure, although the comment above it states the line must stay
released.
The PM1 PWM API also leaves its channel-to-pin mapping and the separate GPIO
function mux requirement undocumented, so a caller can get
truefrom everyPWM call and still see no waveform on the pin.
Fix
log now distinguishes "switched to GPIO mode" from "could not be switched;
silence cannot be guaranteed".
output once it succeeds. On failure the motor output is left disabled and
an error is logged.
channel 1 to GPIO4, and that
setGPIOFunction()withspecialis neededseparately to route the PWM to the pin.
The remaining
IOExpander_Basevirtuals still returnvoid; making themconsistent is left for a separate change, as noted in #320.