SystemTask state refactor #2109
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
|
Build size and comparison to main:
|
State transitions now happen immediately where possible This simplifies state management in general, and prevents bugs such as the chime issue from occurring in the first place
c2a39b3 to
4b6b9d3
Compare
Member
Author
|
Inverted state check to make adding new states smoother |
Merged
JF002
reviewed
Sep 14, 2024
Collaborator
JF002
left a comment
There was a problem hiding this comment.
A documentation of the states of SystemTask (a state diagram for example) would probably be helpful to understand this PR and for future us. This might be not blocking for this PR, but this would be a really nice addition for developers and contributors ;-)
Member
Author
|
I'm pretty terrible at graphics but I'll give it a go! |
JF002
approved these changes
Sep 21, 2024
NeroBurner
added a commit
to InfiniTimeOrg/InfiniSim
that referenced
this pull request
Sep 21, 2024
PR InfiniTimeOrg/InfiniTime#2109 refactors the `SystemTask.h` state machine to prevent `GoingToSleep` race conditions (and similar state transistions). In doing so the function `uxQueueMessagesWaiting()` from NRF SDK was used. Implement this newly used function.
NeroBurner
added a commit
to InfiniTimeOrg/InfiniSim
that referenced
this pull request
Sep 21, 2024
PR InfiniTimeOrg/InfiniTime#2109 refactors the `SystemTask.h` state machine to prevent `GoingToSleep` race conditions (and similar state transistions). In doing so the function `uxQueueMessagesWaiting()` from NRF SDK was used. Implement this newly used function.
Contributor
|
Updating InfiniSim to support the newly used function |
NeroBurner
approved these changes
Sep 21, 2024
Contributor
NeroBurner
left a comment
There was a problem hiding this comment.
Everything looking sound
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Currently SystemTask is somewhat fragile to duplicate state transitions and unusual state transition message orders. Most notably this can cause it to get stuck in
GoingToSleepin some scenarios.This PR introduces GoToSleep() and changes GoToRunning() as immediately executable functions. The guarantee that state transitions happen immediately reduces complexity when it comes to ensuring the device is awake for events such as Chimes
Fixes part of #1790 #2012 (there may be other issues contributing to these, but this is probably part of it)