Skip to content

Return the write status from the IO expander virtuals - #328

Merged
lovyan03 merged 2 commits into
m5stack:developfrom
ainyan03:ioexp_bool_status
Aug 18, 2026
Merged

Return the write status from the IO expander virtuals#328
lovyan03 merged 2 commits into
m5stack:developfrom
ainyan03:ioexp_bool_status

Conversation

@ainyan03

@ainyan03 ainyan03 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

#320 and #322 gave setPullMode and the PWM methods failure reporting, but
the remaining IOExpander_Base virtuals — setDirection, setHighImpedance,
digitalWrite, resetIrq, disableIrq, enableIrq — still return void,
so an I2C failure or an invalid pin is silently indistinguishable from
success. #320 explicitly left making them consistent for a separate change;
this is that change.

Fix

All six now return bool with the same contract as setPullMode: true
only when the requested state was completely established, false for an
invalid pin or an I2C failure. The implementations simply propagate the
status of the underlying register access that they already performed; no
register traffic changes. On the PI4IOE5V6408 the read-to-clear resetIrq
moves from readRegister8 (which cannot report failure) to readRegister,
performing the same one-byte read.

Existing calls stay source-compatible since the result can be ignored; the
callers that must act on failures already do so since #326.

Breaking change

Only classes deriving from IOExpander_Base are affected: overrides written
against the old void signatures no longer compile and must change their
return type to bool. Ordinary callers are unaffected.

Post-review amendment

A local adversarial review (three independent passes) found that the
PI4IOE5V6408 setters accepted out-of-range pins — 1 << pin collapses to a
zero mask for pins 8-31, so the helper rewrote the register unchanged and
returned true, contradicting the documented contract (and shifting by 32+ is
undefined behavior). They now reject pin >= 8 like setPullMode already
did. The @return docs were also reworded to promise what the
implementations actually do — every required register access acknowledged —
rather than "state completely established": the written value is not read
back, and read-modify-write accesses are not atomic against concurrent
access to the same register. M5IOE1_Class::setHighImpedance now documents
that it selects open-drain drive mode rather than disconnecting the pin.

@lovyan03
lovyan03 requested a lite review from Copilot August 18, 2026 04:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lovyan03
lovyan03 merged commit 71ab191 into m5stack:develop Aug 18, 2026
27 checks passed
@ainyan03
ainyan03 deleted the ioexp_bool_status branch August 18, 2026 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants