Fix #978, put task parameters into task record#1151
Merged
astrogeco merged 1 commit intonasa:integration-candidatefrom Feb 24, 2021
Merged
Fix #978, put task parameters into task record#1151astrogeco merged 1 commit intonasa:integration-candidatefrom
astrogeco merged 1 commit intonasa:integration-candidatefrom
Conversation
Contributor
Author
|
This needs a rebase after next baseline. Submitting as draft to get review started, actual change is in commit a0cda23. |
jphickey
added
the
CCB:Ready
astrogeco
added
CCB:2021-02-02
and removed
CCB:Ready
Contributor
|
CCB:2021-02-02 APPROVED
|
Implement a "CFE_ES_TaskStartParams_t" to complement the existing "CFE_ES_AppStartParams_t" and store this in the task record. This permits some nice cleanup: - All tasks can now use the same basic start function CFE_ES_StartAppTask() - No special/different logic is needed for main tasks/child tasks - Simplified APIs as parameters can be encapsulated in a single struct. - Fixes a race condition where child tasks may not be fully instantiated at the time the task function is invoked.
jphickey
force-pushed
the
fix-978-entrypt
branch
from
a0cda23 to
58abd38
Compare
Contributor
Author
|
Rebased to "main" - this is ready to go now. |
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Feb 24, 2021
nasa/cfe #1168 nasa/cfe #1158 nasa/cFE#1151
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Feb 26, 2021
Bumps to: - cFE v6.8.0-rc1+dev365 - osal v5.2.0-rc1+dev280 - cFS-GroundSystem v2.2.0-rc1+dev29 See: - nasa/osal#830 - nasa/cFE#1171 - nasa/cFS-GroundSystem#162 Includes: - nasa/cFE#1148 - nasa/cFE#1168 - nasa/cFE#1158 - nasa/cFE#1151 - nasa/cFE#1173 - nasa/osal#794 - nasa/osal#818 - nasa/osal#820 - nasa/osal#815 - nasa/osal#813 - nasa/osal#822 - nasa/osal#823 - nasa/cFS-GroundSystem#156
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
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.
Describe the contribution
Encapsulate all parameters for apps and tasks into a structure object and clean up internal APIs to pass this object rather than individual parameters. These parameters can then easily be stored with the relevant record in the internal table (tasks, apps, libs) and makes management easier - as code copies one object rather than many individual fields with (potentially) different names.
All parameters are stored in the record, which facilitates future telemetry generation/stats and/or for when an app might get restarted or reloaded in the future.
Notably this changes all task startup to go through the same basic routine (there is no longer a need for different accounting between main tasks and child tasks) and share the same common entry point.
Fixes #978
Testing performed
Build and sanity check CFE. In particular ensure that all tasks + child tasks are starting and running normally. Also checked that the app restart command is working as expected.
Run all unit tests and confirm coverage.
Expected behavior changes
No public API change. Internal APIs are simplified, internal data structures are more consistent, and more detail is stored with the relevant record (i.e. task record has all relevant task details, so when looking up task info one does not have to traverse to the app record which previously held some of this data).
System(s) tested on
Ubuntu 20.04
Additional context
Using the common entry point for child tasks avoids a theoretical race condition where the child task might not have been fully accounted for in the global table at the time the user function started execution. Previously this would directly invoke the user-supplied function immediately. Now using the common entry point this delays until the task record is completely accounted for (linked back to app, etc) before calling the user function. So functions such as
CFE_ES_GetAppID()and others will work right from the beginning.Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.