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

SystemTask state refactor #2109

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

mark9064
Copy link
Contributor

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

Copy link

Build size and comparison to main:

Section Size Difference
text 378868B -32B
data 948B 0B
bss 63488B 0B

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
@mark9064
Copy link
Contributor Author

Inverted state check to make adding new states smoother

@mark9064 mark9064 mentioned this pull request Aug 25, 2024
Copy link
Collaborator

@JF002 JF002 left a comment

Choose a reason for hiding this comment

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

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

src/components/ble/DfuService.cpp Show resolved Hide resolved
@mark9064
Copy link
Contributor Author

I'm pretty terrible at graphics but I'll give it a go!

@JF002 JF002 added this to the 1.15.0 milestone 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.
@NeroBurner
Copy link
Contributor

Updating InfiniSim to support the newly used function uxQueueMessagesWaiting InfiniTimeOrg/InfiniSim#151

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

Everything looking sound

@NeroBurner NeroBurner merged commit c3d0590 into InfiniTimeOrg:main Sep 21, 2024
6 of 7 checks passed
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.

3 participants