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

Fix #28, provide library API #960

Merged
merged 3 commits into from
Oct 28, 2020

Conversation

jphickey
Copy link
Contributor

Describe the contribution

Fixes #28
Provide Library API similar to App API

Allows the existing CFE_ES_AppInfo_t structure to be extended to libraries as well as applications by introducing a new value (3)
for the Type field.

Allows Libraries to be queried via API calls similar to App API.

Also extends the Query All/Query One commands to operate on Libraries or Applications.

Fixes #173
Break up the monolithic AppCreate and LoadLibrary functions and have these call subroutines that operate on the common components.

Fixes #950
Fix race conditions in app request processing state machine.

Testing performed
Build and sanity check operation of CFE
Confirm able to restart/reload sample_app via command as expected
Run all unit tests

Expected behavior changes
Adds Library API.
Query commands that previously worked only on Apps can now be used on Apps or Libs

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Oct 21, 2020
@astrogeco
Copy link
Contributor

astrogeco commented Oct 21, 2020

CCB 2020-10-21 APPROVED pending @acudmore review

@skliper to check if we need requirements for this functionality.

Loading and unloading libraries was discussed previously but we decided against it due to dependencies. A new library load would require either a restarting the system or unloading and reloading all the apps that use said library. The latter option would probably result in greater "downtime".

@skliper skliper requested a review from a user October 21, 2020 16:33
@skliper
Copy link
Contributor

skliper commented Oct 21, 2020

@ejtimmon This will allow CS to checksum libraries

@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Oct 21, 2020
Because the process of handling a control request involves
calling other subsystems, the ES lock needs to be released.
However, this also means that the app record can change state
for other reasons, such as the app self-exiting at the same
time.

To avoid this possibility, process in two phases:

First assemble a list of tasks that have timed out
and need to be cleaned up, while ES is locked.

Next actually perform the cleanup, while ES is unlocked.
In areas during cleanup that need to update the ES global,
the lock is locally re-acquired and released.
Reorganize the global data structures for apps and
libraries into components that can be shared between
the two concepts.

Break up the monolithic AppCreate and LoadLibrary
functions and have these call subroutines that
operate on the common components.
Allows the existing "CFE_ES_AppInfo_t" structure to be extended to
libraries as well as applications by introducing a new value (3)
for the Type field.

Allows Libraries to be queried via API calls similar to App API.

Also extends the Query All/Query One commands to operate on
Libraries or Applications.
@jphickey
Copy link
Contributor Author

Update - no changes, just rebased to current main branch.


/*
**-------------------------------------------------------------------------------------
** Name: CFE_ES_StartMainTask
Copy link

Choose a reason for hiding this comment

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

Name: CFE_ES_StartAppTask

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I like the clean up and continued modularization, and I think the ability to treat loadable libraries like apps is important.

I think the app startup should be fine as is, but I would prefer if the app handled this in CFE_ES_RegisterApp if possible. Not sure if the context exists to do this from CFE_ES_RegisterApp though.

@astrogeco astrogeco merged commit 35f3b03 into nasa:integration-candidate Oct 28, 2020
astrogeco added a commit to nasa/cFS that referenced this pull request Oct 28, 2020
@jphickey jphickey deleted the fix-28-app-lib-api branch December 3, 2020 17:53
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants