-
Notifications
You must be signed in to change notification settings - Fork 204
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 #2378, refactor SB to support additional use cases #2381
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
4f56d7c
to
c4141f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
* Not invoked outside of this unit | ||
* | ||
*-----------------------------------------------------------------*/ | ||
void CFE_SB_MessageTxn_GetEventDetails(const CFE_SB_MessageTxn_State_t *TxnPtr, const CFE_SB_PipeSetEntry_t *ContextPtr, |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
c4141f9
to
0d27889
Compare
Cleans up the internal SB implementation so it can better support future enhancements such as message integrity, additional header fields and timestamping.
0d27889
to
550e7f7
Compare
Rebased this PR to current main branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL-coding-standard found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
550e7f7
to
f20e60a
Compare
The unused variable static analysis warning is fixed, but code coverage is now complaining. The "missed branches" that it is reporting are mostly due to the fact that the |
329695d
to
ea02a67
Compare
Most "switch" cases now fixed (code coverage-wise) by adding a default case that does nothing, and always going through it. Looking at other modules there are still some code coverage lines that could potentially be fixed up before merging. |
Restructure some switch statements and add a "default" case so they are not flagged as having a missing branch in the coverage test.
ea02a67
to
48dd153
Compare
CCB 2 May 2024: Approved pending BVT run. |
*Combines:* cFE equuleus-rc1+dev141 osal equuleus-rc1+dev66 PSP equuleus-rc1+dev42 **Includes:** *cFS* - #751 - #762 *cFE* - nasa/cFE#2537 - nasa/cFE#2381 - nasa/cFE#2551 *osal* - nasa/osal#1460 *PSP* - nasa/PSP#430 Co-authored by: Justin Figueroa <[email protected]> Co-authored by: Joseph Hickey <[email protected]>
Checklist (Please check before submitting)
Describe the contribution
Cleans up the internal SB implementation so it can better support future enhancements such as message integrity, additional header fields and timestamping.
Fixes #2378
Testing performed
Full suite of tests on SB implementation
Expected behavior changes
API behavior is preserved, fully backward compatible
System(s) tested on
Debian
Additional context
Replaces #2367 based on code review and discussion. This PR represents the refactoring change, along with some other refinements. The routing change will be a separate PR.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.