-
Notifications
You must be signed in to change notification settings - Fork 287
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
docs: Add Power Management documentation. #754
Conversation
431bc8a
to
b737a37
Compare
docs/power.rst
Outdated
The wake up sources are configured with the ``wake_source()`` function. | ||
If no wake up sources have been configured it will sleep indefinitely. | ||
|
||
.. py:function:: wake_source(pins=None, buttons=None, ms=None, run_every=False) |
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.
The pins
and buttons
parameters (in plural) could also take a single instance, e.g. pins=pin0
.
So would it be better to keep in it in plural, or singunal?
wake_source(pins=pin0)
vs wake_source(pin=(pin0, pin1))
The music and audio modules already use a pin
as a parameter, and in those cases they do take a single instance, so having it in plural here does differentiate it as well.
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.
This is resolved by combining pins
and buttons
into a single wake_on
parameter: #754 (comment)
b737a37
to
2d95e0a
Compare
On reflection, the alternative listed in the PR description makes sense and I think it's a better starting point. So I've updated the docs to do that, and if based on the review/discussion/testing it turns out we need a separate function for the wake up sources then we can revert it. |
@dpgeorge this is ready to do an initial implementation (although technically is lower priority than the Sound Effects), mostly to see if there are any technical reasons that might affect the functions as they are defined here. |
docs/power.rst
Outdated
will start running from the beginning. | ||
|
||
|
||
.. py:function:: deep_sleep(ms=None, pins=None, buttons=None, run_every=False) |
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.
pins
and buttons
could actually be combined into, eg, events
. Because the code can check the type of the objects passed in. Eg deep_sleep(events=(button_a, pin0))
.
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.
Yeah, having the buttons for only 2 options doesn't feel quite right. Although I'm not 100% sure about the word events
though, it doesn't feel very obvious deep_sleep(events=(pin1,pin2))
, but it's warming up on me.
Do you think using pins
and include buttons in the same argument would be confusing? deep_sleep(pins=(pin1, button_a))
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.
How about wake_on
or wake_by
? Eg deep_sleep(wake_on=(pin1, button_a))
.
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.
wake_on
sounds good 👍
Added in 0825744.
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.
OK, now implemented.
ed872da
to
d20d5a9
Compare
d20d5a9
to
0825744
Compare
As discussed elsewhere, my suggestion is that |
…e end of the page.
dd3abc3
to
42db7bc
Compare
This has been shipped in v2.1.0-beta.1 in the current form, so I'll merge the docs, and any changes to sleep will need a new PR here as well. And the last comment has been captured in this issue: |
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
* docs: Add Power Management documentation. * docs: Merge the wake_source() function into deep_sleep(). * docs: Merge power.deep_sleep() `pins` & `buttons` params into `wake_on`. * docs: Move power examples to Python files, and the tech details to the end of the page.
Proposal
The current proposal has been simplified to only have two sleep functions
power.deep_sleep()
andpower.off()
(as there is alreadymicrobit.sleep()
we had to use different wording) and a function to configure the wake up sourcespower.wake_source()
.Preview build of the docs: https://microbit-micropython--754.org.readthedocs.build/en/754/power.html
Things Still Under Consideration
lowPowerEnable()
andlowPowerIsEnabled()
as barriers to disable sleep in important portions of code. With the current implementation I don't think is required to expose these to the MicroPython users, but if we find a use case that needs it we might have to reconsider.Alternative Proposals
Wake up sources as
deep_sleep()
argumentsThe reason the wake up sources have been separated to a different function call was allow setting the sources once during startup, so that programmes with multiple
deep_sleep()
calls didn't have to replicate these values around.However, this is the kind of thing that could be resolved by the user creating a wrapper function:
One of the advantages of having the wake up sources as arguments is that it removes any ambiguity about the sources also being applicable to
power.off()
, as it only applies fordeep_sleep()
.At this point I'm leaning towards this simplification, unless somebody can think of a good reason to keep them separated.
Implementation details not shown in the docs