Skip to content

Conversation

@jphickey
Copy link
Contributor

@jphickey jphickey commented Apr 6, 2020

Describe the contribution
Apply the CFE_SB_MsgIdToValue() and CFE_SB_ValueToMsgId() routines where compatibility with an integer MsgId is necessary - syslog prints, events, compile-time MID #define values.

Fixes #53

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.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Apr 6, 2020
@jphickey jphickey force-pushed the fix-53-opaque-msgid branch from b9a5193 to 73c9083 Compare April 8, 2020 03:28
@jphickey
Copy link
Contributor Author

jphickey commented Apr 8, 2020

Note - pushed a simpler version of the update. However this one requires that nasa/cFE#592 be pulled first or in the same cycle.

The main changes are:

  • define SAMPLE_APP_CMD_MID to be properly typed as a CFE_SB_MsgId_t (not necessarily an integer)
  • Do not use switch() in the message dispatcher - use a nested if with CFE_SB_MsgId_Equal instead
  • Use CFE_SB_MsgIdToValue() converter whenever printing or syslogging or forming event strings, to get the integer representation of the MsgId.

@astrogeco
Copy link
Contributor

Related to nasa/cFE#592

@astrogeco
Copy link
Contributor

CCB 20200408 - Split up into parts.

@astrogeco astrogeco added CCB - 20200408 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 8, 2020
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.

Marking based on CCB 20200408

@skliper skliper added the CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. label Apr 28, 2020
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.
@jphickey jphickey force-pushed the fix-53-opaque-msgid branch from 73c9083 to e691304 Compare May 5, 2020 19:58
@astrogeco astrogeco added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) and removed CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. labels May 6, 2020
@astrogeco
Copy link
Contributor

CCB 20200506 - APPROVED

@astrogeco astrogeco added CCB-20200506 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels May 6, 2020
@astrogeco astrogeco added CCB:Approved Indicates code approval by CCB IC-20200429 labels May 8, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate May 8, 2020 19:23
@astrogeco astrogeco merged commit 8a77aa7 into nasa:integration-candidate May 8, 2020
@skliper skliper added this to the 1.2.0 milestone Jun 1, 2020
@jphickey jphickey deleted the fix-53-opaque-msgid branch March 29, 2021 19:20
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.

App should treat CFE_SB_MsgId_t values as opaque

3 participants