Give the PWM duty APIs explicit units and leave setPwmDuty vacant

[ainyan03]

Supersedes #319, which tried to solve this by renaming the raw entry point and
giving setPwmDuty the percent meaning the standalone drivers use. The review
there showed why that does not work, so this takes a different route.

Problem

M5IOE1_Class::setPwmDuty takes a raw 12-bit value, while the standalone
M5IOE1 and M5PM1 drivers use that name for a percentage and order the two
booleans the other way round. Reading their documentation and writing
setPwmDuty(ch, 50, false, true) against this class compiles and quietly means
"duty 50 out of 4095, disabled" instead of "50 percent, enabled".

Giving setPwmDuty the percent meaning here would not fix that, it would move
it: existing raw calls would keep compiling and change meaning. A range check
cannot catch it either, because a raw duty that happens to land in 0-100 is a
valid percentage.

Fix

The name is the problem, so it is not reused. Both classes now offer

• setPwmDutyPercent for 0-100
• setPwmDuty12bit for 0-4095

and setPwmDuty is gone from both, so a call written against either old meaning
fails to compile rather than changing behaviour.

Two smaller changes come with it:

The duty parameters are uint32_t. The point of a documented 0-100 range is
that values outside it are rejected, and with a narrower parameter the check only
sees whatever survived the conversion at the call boundary. Existing in-range
calls are unaffected.

Polarity is m5::pwm_polarity_t rather than a bool, so the two adjacent
booleans can no longer be transposed. Enable stays a bool; with only one left
there is nothing to transpose it with. The type lives in a small shared header
because both classes use it, matching how the other cross-class enums in this
library are placed, while the channel enums stay per class because the numbering
follows each chip.

M5PM1_Class is included even though its percent API only landed recently and
matches the standalone drivers, because leaving it behind would mean one class
saying setPwmDutyPercent and the other saying setPwmDuty for the same thing.

The register layouts are untouched: the enable and polarity bit positions stay
the ones each chip uses.

Breaking changes

Each item below is a separate source-level break.

• The unqualified setPwmDuty is gone from both classes. Duty goes through a
  name that states its unit instead: setPwmDutyPercent for 0-100 or
  setPwmDuty12bit for 0-4095.
• The M5IOE1_Class duty methods now require pwm_channel_t instead of
  std::uint8_t. The enumerators pwm_ch1 through pwm_ch4 retain the
  existing underlying values 0 through 3, so correctly migrated calls select the
  same channels. Callers that passed numeric channels must replace them with the
  corresponding enumerator; no std::uint8_t compatibility overload is provided
  because it would also accept unrelated unscoped enums, and numeric literals
  would prefer it, which together would defeat the channel type checking this
  change is for.
• Polarity is m5::pwm_polarity_t instead of bool, and on M5IOE1_Class the
  last two arguments change order from (enable, polarity) to
  (polarity, enable), matching M5PM1_Class and the standalone drivers. A
  bool written in either position no longer compiles, so the reordering cannot
  pass silently.
• The duty parameters widen to std::uint32_t. In-range calls are
  unaffected; code that names the exact member-function type has to follow. This
  applies to M5PM1_Class::setPwmDuty12bit too, which keeps its name.
• The duty methods return bool. An out-of-range duty and an I2C failure are
  now reportable instead of being masked or discarded.

Old	New
M5IOE1_Class::setPwmDuty(0, raw, enable, polarity)	setPwmDuty12bit(pwm_ch1, raw, polarity, enable)
M5PM1_Class::setPwmDuty(ch, percent, polarity, enable)	setPwmDutyPercent(ch, percent, polarity, enable)
a bool polarity argument	m5::pwm_polarity_t::normal / ::inverted

Only M5IOE1_Class::setPwmDuty is a released signature; it has been present
since 0.2.19. The M5PM1_Class PWM methods were added after 0.2.20 and have
never been in a release, so the changes to those affect development builds only.

The setPwmDuty name is left vacant deliberately. Nothing here defines or
promises a future use for it, but keeping it out of the way means it could be
reintroduced later without repeating the situation this change is fixing.

Intended for the next release, bumping the minor version to 0.3.0.

Verification

Built for ESP32-S3, ESP32-P4, ESP32-C5 and ESP32, and the first commit builds on
its own. Old calls were confirmed to fail with "no member named setPwmDuty" on
both classes, and the guards were checked at 0, 100 and 101 percent and at 0,
4095 and 4096 raw. The five internal call sites all pass raw values and keep
their previous output.

[Copilot]

Pull request overview

This PR refactors the PWM duty APIs on M5IOE1_Class and M5PM1_Class to make duty units explicit (percent vs raw 12-bit) and to prevent accidental boolean-parameter transposition by introducing a shared m5::pwm_polarity_t type, while intentionally leaving setPwmDuty undefined so old call sites fail to compile.

Changes:

