Skip to content

Commit

Permalink
Fix nasa#263, Consistent use of MsgId type
Browse files Browse the repository at this point in the history
Make CFE core apps consistent in their use of the CFE_SB_MsgId type
and with the CFE SB API.  This employs the CFE SB API whenever any
of the following needs to happen:

- Use of a CFE_SB_MsgId_t value within a printf (event, syslog, etc).
- Initialization of a CFE_SB_MsgId_t from an integer value
- Comparison of two CFE_SB_MsgId_t values
- Checking if a CFE_SB_MsgId_t value is within the valid set

A few new macros are introduced, mainly because the inline functions
that already existed for this purpose cannot be used where evaluation
must be done at compile time (e.g. constants, struct initialization).
These are initially just typecasts, but could become more interesting
in future revisions.
  • Loading branch information
jphickey committed Apr 13, 2020
1 parent 60a5f65 commit f0b101f
Show file tree
Hide file tree
Showing 23 changed files with 505 additions and 329 deletions.
31 changes: 19 additions & 12 deletions fsw/cfe-core/src/es/cfe_es_task.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,23 +251,30 @@ int32 CFE_ES_TaskInit(void)
/*
** Initialize housekeeping packet (clear user data area)
*/
CFE_SB_InitMsg(&CFE_ES_TaskData.HkPacket, CFE_ES_HK_TLM_MID, sizeof(CFE_ES_TaskData.HkPacket), true);
CFE_SB_InitMsg(&CFE_ES_TaskData.HkPacket,
CFE_SB_ValueToMsgId(CFE_ES_HK_TLM_MID),
sizeof(CFE_ES_TaskData.HkPacket), true);

/*
** Initialize shell output packet (clear user data area)
*/
CFE_SB_InitMsg(&CFE_ES_TaskData.ShellPacket, CFE_ES_SHELL_TLM_MID, sizeof(CFE_ES_TaskData.ShellPacket), true);
CFE_SB_InitMsg(&CFE_ES_TaskData.ShellPacket,
CFE_SB_ValueToMsgId(CFE_ES_SHELL_TLM_MID),
sizeof(CFE_ES_TaskData.ShellPacket), true);

/*
** Initialize single application telemetry packet
*/
CFE_SB_InitMsg(&CFE_ES_TaskData.OneAppPacket, CFE_ES_APP_TLM_MID, sizeof(CFE_ES_TaskData.OneAppPacket), true);
CFE_SB_InitMsg(&CFE_ES_TaskData.OneAppPacket,
CFE_SB_ValueToMsgId(CFE_ES_APP_TLM_MID),
sizeof(CFE_ES_TaskData.OneAppPacket), true);

/*
** Initialize memory pool statistics telemetry packet
*/
CFE_SB_InitMsg(&CFE_ES_TaskData.MemStatsPacket, CFE_ES_MEMSTATS_TLM_MID,
sizeof(CFE_ES_TaskData.MemStatsPacket), true);
CFE_SB_InitMsg(&CFE_ES_TaskData.MemStatsPacket,
CFE_SB_ValueToMsgId(CFE_ES_MEMSTATS_TLM_MID),
sizeof(CFE_ES_TaskData.MemStatsPacket), true);

