-
Notifications
You must be signed in to change notification settings - Fork 37
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 #37, Opaque CFE_SB_MsgId_t values #38
Fix #37, Opaque CFE_SB_MsgId_t values #38
Conversation
3cc2bec
to
ab469df
Compare
Related to nasa/cFE#592 |
CCB 20200408 - Split up into parts. |
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.
Marking based on CCB 20200408
Reviving this -- necessary to fix compile errors with RTEMS and extended headers enabled. There was nothing to update in this app (SCH_LAB). Same commit as previous review. |
CCB 20200506 - APPROVED |
Do not assume CFE_SB_MsgId_t is implicitly integral in nature. When an integer value is required for printing or backward compatibility, use the explicit conversion routine to get this.
ab469df
to
c6f0ef5
Compare
Correction - this is almost the same, but the MID constants do not have a _VALUE suffix since the constants did not change names. |
Describe the contribution
Apply the
CFE_SB_MsgIdToValue()
andCFE_SB_ValueToMsgId()
routines where compatibility with an integer MsgId is necessary - syslog prints, events, compile-time MID#define
values.Fixes #37
Testing performed
Unit test
Execute CFE and sanity-check normal operation - send commands to app using
cmdUtil
and confirm commands are processed normally.Expected behavior changes
No impact to behavior.
System(s) tested on
Ubuntu 18.04 LTS, 64-bit
Additional context
In future versions of CFE SB the MsgId type might not be a simple integer, so this is one step in the direction of avoiding this assumption in apps.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.