Report M5IOE1 PWM frequency write results - #322
Merged
Conversation
Return the underlying I2C write status from setPwmFrequency and document that the configured frequency is shared by every PWM channel. BREAKING: the setPwmFrequency return type changes from void to bool. Ordinary calls that discard the result remain source-compatible, but code that depends on the exact member-function type must update from void (M5IOE1_Class::*)(uint16_t) to bool (M5IOE1_Class::*)(uint16_t).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the M5IOE1 PWM frequency setter API so callers can detect I2C write failures, and clarifies in the public API docs that PWM frequency is a shared device-wide setting across channels.
Changes:
- Change
M5IOE1_Class::setPwmFrequencyreturn type fromvoidtobooland return the underlyingwriteRegister(...)status. - Add API documentation noting the PWM frequency is shared across all PWM channels (changing it affects already-running channels).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/utility/M5IOE1_Class.hpp | Updates the public API signature to bool and documents the shared-frequency behavior. |
| src/utility/M5IOE1_Class.cpp | Propagates the writeRegister result by returning it from setPwmFrequency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Closed
This was referenced Aug 18, 2026
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.
Return the I2C write status from
M5IOE1_Class::setPwmFrequencyinstead ofdiscarding it, and document that the configured frequency is shared by every
PWM channel, so setting it for one channel also retunes a channel that is
already running.
This was part of #319 and was then carried inside #321 until review flagged it
as an undocumented API change there. It is a public API decision of its own
rather than part of the duty renames, so it gets its own change where the exact
compatibility story can be stated.
Breaking change
The return type changes from
voidtobool. Ordinary calls that discard theresult stay source-compatible; only code that depends on the exact
member-function type has to move from
void (M5IOE1_Class::*)(std::uint16_t)to
bool (M5IOE1_Class::*)(std::uint16_t).M5PM1_Class::setPwmFrequencyalready returnsbool, so the two classes nowagree, and a caller can find out that the write never reached the chip instead
of proceeding on a stale frequency.
Intended for the next release, bumping the minor version to 0.3.0.
Verification
Built for ESP32, ESP32-S3, ESP32-P4 and ESP32-C5 on top of the current develop.