/*
** Create Software Bus message pipe
Expand All @@ -282,7 +289,7 @@ int32 CFE_ES_TaskInit(void)
/*
** Subscribe to Housekeeping request commands
*/
Status = CFE_SB_SubscribeEx(CFE_ES_SEND_HK_MID, CFE_ES_TaskData.CmdPipe,
Status = CFE_SB_SubscribeEx(CFE_SB_ValueToMsgId(CFE_ES_SEND_HK_MID), CFE_ES_TaskData.CmdPipe,
CFE_SB_Default_Qos, CFE_ES_TaskData.LimitHK);
if ( Status != CFE_SUCCESS )
{
Expand All @@ -293,8 +300,8 @@ int32 CFE_ES_TaskInit(void)
/*
** Subscribe to ES task ground command packets
*/
Status = CFE_SB_SubscribeEx(CFE_ES_CMD_MID, CFE_ES_TaskData.CmdPipe,
CFE_SB_Default_Qos, CFE_ES_TaskData.LimitCmd);
Status = CFE_SB_SubscribeEx(CFE_SB_ValueToMsgId(CFE_ES_CMD_MID), CFE_ES_TaskData.CmdPipe,
CFE_SB_Default_Qos, CFE_ES_TaskData.LimitCmd);
if ( Status != CFE_SUCCESS )
{
CFE_ES_WriteToSysLog("ES:Cannot Subscribe to ES ground commands, RC = 0x%08X\n", (unsigned int)Status);
Expand Down Expand Up @@ -427,7 +434,7 @@ void CFE_ES_TaskPipe(CFE_SB_MsgPtr_t Msg)
uint16 CommandCode;

MessageID = CFE_SB_GetMsgId(Msg);
switch (MessageID)
switch (CFE_SB_MsgIdToValue(MessageID))
{
/*
** Housekeeping telemetry request
Expand Down Expand Up @@ -622,7 +629,7 @@ void CFE_ES_TaskPipe(CFE_SB_MsgPtr_t Msg)
default:
CFE_EVS_SendEvent(CFE_ES_CC1_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid ground command code: ID = 0x%X, CC = %d",
(unsigned int)MessageID, (int)CommandCode);
(unsigned int)CFE_SB_MsgIdToValue(MessageID), (int)CommandCode);
CFE_ES_TaskData.CommandErrorCounter++;
break;
}
Expand All @@ -632,7 +639,7 @@ void CFE_ES_TaskPipe(CFE_SB_MsgPtr_t Msg)

CFE_EVS_SendEvent(CFE_ES_MID_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid command pipe message ID: 0x%X",
(unsigned int)MessageID);
(unsigned int)CFE_SB_MsgIdToValue(MessageID));
CFE_ES_TaskData.CommandErrorCounter++;
break;
}
Expand Down Expand Up @@ -1692,7 +1699,7 @@ bool CFE_ES_VerifyCmdLength(CFE_SB_MsgPtr_t Msg, uint16 ExpectedLength)

CFE_EVS_SendEvent(CFE_ES_LEN_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid cmd length: ID = 0x%X, CC = %d, Exp Len = %d, Len = %d",
(unsigned int)MessageID, (int)CommandCode, (int)ExpectedLength, (int)ActualLength);
(unsigned int)CFE_SB_MsgIdToValue(MessageID), (int)CommandCode, (int)ExpectedLength, (int)ActualLength);
result = false;
CFE_ES_TaskData.CommandErrorCounter++;
}
Expand Down
20 changes: 13 additions & 7 deletions fsw/cfe-core/src/evs/cfe_evs_task.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ int32 CFE_EVS_EarlyInit ( void )
CFE_EVS_GlobalData.EVS_AppID = CFE_EVS_UNDEF_APPID;

/* Initialize housekeeping packet */
CFE_SB_InitMsg(&CFE_EVS_GlobalData.EVS_TlmPkt, CFE_EVS_HK_TLM_MID,
CFE_SB_InitMsg(&CFE_EVS_GlobalData.EVS_TlmPkt, CFE_SB_ValueToMsgId(CFE_EVS_HK_TLM_MID),
sizeof(CFE_EVS_GlobalData.EVS_TlmPkt), false);

/* Elements stored in the hk packet that have non-zero default values */
Expand Down Expand Up @@ -315,15 +315,15 @@ int32 CFE_EVS_TaskInit ( void )
}

/* Subscribe to command and telemetry requests coming in on the command pipe */
Status = CFE_SB_SubscribeEx(CFE_EVS_CMD_MID, CFE_EVS_GlobalData.EVS_CommandPipe,
Status = CFE_SB_SubscribeEx(CFE_SB_ValueToMsgId(CFE_EVS_CMD_MID), CFE_EVS_GlobalData.EVS_CommandPipe,
CFE_SB_Default_Qos, CFE_EVS_MSG_LIMIT);
if (Status != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("EVS:Subscribing to Cmds Failed:RC=0x%08X\n",(unsigned int)Status);
return Status;
}

Status = CFE_SB_SubscribeEx(CFE_EVS_SEND_HK_MID, CFE_EVS_GlobalData.EVS_CommandPipe,
Status = CFE_SB_SubscribeEx(CFE_SB_ValueToMsgId(CFE_EVS_SEND_HK_MID), CFE_EVS_GlobalData.EVS_CommandPipe,
CFE_SB_Default_Qos, CFE_EVS_MSG_LIMIT);
if (Status != CFE_SUCCESS)
{
Expand Down Expand Up @@ -354,8 +354,12 @@ int32 CFE_EVS_TaskInit ( void )
*/
void CFE_EVS_ProcessCommandPacket ( CFE_SB_MsgPtr_t EVS_MsgPtr )
{
CFE_SB_MsgId_t MessageID;

MessageID = CFE_SB_GetMsgId(EVS_MsgPtr);

/* Process all SB messages */
switch (CFE_SB_GetMsgId(EVS_MsgPtr))
switch (CFE_SB_MsgIdToValue(MessageID))
{
case CFE_EVS_CMD_MID:
/* EVS task specific command */
Expand All @@ -372,7 +376,7 @@ void CFE_EVS_ProcessCommandPacket ( CFE_SB_MsgPtr_t EVS_MsgPtr )
CFE_EVS_GlobalData.EVS_TlmPkt.Payload.CommandErrorCounter++;
EVS_SendEvent(CFE_EVS_ERR_MSGID_EID, CFE_EVS_EventType_ERROR,
"Invalid command packet, Message ID = 0x%08X",
(unsigned int)CFE_SB_GetMsgId(EVS_MsgPtr));
(unsigned int)CFE_SB_MsgIdToValue(MessageID));
break;
}

Expand Down Expand Up @@ -573,7 +577,8 @@ void CFE_EVS_ProcessGroundCommand ( CFE_SB_MsgPtr_t EVS_MsgPtr )

EVS_SendEvent(CFE_EVS_ERR_CC_EID, CFE_EVS_EventType_ERROR,
"Invalid command code -- ID = 0x%08x, CC = %d",
(unsigned int)CFE_SB_GetMsgId(EVS_MsgPtr), (int)CFE_SB_GetCmdCode(EVS_MsgPtr));
(unsigned int)CFE_SB_MsgIdToValue(CFE_SB_GetMsgId(EVS_MsgPtr)),
(int)CFE_SB_GetCmdCode(EVS_MsgPtr));
Status = CFE_STATUS_BAD_COMMAND_CODE;

break;
Expand Down Expand Up @@ -618,7 +623,8 @@ bool CFE_EVS_VerifyCmdLength(CFE_SB_MsgPtr_t Msg, uint16 ExpectedLength)

EVS_SendEvent(CFE_EVS_LEN_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid cmd length: ID = 0x%X, CC = %d, Exp Len = %d, Len = %d",
(unsigned int)MessageID, (int)CommandCode, (int)ExpectedLength, (int)ActualLength);
(unsigned int)CFE_SB_MsgIdToValue(MessageID),
(int)CommandCode, (int)ExpectedLength, (int)ActualLength);
result = false;
}

Expand Down
6 changes: 4 additions & 2 deletions fsw/cfe-core/src/evs/cfe_evs_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ void EVS_GenerateEventTelemetry(uint32 AppID, uint16 EventID, uint16 EventType,
int ExpandedLength;

/* Initialize EVS event packets */
CFE_SB_InitMsg(&LongEventTlm, CFE_EVS_LONG_EVENT_MSG_MID, sizeof(LongEventTlm), true);
CFE_SB_InitMsg(&LongEventTlm, CFE_SB_ValueToMsgId(CFE_EVS_LONG_EVENT_MSG_MID),
sizeof(LongEventTlm), true);
LongEventTlm.Payload.PacketID.EventID = EventID;
LongEventTlm.Payload.PacketID.EventType = EventType;

Expand Down Expand Up @@ -408,7 +409,8 @@ void EVS_GenerateEventTelemetry(uint32 AppID, uint16 EventID, uint16 EventType,
*
* This goes out on a separate message ID.
*/
CFE_SB_InitMsg(&ShortEventTlm, CFE_EVS_SHORT_EVENT_MSG_MID, sizeof(ShortEventTlm), true);
CFE_SB_InitMsg(&ShortEventTlm, CFE_SB_ValueToMsgId(CFE_EVS_SHORT_EVENT_MSG_MID),
sizeof(ShortEventTlm), true);
CFE_SB_SetMsgTime((CFE_SB_Msg_t *) &ShortEventTlm, *TimeStamp);
ShortEventTlm.Payload.PacketID = LongEventTlm.Payload.PacketID;
CFE_SB_SendMsg((CFE_SB_Msg_t *) &ShortEventTlm);
Expand Down
72 changes: 67 additions & 5 deletions fsw/cfe-core/src/inc/cfe_sb.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,54 @@
#define CFE_SB_SUBSCRIPTION 0 /**< \brief Subtype specifier used in #CFE_SB_SingleSubscriptionTlm_t by SBN App */
#define CFE_SB_UNSUBSCRIPTION 1 /**< \brief Subtype specified used in #CFE_SB_SingleSubscriptionTlm_t by SBN App */

#define CFE_SB_INVALID_MSG_ID 0xFFFF /**< \brief Initializer for #CFE_SB_MsgId_t values that will not match any real MsgId */
/* ------------------------------------------------------ */
/* Macro Constants for use with the CFE_SB_MsgId_t type */
/* ------------------------------------------------------ */

/**
* \brief Translation macro to convert from MsgId integer values to opaque/abstract API values
*
* This conversion exists in macro form to allow compile-time evaluation for constants, and
* should not be used directly in application code.
*
* For applications, use the CFE_SB_ValueToMsgId() inline function instead.
*
* \sa CFE_SB_ValueToMsgId()
*/
#define CFE_SB_MSGID_WRAP_VALUE(val) ((CFE_SB_MsgId_t)(val))

/**
* \brief Translation macro to convert to MsgId integer values from opaque/abstract API values
*
* This conversion exists in macro form to allow compile-time evaluation for constants, and
* should not be used directly in application code.
*
* For applications, use the CFE_SB_MsgIdToValue() inline function instead.
*
* \sa CFE_SB_MsgIdToValue()
*/
#define CFE_SB_MSGID_UNWRAP_VALUE(mid) ((CFE_SB_MsgId_Atom_t)(mid))

/**
* \brief Reserved value for CFE_SB_MsgId_t that will not match any valid MsgId
*
* This rvalue macro can be used for static/compile-time data initialization to ensure that
* the initialized value does not alias to a valid MsgId object.
*/
#define CFE_SB_MSGID_RESERVED CFE_SB_MSGID_WRAP_VALUE(-1)

/**
* \brief A literal of the CFE_SB_MsgId_t type representing an invalid ID
*
* This value should be used for runtime initialization of CFE_SB_MsgId_t values.
*
* \note This may be a compound literal in a future revision. Per C99, compound
* literals are lvalues, not rvalues, so this value should not be used in
* static/compile-time data initialization. For static data initialization
* purposes (rvalue), #CFE_SB_MSGID_RESERVED should be used instead.
* However, in the current implementation, they are equivalent.
*/
#define CFE_SB_INVALID_MSG_ID CFE_SB_MSGID_RESERVED

/*
** Macro Definitions
Expand Down Expand Up @@ -1278,7 +1325,21 @@ bool CFE_SB_ValidateChecksum(CFE_SB_MsgPtr_t MsgPtr);

/*****************************************************************************/
/**
* \brief Identifies whether a two #CFE_SB_MsgId_t values are equal
* \brief Identifies whether a given CFE_SB_MsgId_t is valid
*
* \par Description
* Implements a basic sanity check on the value provided
*
* \return Boolean message ID validity indicator
* \retval true Message ID is within the valid range
* \retval false Message ID is not within the valid range
*/
bool CFE_SB_IsValidMsgId(CFE_SB_MsgId_t MsgId);


/*****************************************************************************/
/**
* \brief Identifies whether two #CFE_SB_MsgId_t values are equal
*
* \par Description
* In cases where the #CFE_SB_MsgId_t type is not a simple integer
Expand All @@ -1296,7 +1357,7 @@ bool CFE_SB_ValidateChecksum(CFE_SB_MsgPtr_t MsgPtr);
*/
static inline bool CFE_SB_MsgId_Equal(CFE_SB_MsgId_t MsgId1, CFE_SB_MsgId_t MsgId2)
{
return (MsgId1 == MsgId2);
return CFE_SB_MSGID_UNWRAP_VALUE(MsgId1) == CFE_SB_MSGID_UNWRAP_VALUE(MsgId2);
}

/*****************************************************************************/
Expand Down Expand Up @@ -1327,7 +1388,7 @@ static inline bool CFE_SB_MsgId_Equal(CFE_SB_MsgId_t MsgId1, CFE_SB_MsgId_t MsgI
*/
static inline CFE_SB_MsgId_Atom_t CFE_SB_MsgIdToValue(CFE_SB_MsgId_t MsgId)
{
return MsgId;
return CFE_SB_MSGID_UNWRAP_VALUE(MsgId);
}

/*****************************************************************************/
Expand Down Expand Up @@ -1356,7 +1417,8 @@ static inline CFE_SB_MsgId_Atom_t CFE_SB_MsgIdToValue(CFE_SB_MsgId_t MsgId)
*/
static inline CFE_SB_MsgId_t CFE_SB_ValueToMsgId(CFE_SB_MsgId_Atom_t MsgIdValue)
{
return MsgIdValue;
CFE_SB_MsgId_t Result = CFE_SB_MSGID_WRAP_VALUE(MsgIdValue);
return Result;
}
/**@}*/

Expand Down
2 changes: 1 addition & 1 deletion fsw/cfe-core/src/inc/cfe_sb_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ typedef struct{
** Structure of one element of the map information in response to #CFE_SB_SEND_MAP_INFO_CC
*/
typedef struct{
CFE_SB_MsgId_Atom_t MsgId;/**< \brief Message Id which has been subscribed to */
CFE_SB_MsgId_t MsgId;/**< \brief Message Id which has been subscribed to */
CFE_SB_MsgRouteIdx_Atom_t Index;/**< \brief Routing table index where pipe destinations are found */
}CFE_SB_MsgMapFileEntry_t;

Expand Down
Loading

0 comments on commit f0b101f

Please sign in to comment.