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.

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

Add a button HAL and corresponding drivers #169

Merged
merged 26 commits into from
Mar 21, 2018

Conversation

g-oikonomou
Copy link
Member

@g-oikonomou g-oikonomou commented Nov 10, 2017

This pull proposes a platform-independent button API (and corresponding example) that has the following key features:

  • Provides a uniform way to determine button-related events (which are no longer a sensors_event).

    • On button press (button_hal_press_event)
    • On button release (button_hal_release_event)
    • On continuous button press, generated with a period of 1 second (button_hal_periodic_event). (I did consider making the period configurable, but I think it's over-design.).

    This allows us to do something on button press, something else on continuous press for 5 seconds, something yet different on release.

  • Supports an arbitrary number of buttons.

  • Handles software debounce internally, on press as well as on release.

  • The PR assumes that a platform already supports the new GPIO HAL. Platform developers/maintainers only need to define their platform's buttons, using a single macro call. Something as simple as:

    BUTTON_HAL_BUTTON(key_left, "Key Left", BOARD_IOID_KEY_LEFT, \
                      GPIO_HAL_PIN_CFG_PULL_UP, BOARD_BUTTON_HAL_INDEX_KEY_LEFT, \
                      true);
    
    BUTTON_HAL_BUTTON(key_right, "Key Right", BOARD_IOID_KEY_RIGHT, \
                      GPIO_HAL_PIN_CFG_PULL_UP, BOARD_BUTTON_HAL_INDEX_KEY_RIGHT, \
                      true);
    

This design brings a number of benefits:

  • We expose a simple single button API to examples
  • A button is no longer a sensor and therefore not bound to the requirements of the sensors API.
  • We no longer make assumptions in os/ as to how many buttons a platform will have, nor of what the symbol names will be.
  • We have a single implementation of a s/w debounce, instead of dozens of instances of it. Platform/product developers don't need to spend time understanding/implementing these things.
  • Minimizes code duplication: Most current platform button drivers are simply a different platform's driver pasted across.
  • All platform buttons support the same features (whereas previously some supported press duration events, some did not).

Platforms currently supported:

  • All CC13xx/CC26xx
  • All CC2538

@g-oikonomou g-oikonomou added the pr/work-in-progress Used for PRs that are a work-in-progress and should be commented on but not merged label Nov 10, 2017
@simonduq
Copy link
Member

We had a discussion on an earlier version of this branch didn't we? Was trying to find it so we can link to it and followup.

@g-oikonomou
Copy link
Member Author

Closing this till further notice

@g-oikonomou g-oikonomou reopened this Mar 11, 2018
@g-oikonomou g-oikonomou force-pushed the wip/button-hal branch 3 times, most recently from 0c8f88c to 0828767 Compare March 11, 2018 22:11
@g-oikonomou
Copy link
Member Author

g-oikonomou commented Mar 11, 2018

I have done a major rework of this pull and I am re-opening it.

Now uses the GPIO HAL, so basically if a device supports the GPIO HAL, it automatically also supports buttons.

I am editing the OP to reflect recent changes.

Tested with Launchpad/CC2650, zoul/firefly, zoul/remote-reva.

More testing to come. Help with testing welcome.

@g-oikonomou g-oikonomou removed pr/work-in-progress Used for PRs that are a work-in-progress and should be commented on but not merged roadmap/long-term Used to document a feature in our long-term wishlist labels Mar 11, 2018
@g-oikonomou g-oikonomou force-pushed the wip/button-hal branch 2 times, most recently from e5bd8c4 to 192a809 Compare March 11, 2018 23:33
@g-oikonomou g-oikonomou mentioned this pull request Mar 12, 2018
6 tasks
@g-oikonomou
Copy link
Member Author

After a chat with @nfi last night:

  • The interrupt handler now just polls a process. Debounce management is scheduled by the process.
  • Added a way to retrieve the count of buttons on a device.
  • Added a way to retrieve a button by index (0, 1, ... button_hal_button_count - 1).

Tested on launchpad/cc2650 and zoul/remote-reva

@nfi
Copy link
Member

nfi commented Mar 19, 2018

I have tested the ipso-object example on Zolertia Firefly with button CoAP notifications up to Leshan and I think everything looks good.

@g-oikonomou
Copy link
Member Author

Can we like merge this? :)

@joakimeriksson
Copy link
Member

Yes, let's get it in! I updated branch now - can do the merge after that.

@joakimeriksson joakimeriksson merged commit 27aa381 into contiki-ng:develop Mar 21, 2018
@g-oikonomou g-oikonomou deleted the wip/button-hal branch March 21, 2018 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants