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.

Sign up for GitHub

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

Jump to bottom

Fix #583, ES start table #660

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix #583, ES start table #660

wants to merge 2 commits into from

Conversation

CDKnightNASA
Copy link
Contributor

@CDKnightNASA CDKnightNASA commented Apr 30, 2020 •
edited by astrogeco
Loading

Describe the contribution
Fix #583
This is a re-do of the CFE ES "startup" table code. This includes unit tests (coverage) and requested "volatile" boot option.

Testing performed
both unit tests and running the core

Expected behavior changes
This changes ES to use a table to load libraries and load and start applications. This is a significant change to the internals, as libraries are loaded and applications are started in the ES task (spawned by the ES main task) and, of course, a table is used instead of a .scr file.

  • API Change: internal API changes
  • Behavior Change: loading from .tbl instead of .scr

System(s) tested on
Debian 9 VM

Contributor Info - All information REQUIRED for consideration of pull request
[email protected]

@CDKnightNASA CDKnightNASA self-assigned this Apr 30, 2020
@CDKnightNASA
Copy link
Contributor Author

CDKnightNASA commented May 1, 2020 •
edited
Loading

  • update doc

@CDKnightNASA
Copy link
Contributor Author

doc changes added (note that the doxygen references a deployment guide...guessing that doesn't exist? ;) )

@CDKnightNASA
Copy link
Contributor Author

Also should I remove the ability to exclude TBL from a build as part of this pull request?

@CDKnightNASA CDKnightNASA marked this pull request as ready for review May 5, 2020 15:16
@CDKnightNASA CDKnightNASA added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 5, 2020
@skliper
Copy link
Contributor

skliper commented May 5, 2020

Also should I remove the ability to exclude TBL from a build as part of this pull request?

I suggest a separate issue/pull request

@skliper
Copy link
Contributor

skliper commented May 5, 2020

doc changes added (note that the doxygen references a deployment guide...guessing that doesn't exist? ;) )

The deployment guide costs extra. Yeah, should remove reference.

@astrogeco astrogeco changed the title fix #583 - ES start table Fix #583, ES start table May 5, 2020
astrogeco
astrogeco reviewed May 6, 2020
View reviewed changes
docs/cFE Application Developers Guide.md Outdated
@@ -583,7 +583,7 @@ Reference cFE API documentation for more detail on reset types.

The cFE contains support for shared libraries. For the current version
of the cFE, the shared libraries must be loaded on cFE startup (see the
cFE Deployment Guide on how to modify the cfe_es_startup.scr in order
cFE Deployment Guide on how to configure the cfe_es_start.tbl in order
Copy link
Contributor

Choose a reason for hiding this comment

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

Open ticket for this

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be in users guide

@skliper skliper added this to the 7.0.0 milestone May 6, 2020
@astrogeco
Copy link
Contributor

CCB-20200506 Concept is good. Review again once 6.8 is finished

@astrogeco astrogeco added CCB:Ignore Pull Request can be ignored. Will be re-examined at by next CCB. CCB-20200506 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 6, 2020
@CDKnightNASA CDKnightNASA mentioned this pull request May 6, 2020
Closed
@astrogeco astrogeco marked this pull request as draft June 24, 2020 14:44
@skliper skliper linked an issue Aug 21, 2020 that may be closed by this pull request
ES should use a table for the "startup script" #583
Open
@skliper skliper removed this from the 7.0.0 milestone Aug 21, 2020
@astrogeco astrogeco closed this Oct 13, 2020
@astrogeco astrogeco reopened this Oct 13, 2020
@astrogeco astrogeco changed the base branch from master to main October 13, 2020 21:30
@astrogeco astrogeco mentioned this pull request Feb 3, 2021
Merged
@skliper skliper added this to the cFS-Draco milestone Aug 6, 2021
@skliper
Copy link
Contributor

skliper commented Aug 6, 2021

Consider as alternate implementation inclusion in Draco (split into module and allow selection). Trade additional test/maintenance complexity for supporting both options. Likely an easier full switch/replacement if there was support for a simple table editor (EDS based?), in general all tables would benefit from this...

@astrogeco astrogeco marked this pull request as ready for review October 20, 2021 13:55
@astrogeco astrogeco marked this pull request as draft October 20, 2021 13:56
@skliper skliper removed this from the Draco milestone Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astrogeco astrogeco astrogeco left review comments

@jwilmot jwilmot Awaiting requested review from jwilmot

@jphickey jphickey Awaiting requested review from jphickey

@skliper skliper Awaiting requested review from skliper

Assignees

@CDKnightNASA CDKnightNASA

Labels
CCB:Ignore Pull Request can be ignored. Will be re-examined at by next CCB. cFE-ES conflicts enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ES should use a table for the "startup script"
3 participants
@CDKnightNASA @skliper @astrogeco