Skip to content

drivers/periph_common: add periph_init_buttons to init on-board buttons#17711

Merged
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:periph_init_buttons
Mar 29, 2022
Merged

drivers/periph_common: add periph_init_buttons to init on-board buttons#17711
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:periph_init_buttons

Conversation

@benpicco
Copy link
Contributor

@benpicco benpicco commented Feb 26, 2022

Contribution description

This adds a periph_init_buttons module analogous to the periph_init_leds.
However, unlike the LED module, this is not a DEFAULT_MODULE as most applications that make use of the button will take care of initializing it themselves (e.g. as an interrupt).

There is also no consistency with only a few boards auto-initializing the button.
Now with the new module, applications that rely on button auto-init can request it reliably.

Testing procedure

There is no in-tree code that makes use of this feature.
But in it's simplest form, this can be tested with

#include "board.h"
#include "periph/gpio.h"

int main(void)
{
    while (1) {
        gpio_write(LED0_PIN, gpio_read(BTN0_PIN));
    }

    return 0;
}

with USEMODULE += periph_init_buttons

Issues/PRs references

similar to #17584

@github-actions github-actions bot added Area: boards Area: Board ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration labels Feb 26, 2022
@benpicco benpicco requested a review from fjmolinas February 26, 2022 22:00
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 27, 2022
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

LGTM, how about adding it by default to those BOARDS that were initializing buttons? That way there is no change for any user relying on that? (it also adds a compile test for the module)

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Mar 25, 2022
@benpicco
Copy link
Contributor Author

how about adding it by default to those BOARDS that were initializing buttons?

I don't know, most of those will be just copy & paste without any deeper meaning.
The goal of this PR was to get rid of that very chaos and establish a unified behavior across all boards.

If this really breaks something, I hope people will notice 😉

@benpicco benpicco added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Mar 25, 2022
@benpicco benpicco force-pushed the periph_init_buttons branch from cab49bc to 8c78575 Compare March 25, 2022 19:39
* @}
*/

#include "board.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add this test? there is already a tests/leds and tests/buttons.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't this test is needed, better adapt the already present tests no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I added it to tests/leds

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you extended the README accordingly?|

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

@benpicco benpicco force-pushed the periph_init_buttons branch from 8c78575 to e8bd56a Compare March 29, 2022 06:59
@benpicco benpicco force-pushed the periph_init_buttons branch from e8bd56a to 8da2e10 Compare March 29, 2022 07:05
@github-actions github-actions bot added the Area: doc Area: Documentation label Mar 29, 2022
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK

@fjmolinas fjmolinas enabled auto-merge March 29, 2022 07:07
@fjmolinas fjmolinas merged commit 52f12e0 into RIOT-OS:master Mar 29, 2022
@benpicco benpicco deleted the periph_init_buttons branch March 29, 2022 08:29
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: boards Area: Board ports Area: doc Area: Documentation Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants