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

System.sleep(): support for multiple wake up pins #1405

Merged
merged 4 commits into from Jan 22, 2018

Conversation

@avtolstoy
Copy link
Member

commented Oct 12, 2017

submission notes
**Important:** Please sanitize/remove any confidential info like usernames, passwords, org names, product names/ids, access tokens, client ids/secrets, or anything else you don't wish to share.

Please Read and Sign the Contributor License Agreement ([Info here](https://github.com/spark/firmware/blob/develop/CONTRIBUTING.md)).

You may also delete this submission notes header if you'd like. Thank you for contributing!

Problem

Original issue #1349

Solution

Implements the support in HAL, adds four additional System.sleep() variants:

  • System.sleep(std::initializer_list<pin_t>, InterruptMode, ...);
  • System.sleep(std::initializer_list<pin_t>, std::initializer_list<InterruptMode>, ...);
  • System.sleep(const pin_t*, size_t, InterruptMode, ...);
  • System.sleep(const pin_t*, size_t, const InterruptMode*, size_t, ...);

Steps to Test

  • wiring/api
  • app/sleep_multiple_pins

Example App

void setup() {
}

void loop() {
  System.sleep({D0, D1}, RISING);
  System.sleep({D0, D1}, {RISING, FALLING});

  const pin_t pins[] = {D0, D1};
  const InterruptMode modes[] = {RISING, FALLING};
  System.sleep(pins, 2, RISING);
  System.sleep(pins, 2, modes, 2);
}

References


Completeness

TODO: Add documentation

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy requested a review from m-mcgowan Oct 12, 2017

@pkourany

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

@avtolstoy, if I understand correctly, std::initializer_list<pin_t> uses dynamic allocation. Using this a lot could lead to heap fragmentation. If that is the case, perhaps the other forms of the function call are better.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Oct 17, 2017

EDIT
You are right, of course it will use a temporary copy in certain cases:

An object of type std::initializer_list is constructed from an initializer list as if the implementation allocated a temporary array of N elements of type const E, where N is the number of elements in the initializer list. Each element of that array is copy-initialized with the corresponding element of the initializer list, and the std::initializer_list object is constructed to refer to that array.

It won't be constructed on heap though, and will use stack.
/EDIT

@pkourany It doesn't use dynamic allocations (on heap). std::initializer_list<T> is just a tiny convenient wrapper over a compiler-generated const array of elements or over a temporarily stack-allocated const array of copies.

Having an std::initializer_list<pin_t> variant allows easy in-place passing of the pin identifiers, which sounds really convenient:

System.sleep({D0, D1, D2, D3, D4}, RISING);
System.sleep({D0, D1, D2, D3, D4}, {RISING, FALLING, RISING, FALLING, RISING});

Of course the PR includes sleep(pin_t*, size_t, InterruptMode) and sleep(pin_t*, size_t, InterruptMode*, size_t) variants for some advanced use cases.

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2017

I really like this, bravo!

@avtolstoy avtolstoy referenced this pull request Oct 23, 2017
3 of 6 tasks complete
@LouOtway

This comment has been minimized.

Copy link

commented Oct 25, 2017

Is there a way for application to know which pin caused the interrupt?

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2017

@LouOtway Yeap, please take a look at #1410

@m-mcgowan m-mcgowan force-pushed the feature/system-sleep-multiple-pins branch from c53601e to 02618ea Jan 22, 2018

@m-mcgowan m-mcgowan merged commit e2bbf92 into develop Jan 22, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
avtolstoy added a commit to particle-iot/docs that referenced this pull request Feb 23, 2018
System.sleep(): support for multiple wake up pins (particle-iot/devic…
…e-os#1405)

Documents new `System.sleep()` overloads:
- `System.sleep(std::initializer_list<pin_t>, InterruptMode, ...);`
- `System.sleep(std::initializer_list<pin_t>, std::initializer_list<InterruptMode>, ...);`
- `System.sleep(const pin_t*, size_t, InterruptMode, ...);`
- `System.sleep(const pin_t*, size_t, const InterruptMode*, size_t, ...);`
technobly added a commit to particle-iot/docs that referenced this pull request Apr 9, 2018
Merge pull request #740 from particle-iot/feature/system-sleep-multip…
…le-pins

 System.sleep(): support for multiple wake up pins (particle-iot/device-os#1405)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.