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 #147, Add check for success of CFE_TBL_Load() during Initialization #190

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Oct 24, 2022

Checklist

Describe the contribution

Testing performed
Tested using same steps as @jphickey used in raising the issue (Start cFS without the sample app table file present in the /cf directory). Confirmed action of new code as per screenshot below:
Screenshot 2022-10-24 13 15 05

Expected behavior changes
Sample App will exit during initialization if return value of CFE_TBL_Load() is not CFE_SUCCESS.

System(s) tested on
Intel(R) Celeron(R) N4100 CPU @ 1.10GHz x86_64
Debian GNU/Linux 11 (bullseye)
cFE v7.0.0-rc4+dev197
Sample App v1.3.0-rc4+dev35

Contributor Info
Avi Weiss @thnkslprpt

Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

This is probably a good thing, at least to display the status after load.

One could debate about returning the error status though - that will probably cause the app to exit itself, but in a "real" FSW environment the tables are loadable at runtime via command. So in that context a real app might just start but go dormant until a table is loaded. On the flip side, the app can also be restarted via command too.

I do concur that the status of CFE_TBL_Load() should be checked in some way though, it is an important return code.

@thnkslprpt
Copy link
Contributor Author

One could debate about returning the error status though - that will probably cause the app to exit itself, but in a "real" FSW environment the tables are loadable at runtime via command. So in that context a real app might just start but go dormant until a table is loaded. On the flip side, the app can also be restarted via command too.

Fair point Joe. Is it worth adding a comment above the return status line, something like:
/* The early return with error status below can be commented out/removed if user prefers silent fail and syslog report only */

Or just leave as is?

@dzbaker dzbaker added the CCB:Approved Indicates code approval by CCB label Oct 27, 2022
@dzbaker dzbaker added this to the Fornax milestone Nov 21, 2022
@dzbaker dzbaker modified the milestones: Fornax, Equuleus Dec 7, 2022
dzbaker added a commit to nasa/cFS that referenced this pull request Nov 13, 2023
*Combines:*

sch_lab v2.5.0-rc4+dev71
sample_app v1.3.0-rc4+dev65
to_lab v2.5.0-rc4+dev66
ci_lab v2.5.0-rc4+dev69
cFE v7.0.0-rc4+dev411
PSP v1.6.0-rc4+dev96

**Includes:**

*sch_lab*
- nasa/sch_lab#129
- nasa/sch_lab#149
- nasa/sch_lab#142
- nasa/sch_lab#134

*sample_app*
- nasa/sample_app#212
- nasa/sample_app#187
- nasa/sample_app#190

*to_lab*
- nasa/to_lab#168
- nasa/to_lab#134
- nasa/to_lab#146
- nasa/to_lab#148
- nasa/to_lab#152
- nasa/to_lab#158
- nasa/to_lab#163

*ci_lab*
- nasa/ci_lab#152
- nasa/ci_lab#153

*cFE*
- nasa/cFE#2462
- nasa/cFE#2408

*PSP*
- nasa/PSP#417

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman  <skliper@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Nov 13, 2023
2 tasks
@dzbaker dzbaker merged commit ccd0d57 into nasa:main Nov 14, 2023
dzbaker added a commit to nasa/cFS that referenced this pull request Nov 14, 2023
*Combines:*

sch_lab v2.5.0-rc4+dev71
sample_app v1.3.0-rc4+dev65
to_lab v2.5.0-rc4+dev66
ci_lab v2.5.0-rc4+dev69
cFE v7.0.0-rc4+dev411
PSP v1.6.0-rc4+dev96

**Includes:**

*sch_lab*
- nasa/sch_lab#129
- nasa/sch_lab#149
- nasa/sch_lab#142
- nasa/sch_lab#134

*sample_app*
- nasa/sample_app#212
- nasa/sample_app#187
- nasa/sample_app#190

*to_lab*
- nasa/to_lab#168
- nasa/to_lab#134
- nasa/to_lab#146
- nasa/to_lab#148
- nasa/to_lab#152
- nasa/to_lab#158
- nasa/to_lab#163

*ci_lab*
- nasa/ci_lab#152
- nasa/ci_lab#153

*cFE*
- nasa/cFE#2462
- nasa/cFE#2408

*PSP*
- nasa/PSP#417

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman  <skliper@users.noreply.github.com>
@thnkslprpt thnkslprpt deleted the fix-147-exit-initialization-if-CFE_TBL_Load-fails branch November 14, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code approval by CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to check the status of CFE_TBL_Load() call
3 participants