Internal API revisions to sleep #4606
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Old structure: New structure: |
Closed
dhalbert
previously approved these changes
May 10, 2021
<
8000
p dir="auto">@dhalbert Thanks for the review! But let's get STM32 sleep merged in first. That way I can merge the API changes to all existing Alarm modules at once, plus the STM32 port has a minor API adjustment (makes PinAlarms use const for pin objects, like other pin-related modules) that is better to put in before this.
|
|
NRF isn't returning the correct global object in testing, will resolve tomorrow morning. |
|
@dhalbert nevermind, object problems were just a testing mistake. I've tested this and it works across all 4 possible sleep modes on all platforms, NRF/STM/ESP. Should be good to go. |
dhalbert
approved these changes
May 20, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR suggests some changes to the internal structure of the Alarm module to fix some bugs with light sleep and hopefully make the sleep program flow easier to follow. Starting as draft so it isn't accidentally merged.
shared_alarm_save_wake_alarmto take an alarm parameter. This means it can be called directly in Light Sleep to set the current alarm without an intermediary static variable, and it doesn't impact deep sleep inmain.csincecommon_hal_alarm_get_wake_alarm/common_hal_alarm_create_wake_alarm(see below) can be called as the parameter. Setting the global variable to an alarm out of the current VM memory is now contained entirely within the light sleep function.common_hal_alarm_get_wake_alarmis renamedcommon_hal_alarm_create_wake_alarm. Now that light sleep "real" alarm management is entirely self contained, this function can be upfront about only returning generated "copy" alarms for deep sleep.x_woke_us_upfunctions are renamedx_woke_this_cycle, making it clear they apply only to wakeup detection within the current RAM cycle, and won't work for deep sleep._get_wake_alarmhas been completely merged intocommon_hal_alarm_create_wake_alarm, and now has no parameters, since all current VM alarm objects are handled by light sleep. The internal functions used are ofx_create_wakeup_alarmtype, see below.x_get_wakeup_alarmfunctions have been separated into two:x_create_wakeup_alarmandx_find_triggered_alarm. The creation of alarms is used for deep sleep (real and fake), whereas finding triggered alarms is for light sleep.common_hal_alarm_light_sleep_until_alarmsnow detects the wakeup cause itself, searches through the alarm list itself, and saves the real alarm object directly to the global array using theshared_alarm_save_wake_alarm, rather than saving an incorrect fake alarm to the global array (old ESP case) or using a static variable to hack around the existing system (old STM case). The alarm in the global array now matches the alarm returned by the function._idle_until_alarmhas been factored into light sleep as it was not used anywhere else.Considered but unchanged:
_get_wakeup_causestill handles both light and deep sleep, as deep sleep calls tocommon_hal_alarm_create_wake_alarmmay need either fake or true deep sleep wakeup detection._setup_sleep_alarmsstill handles all kinds of sleep, since platforms like the STM32 have overlap in light/deep sleep setup, especially for the RTC.