Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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