• Replaces ambiguous setPwmDuty with setPwmDutyPercent(0–100) and setPwmDuty12bit(0–4095) on both classes, and switches polarity from bool to m5::pwm_polarity_t.
• Tightens validation by rejecting out-of-range duty inputs rather than masking/truncating.
• Updates internal call sites to the new APIs and adds a shared PWM polarity type header.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/utility/pwm_types.hpp	Adds shared m5::pwm_polarity_t enum for explicit PWM polarity.
src/utility/power/M5PM1_Class.hpp	Renames/reshapes PM1 PWM duty APIs to explicit units and polarity type.
src/utility/power/M5PM1_Class.cpp	Implements setPwmDutyPercent and updates polarity handling.
src/utility/M5IOE1_Class.hpp	Introduces explicit-unit PWM duty APIs and polarity type; changes PWM method signatures.
src/utility/M5IOE1_Class.cpp	Implements percent/12-bit duty setters with range checks and explicit polarity.
src/utility/Power_Class.cpp	Updates internal PWM call sites to setPwmDuty12bit + explicit polarity.
src/utility/led/LED_PaperMono_Class.cpp	Updates IOE1 PWM call to the new 12-bit API with explicit polarity.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ainyan03]

Fair point, and it is now split out. The setPwmFrequency change is gone from
this branch, so this pull request is only about the duty renames again and
setPwmFrequency keeps its void signature here.

It was intentional rather than accidental -- the same method on M5PM1_Class
returns bool, and leaving this one void keeps the two disagreeing -- but you
are right that it is a separate public API decision and that the description did
not cover it. Deciding it on its own is better than carrying it along, so it goes
in its own change where the exact compatibility story can be stated: ordinary
calls that discard the result stay source-compatible, while code that depends on
the exact member-function type has to move from
void (M5IOE1_Class::*)(uint16_t) to bool (M5IOE1_Class::*)(uint16_t).

[ainyan03]

Fair point, and it is now split out. The setPwmFrequency change is gone from
this branch, so this pull request is only about the duty renames again and
setPwmFrequency keeps its void signature here.

It was intentional rather than accidental -- the same method on M5PM1_Class
returns bool, and leaving this one void keeps the two disagreeing -- but you
are right that it is a separate public API decision and that the description did
not cover it. Deciding it on its own is better than carrying it along, so it goes
in its own change where the exact compatibility story can be stated: ordinary
calls that discard the result stay source-compatible, while code that depends on
the exact member-function type has to move from
void (M5IOE1_Class::*)(uint16_t) to bool (M5IOE1_Class::*)(uint16_t).

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/utility/M5IOE1_Class.hpp:71

• PR description says the breaking change is that setPwmDuty no longer exists, but for M5IOE1_Class the channel parameter also changes from std::uint8_t to the pwm_channel_t enum. This is an additional source-breaking change for callers that previously passed a numeric channel (even after renaming to setPwmDuty12bit/setPwmDutyPercent). Either document this explicitly in the PR description / release notes, or consider adding a compatibility overload that accepts std::uint8_t and validates 0-3 (or 1-4 if that’s the intended external numbering).

    /// set PWM duty in percent.
    /// @param channel PWM channel (pwm_ch1 - pwm_ch4).
    /// @param duty duty cycle in percent (0-100).
    /// @param polarity PWM output polarity.
    /// @param enable true=enable / false=disable.
    bool setPwmDutyPercent(pwm_channel_t channel, std::uint32_t duty,
                           pwm_polarity_t polarity = pwm_polarity_t::normal, bool enable = true);

[ainyan03]

The suppressed comment on the channel parameter is correct, and the description
did not cover it, so the description is now updated.

The M5IOE1_Class duty methods do change the channel parameter from
std::uint8_t to pwm_channel_t, and that is a break on its own: the released
setPwmDuty took a number, and no implicit conversion carries a number into an
unscoped enum. It is spelled out in the breaking list now, together with the fact
that pwm_ch1 through pwm_ch4 keep the underlying values 0 through 3, so a
call migrated to the enumerators selects the same channel it selected before.

The std::uint8_t compatibility overload is deliberately not added. Because
pwm_channel_t is an unscoped enum, an overload taking std::uint8_t would also
accept the channel enum of the other class, so passing
M5PM1_Class::pwm_ch1 into an M5IOE1_Class call would compile again and select
the wrong channel. Numeric literals would also prefer that overload, so the enum
parameter would stop being enforced in practice. Those are exactly the mistakes
the type is there to catch, and this change accepts the break rather than keeping
them reachable.

While rewriting the section, the remaining breaking items are enumerated instead
of left implicit in the rename: the channel type tightening, the polarity type
and the argument order on M5IOE1_Class, the duty widening to std::uint32_t
(which applies to M5PM1_Class::setPwmDuty12bit as well, since that name does
not change), and the bool return. The description also notes which of these
touch a released signature: only M5IOE1_Class::setPwmDuty, present since
0.2.19. The M5PM1_Class PWM methods landed after 0.2.20 and have never been
released.

The commits are unchanged; the description is the only thing that moved.