-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
drivers/periph_gpio: let gpio_read() return bool #20936
drivers/periph_gpio: let gpio_read() return bool #20936
Conversation
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.
We just changed the API so that it returns bool anyway.
0bcbca1
to
815fe17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me.
Don't forget to remove the extra commit :) |
815fe17
to
7d1313b
Compare
Done. But since this requires recently merged changes to the rust wrappers to work with both old and new for dir in $(dirname $(find . -name Cargo.lock)); do
pushd $dir
cargo update riot-wrappers
popd
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM at a glance, and I checked the Rust changes to be really just what is needed here.
In general I softly prefer doing the Rust changes outside the breaking PR (so that not just the riot-wrappers' limited CI runs, but it gets a full murdock run), but the change is so small here that doing it in the breaking PR is just as fine.
@benpicco Should this PR go into master ? |
Thx :) |
Contribution description
Since #20935 gpio_write() uses a
bool
instead of anint
. This does the same treatment forgpio_read()
.This does indeed add an instruction to
gpio_read()
implementations. However, users caring about an instruction more are better served withgpio_ll_read()
anyway. Andgpio_read() == 1
is often seen in newcomer's code, which would now work as expected.Testing procedure
This should not cause any regressions.
I cannot find any way to use
int gpio_read()
that won't work withbool gpio_read()
anymore at the top of my head.Issues/PRs references
Same that #20935 did for
gpio_write()
, but forgpio_read()
.