Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

drivers/periph/gpio: make gpio_write() take a bool #20935

Merged
merged 1 commit into from
Oct 22, 2024
Merged

drivers/periph/gpio: make gpio_write() take a bool #20935

merged 1 commit into from
Oct 22, 2024

Conversation

benpicco
Copy link
Contributor

Contribution description

There is no reason for gpio_write() to take an int when a GPIO can only be either on or off.
To avoid confusion, switch to bool

Testing procedure

Since a bool is usually just an int in disguise, there should be no code or semantic changes.

Issues/PRs references

@github-actions github-actions bot added Platform: native Platform: This PR/issue effects the native platform Platform: MSP Platform: This PR/issue effects MSP-based platforms Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Area: drivers Area: Device drivers Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Area: cpu Area: CPU/MCU ports labels Oct 22, 2024
@benpicco benpicco changed the title drivers/periph/gpio: make gpio_write() take a bool drivers/periph/gpio: make gpio_write() take a bool Oct 22, 2024
@benpicco benpicco added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 22, 2024
@riot-ci
Copy link

riot-ci commented Oct 22, 2024 •
edited
Loading

Murdock results

✔️ PASSED

4627f66 drivers/periph/gpio: make gpio_write() take a bool

Success Failures Total Runtime
10214 0 10215 13m:54s

Artifacts

maribu
maribu approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

lgtm

Normally I would say this is not fit for a last minute merge before branching of the release branch, but your release manager authority should allow you to do just that :-)

@benpicco benpicco added this pull request to the merge queue Oct 22, 2024
Merged via the queue into RIOT-OS:master with commit faa1003 Oct 22, 2024
26 checks passed
maribu added a commit to maribu/RIOT that referenced this pull request Oct 22, 2024
@maribu
drivers/periph_gpio: let gpio_read() return bool
053864e 
Since RIOT-OS#20935 gpio_write()
uses a `bool` instead of an `int`. This does the same treatment for
`gpio_read()`.

This does indeed add an instruction to `gpio_read()` implementations.
However, users caring about an instruction more are better served with
`gpio_ll_read()` anyway. And `gpio_read() == 1` is often seen in
newcomer's code, which would now work as expected.
@maribu maribu mentioned this pull request Oct 22, 2024
Merged
@benpicco benpicco deleted the gpio_write-bool branch October 23, 2024 10:02
maribu added a commit to maribu/RIOT that referenced this pull request Oct 23, 2024
@maribu
drivers/periph_gpio: let gpio_read() return bool
c2c2cc8 
Since RIOT-OS#20935 gpio_write()
uses a `bool` instead of an `int`. This does the same treatment for
`gpio_read()`.

This does indeed add an instruction to `gpio_read()` implementations.
However, users caring about an instruction more are better served with
`gpio_ll_read()` anyway. And `gpio_read() == 1` is often seen in
newcomer's code, which would now work as expected.
@benpicco benpicco added this to the Release 2024.10 milestone Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@vincent-d vincent-d Awaiting requested review from vincent-d vincent-d is a code owner

@dylad dylad Awaiting requested review from dylad dylad is a code owner

@keestux keestux Awaiting requested review from keestux keestux is a code owner

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@gschorcht gschorcht Awaiting requested review from gschorcht gschorcht is a code owner

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@basilfx basilfx Awaiting requested review from basilfx basilfx is a code owner

@nandojve nandojve Awaiting requested review from nandojve nandojve is a code owner

@kYc0o kYc0o Awaiting requested review from kYc0o kYc0o is a code owner

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: MSP Platform: This PR/issue effects MSP-based platforms Platform: native Platform: This PR/issue effects the native platform Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benpicco @riot-ci @maribu