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

docs: Add Power Management documentation. #754

Merged
merged 4 commits into from
Sep 5, 2022

Conversation

microbit-carlos
Copy link
Collaborator

@microbit-carlos microbit-carlos commented Jun 20, 2022

Proposal

The current proposal has been simplified to only have two sleep functions power.deep_sleep() and power.off() (as there is already microbit.sleep() we had to use different wording) and a function to configure the wake up sources power.wake_source().

Preview build of the docs: https://microbit-micropython--754.org.readthedocs.build/en/754/power.html

Things Still Under Consideration

  • The MakeCode implementation, which is a relatively thin wrapper around CODAL, had to offer the option to "request sleep" which goes to sleep when the current fiber yields, or to "request sleep and block the fiber until it goes to sleep". For MicroPython I think we could have only the second option, as the usage of fibers is limited
  • CODAL also offers lowPowerEnable() and lowPowerIsEnabled() 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() arguments

The 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.

from microbit import *
import radio

power.wake_source(pins=(pin0, pin1), buttons=button_a)

@run_every(s=1)
def check_pin_every_second():
    if pin2.read_digital():
        power.deep_sleep()

radio.on()
while True:
    if button_b.is_pressed():
        power.deep_sleep()
    if radio.receive() == 'sleep':
        power.deep_sleep()

However, this is the kind of thing that could be resolved by the user creating a wrapper function:

from microbit import *
import radio

def go_to_sleep():
    power.deep_sleep(pins=(pin0, pin1), buttons=button_a)

@run_every(s=1)
def check_pin_every_second():
    if pin2.read_digital():
        go_to_sleep()

radio.on()
while True:
    if button_b.is_pressed():
        go_to_sleep()
    if radio.receive() == 'sleep':
        go_to_sleep()

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 for deep_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

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)
Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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)

@microbit-carlos
Copy link
Collaborator Author

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.

@microbit-carlos
Copy link
Collaborator Author

@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.
The MakeCode implementation turned out a bit more complex than we anticipated, mostly to be able to deal with how all fibers go to sleep, but I think that should not affect MicroPython.

docs/power.rst Outdated Show resolved Hide resolved
docs/power.rst Outdated
will start running from the beginning.


.. py:function:: deep_sleep(ms=None, pins=None, buttons=None, run_every=False)
Copy link
Member

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)).

Copy link
Collaborator Author

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))

Copy link
Member

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)).

Copy link
Collaborator Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

OK, now implemented.

@dpgeorge
Copy link
Member

As discussed elsewhere, my suggestion is that run_every=True does not end the deep_sleep() call, but rather wakes briefly to execute the run every and then resumes the deep sleep. The deep sleep only ends when the given timeout expires, or a wake_on event occurs. In particular, power.deep_sleep(run_every=True) will never return. (And maybe we can then make run_every=True the default.)

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Sep 5, 2022

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:

@microbit-carlos microbit-carlos merged commit ffcd3a6 into bbcmicrobit:v2-docs Sep 5, 2022
@microbit-carlos microbit-carlos deleted the docs-pwr branch September 5, 2022 18:30
microbit-carlos added a commit to microbit-carlos/micropython that referenced this pull request Sep 6, 2022
* 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.
microbit-carlos added a commit that referenced this pull request Nov 15, 2022
* 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.
microbit-carlos added a commit to microbit-carlos/micropython that referenced this pull request Feb 26, 2024
* 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.
microbit-carlos added a commit that referenced this pull request Feb 26, 2024
* 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.
microbit-carlos added a commit that referenced this pull request May 7, 2024
* 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.
microbit-carlos added a commit that referenced this pull request Jul 18, 2024
* 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.
microbit-carlos added a commit that referenced this pull request Sep 16, 2024
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants