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 #1417, #1387, #1393, generated coverage stubs for CFE core #1463

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Apr 30, 2021

Describe the contribution
Update CFE core stub libraries to use generated stubs, using the generator script now part of UT Assert. All existing stub logic is converted to a default handler routine.

This also entailed moving all of the internal API prototypes to be in the "core_private" interface lib rather than in "core_api". The
only reason this was not done earlier was due to issues with the stubs, but that is alleviated when using generated stubs.

Fixes #1417
Fixes #1387
Fixes #1393

Testing performed
Build and sanity check CFE, run all unit tests

Expected behavior changes
No impact to FSW.
For unit testing, this allows the stub handler to be completely replaced by a test case, bypassing any default handler logic, if the default handler function is not desired.

System(s) tested on
Ubuntu 20.04

Additional Context
Originally when the directory reorganization was done (#972) the "core internal" function prototypes were kept in core_api because that's where the stubs were, and splitting them would require splitting the stub library.

When using generated stubs, this is no longer an issue, and actually flips the problem around - such that to maintain the patterns these header files should be moved so that the stubs are generated properly with the right scope.

As a result - this moves the "internal" header files from core_api to core_private, but does not change them. This is where they really should have been all along.

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

@skliper
Copy link
Contributor

skliper commented Apr 30, 2021

Does this also fix #1393 and #1387? If so, link.

@skliper skliper added this to the 7.0.0 milestone Apr 30, 2021
@jphickey jphickey force-pushed the fix-1417-cfe-coverage-stubs branch from c5c31b4 to b35bc34 Compare April 30, 2021 12:58
@jphickey jphickey changed the title Fix #1417, generated coverage stubs for CFE core Fix #1417, 1387, 1393, generated coverage stubs for CFE core Apr 30, 2021
@jphickey jphickey force-pushed the fix-1417-cfe-coverage-stubs branch from 439a33d to 8201deb Compare April 30, 2021 13:08
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 5, 2021
@astrogeco astrogeco changed the title Fix #1417, 1387, 1393, generated coverage stubs for CFE core Fix #1417, #1387, #1393, generated coverage stubs for CFE core May 5, 2021
@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label May 5, 2021
@astrogeco
Copy link
Contributor

astrogeco commented May 5, 2021

CCB:2021-05-05 APPROVED with changes

  • Extends "handler function" concept from osal to cfe
  • Do we have a better naming scheme? We have hooks and handler.
  • Rename the files to handler.c instead of hooks.c. Open issue in osal to rename as well

Copy link
Contributor

@astrogeco astrogeco left a comment

Choose a reason for hiding this comment

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

Rename *hooks.c as *handler.c where appropriate

jphickey added a commit to jphickey/cFE that referenced this pull request May 6, 2021
CCB 2021-05-05 review item, matches name used in code.
Update CFE core stub libraries to use generated stubs, using the generator
script now part of UT Assert.  All existing stub logic is converted to
a default handler routine.

This also entailed moving all of the internal API prototypes to be
in the "core_private" interface lib rather than in "core_api".  The
only reason this was not done earlier was due to issues with the stubs,
but that is alleviated when using generated stubs.
This function should only be called internally from ES and
therefore does not need to be in public API.
CCB 2021-05-05 review item, matches name used in code.
@jphickey jphickey force-pushed the fix-1417-cfe-coverage-stubs branch from 2a2a434 to 2d2a7f9 Compare May 6, 2021 13:06
@jphickey jphickey requested a review from astrogeco May 6, 2021 13:07
@jphickey
Copy link
Contributor Author

jphickey commented May 6, 2021

Rebased to main to resolve conflict, and updated to rename hooks to handlers (see commit 2d2a7f9)

@jphickey jphickey dismissed astrogeco’s stale review May 6, 2021 15:10

changes made in commit 2d2a7f9

@astrogeco astrogeco changed the base branch from main to integration-candidate May 11, 2021 01:28
@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 11, 2021
@astrogeco astrogeco merged commit fc766c5 into nasa:integration-candidate May 11, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request May 11, 2021
nasa/cFE#1487, Remove broken travis-ci script

nasa/cFE#1463, generated coverage stubs for CFE core
nasa/cFE#1463, Move CFE_FS_RunBackgroundFileDump to internal API

nasa/cFE#1451, OSAL config file simplification
astrogeco added a commit to nasa/cFS that referenced this pull request May 12, 2021
nasa/cFE#1492, cFE v6.8.0-rc1+dev575
nasa/osal#996, osal v5.1.0-rc1+dev434

nasa/cFE#1487, Remove broken travis-ci script
nasa/cFE#1463, generated coverage stubs for CFE core
nasa/cFE#1463, Move CFE_FS_RunBackgroundFileDump to internal API
nasa/cFE#1451, OSAL config file simplification
nasa/cFE#1489, removes --quiet option so files checked go to stdout

nasa/osal#978, configuration guide updates
nasa/osal#974, improve documentation of UtAssert API calls
nasa/osal#977, update OS_TaskCreate doc
nasa/osal#997, Enable cppcheck results output
nasa/osal#980, Scrub return values
nasa/osal#992, add local mutex to BSP console
nasa/osal#993, do not require nonblock mode
astrogeco added a commit to nasa/cFS that referenced this pull request May 12, 2021
nasa/cFE#1492, cFE v6.8.0-rc1+dev575
nasa/osal#996, osal v5.1.0-rc1+dev434

nasa/cFE#1487, Remove broken travis-ci script
nasa/cFE#1463, generated coverage stubs for CFE core
nasa/cFE#1463, Move CFE_FS_RunBackgroundFileDump to internal API
nasa/cFE#1451, OSAL config file simplification
nasa/cFE#1489, removes --quiet option so files checked go to stdout

nasa/osal#978, configuration guide updates
nasa/osal#974, improve documentation of UtAssert API calls
nasa/osal#977, update OS_TaskCreate doc
nasa/osal#997, Enable cppcheck results output
nasa/osal#980, Scrub return values
nasa/osal#992, add local mutex to BSP console
nasa/osal#993, do not require nonblock mode

Co-authored-by: Jacob Hageman <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
@jphickey jphickey deleted the fix-1417-cfe-coverage-stubs branch May 14, 2021 14:23
@skliper
Copy link
Contributor

skliper commented Jun 29, 2021

Note this also fixed #1055

@skliper skliper linked an issue Jun 29, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
3 participants