Skip to content

Commit

Permalink
Merge pull request nasa#150 from jphickey/fix-148-msgid-type
Browse files Browse the repository at this point in the history 
Fix nasa#148, use proper types for MsgId and mid values
  • Loading branch information
@astrogeco
astrogeco authored Jan 12, 2022
2 parents c0b1f53 + c43981b commit cfe2881
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 77 deletions.
4 changes: 2 additions & 2 deletions fsw/platform_inc/cf_tbldefs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ typedef struct CF_ChannelConfig
uint32 max_outgoing_messages_per_wakeup; /* max number of messages to send per wakeup (0 - unlimited) */
uint32 rx_max_messages_per_wakeup; /* max number of rx messages to process per wakeup */

uint16 apid_input; /* apid for incoming messages */
uint16 apid_output; /* apid for outgoing messages */
CFE_SB_MsgId_Atom_t mid_input; /* msgid integer value for incoming messages */
CFE_SB_MsgId_Atom_t mid_output; /* msgid integer value for outgoing messages */

uint16 pipe_depth_input; /* depth of pipe to receive incoming pdu */

Expand Down
26 changes: 12 additions & 14 deletions fsw/src/cf_app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,14 @@ int32 CF_Init(void)
{CF_EID_ERR_CMD_WHIST_WRITE, 0x0000},
};

int32 status = CFE_SUCCESS;
int32 status = CFE_SUCCESS;
static const CFE_SB_MsgId_Atom_t MID_VALUES[] = {CF_CMD_MID, CF_SEND_HK_MID, CF_WAKE_UP_MID};
uint32 i;

CF_AppData.run_status = CFE_ES_RunStatus_APP_RUN;

CFE_MSG_Init(&CF_AppData.hk.tlm_header.Msg, CF_HK_TLM_MID, sizeof(CF_AppData.hk));
CFE_MSG_Init(&CF_AppData.cfg.tlm_header.Msg, CF_CONFIG_TLM_MID, sizeof(CF_AppData.cfg));
CFE_MSG_Init(&CF_AppData.hk.tlm_header.Msg, CFE_SB_ValueToMsgId(CF_HK_TLM_MID), sizeof(CF_AppData.hk));
CFE_MSG_Init(&CF_AppData.cfg.tlm_header.Msg, CFE_SB_ValueToMsgId(CF_CONFIG_TLM_MID), sizeof(CF_AppData.cfg));

if ((status = CFE_EVS_Register(cf_event_filters, sizeof(cf_event_filters) / sizeof(*cf_event_filters),
CFE_EVS_EventFilter_BINARY)) != CFE_SUCCESS)
Expand All @@ -362,17 +364,13 @@ int32 CF_Init(void)
goto err_out;
}

for (i = 0; i < (sizeof(MID_VALUES) / sizeof(MID_VALUES[0])); ++i)
{
const CFE_SB_MsgId_t mids[] = {CF_CMD_MID, CF_SEND_HK_MID, CF_WAKE_UP_MID};
int i;

for (i = 0; i < (sizeof(mids) / sizeof(*mids)); ++i)
if ((status = CFE_SB_Subscribe(CFE_SB_ValueToMsgId(MID_VALUES[i]), CF_AppData.cmd_pipe)) != CFE_SUCCESS)
{
if ((status = CFE_SB_Subscribe(mids[i], CF_AppData.cmd_pipe)) != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("CF app: failed to subscribe to MID 0x%04x, returned 0x%08x", mids[i], status);
goto err_out;
}
CFE_ES_WriteToSysLog("CF app: failed to subscribe to MID 0x%04lx, returned 0x%08lx",
(unsigned long)MID_VALUES[i], (unsigned long)status);
goto err_out;
}
}

Expand Down Expand Up @@ -435,7 +433,7 @@ void CF_ProcessMsg(CFE_SB_Buffer_t *msg)

CFE_MSG_GetMsgId(&msg->Msg, &msg_id);

switch (msg_id)
switch (CFE_SB_MsgIdToValue(msg_id))
{
case CF_CMD_MID:
CF_ProcessGroundCommand(msg);
Expand All @@ -453,7 +451,7 @@ void CF_ProcessMsg(CFE_SB_Buffer_t *msg)
default:
++CF_AppData.hk.counters.err;
CFE_EVS_SendEvent(CF_EID_ERR_INIT_CMD_LENGTH, CFE_EVS_EventType_ERROR,
"CF: invalid command packet id=0x%02x", msg_id);
"CF: invalid command packet id=0x%lx", (unsigned long)CFE_SB_MsgIdToValue(msg_id));
break;
}
}
Expand Down
8 changes: 4 additions & 4 deletions fsw/src/cf_cfdp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1107,13 +1107,13 @@ int32 CF_CFDP_InitEngine(void)
goto err_out;
}

if ((ret =
CFE_SB_SubscribeLocal(CF_AppData.config_table->chan[i].apid_input, CF_AppData.engine.channels[i].pipe,
CF_AppData.config_table->chan[i].pipe_depth_input)) != CFE_SUCCESS)
if ((ret = CFE_SB_SubscribeLocal(CFE_SB_ValueToMsgId(CF_AppData.config_table->chan[i].mid_input),
CF_AppData.engine.channels[i].pipe,
CF_AppData.config_table->chan[i].pipe_depth_input)) != CFE_SUCCESS)
{
CFE_EVS_SendEvent(CF_EID_ERR_INIT_SUB, CFE_EVS_EventType_ERROR,
"CF: failed to subscribe to MID 0x%04x, returned 0x%08x",
CF_AppData.config_table->chan[i].apid_input, ret);
CF_AppData.config_table->chan[i].mid_input, ret);
goto err_out;
}

Expand Down
3 changes: 2 additions & 1 deletion fsw/src/cf_cfdp_sbintf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ CF_Logical_PduBuffer_t *CF_CFDP_MsgOutGet(const CF_Transaction_t *t, bool silent
goto error_out;
}

CFE_MSG_Init(&CF_AppData.engine.out.msg->Msg, CF_AppData.config_table->chan[t->chan_num].apid_output, 0);
CFE_MSG_Init(&CF_AppData.engine.out.msg->Msg,
CFE_SB_ValueToMsgId(CF_AppData.config_table->chan[t->chan_num].mid_output), 0);
++CF_AppData.engine.outgoing_counter; /* even if max_outgoing_messages_per_wakeup is 0 (unlimited), it's ok
to inc this */

Expand Down
29 changes: 18 additions & 11 deletions unit-test/cf_app_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ void Test_CF_ProcessMsg_ProcessGroundCommand(void)
/* Arrange */
CFE_SB_Buffer_t dummy_msg;
CFE_SB_Buffer_t *arg_msg = &dummy_msg;
CFE_SB_MsgId_t forced_MsgID = CF_CMD_MID;
CFE_SB_MsgId_t forced_MsgID = CFE_SB_ValueToMsgId(CF_CMD_MID);
CFE_SB_Buffer_t *context_CF_ProcessGroundCommand_msg;
CFE_MSG_GetMsgId_context_t context_CFE_MSG_GetMsgId;

Expand All @@ -637,7 +637,7 @@ void Test_CF_ProcessMsg_WakeUp(void)
/* Arrange */
CFE_SB_Buffer_t dummy_msg;
CFE_SB_Buffer_t *arg_msg = &dummy_msg;
CFE_SB_MsgId_t forced_MsgID = CF_WAKE_UP_MID;
CFE_SB_MsgId_t forced_MsgID = CFE_SB_ValueToMsgId(CF_WAKE_UP_MID);
CFE_MSG_GetMsgId_context_t context_CFE_MSG_GetMsgId;

/* CFE_MSG_GetMsgId uses return by ref */
Expand All @@ -661,7 +661,7 @@ void Test_CF_ProcessMsg_SendHk(void)
// CFE_MSG_Message_t dummy_Msg;
// CFE_SB_Buffer_t dummy_msg;
CFE_SB_Buffer_t *arg_msg = NULL;
CFE_SB_MsgId_t forced_MsgID = CF_SEND_HK_MID;
CFE_SB_MsgId_t forced_MsgID = CFE_SB_ValueToMsgId(CF_SEND_HK_MID);
CFE_MSG_GetMsgId_context_t context_CFE_MSG_GetMsgId;

/* CFE_MSG_GetMsgId uses return by ref */
Expand All @@ -687,14 +687,24 @@ void Test_CF_ProcessMsg_SendHk(void)
void Test_CF_ProcessMsg_UnrecognizedCommandEnterDefaultPath(void)
{
/* Arrange */
uint16 initial_err_count = CF_AppData.hk.counters.err;
CFE_SB_MsgId_t excepted_msg_ids[3] = {CF_CMD_MID, CF_WAKE_UP_MID, CF_SEND_HK_MID};
CFE_SB_MsgId_t forced_MsgID = Any_MsgId_ExceptThese(excepted_msg_ids, 3);
CFE_SB_Buffer_t *arg_msg = NULL;
const char *expected_Spec = "CF: invalid command packet id=0x%02x";
uint16 initial_err_count = CF_AppData.hk.counters.err;
CFE_SB_MsgId_Atom_t midval;
CFE_SB_MsgId_t forced_MsgID;
CFE_SB_Buffer_t *arg_msg = NULL;
CFE_MSG_GetMsgId_context_t context_CFE_MSG_GetMsgId;
CFE_EVS_SendEvent_context_t context_CFE_EVS_SendEvent;

/* do not use one of the three real MIDs.
* As this depends on configuration, should not hardcode values here
*/
midval = 1;
while (midval == CF_CMD_MID || midval == CF_WAKE_UP_MID || midval == CF_SEND_HK_MID)
{
++midval;
}

forced_MsgID = CFE_SB_ValueToMsgId(midval);

UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &forced_MsgID, sizeof(forced_MsgID), false);
UT_SetHookFunction(UT_KEY(CFE_MSG_GetMsgId), UT_Hook_CFE_MSG_GetMsgId, &context_CFE_MSG_GetMsgId);
UT_CF_ResetEventCapture(UT_KEY(CFE_EVS_SendEvent));
Expand All @@ -713,9 +723,6 @@ void Test_CF_ProcessMsg_UnrecognizedCommandEnterDefaultPath(void)
UtAssert_True(context_CFE_EVS_SendEvent.EventType == CFE_EVS_EventType_ERROR,
"CFE_EVS_SendEvent received EventType %u and should have received %u (CFE_EVS_EventType_ERROR)",
context_CFE_EVS_SendEvent.EventType, CFE_EVS_EventType_ERROR);
UtAssert_StrCmp(context_CFE_EVS_SendEvent.Spec, expected_Spec,
"CFE_EVS_SendEvent received expected Spec\n'%s' - Received\n'%s' - Expected",
context_CFE_EVS_SendEvent.Spec, expected_Spec);

} /* end Test_CF_ProcessMsg_UnrecognizedCommandEnterDefaultPath */

Expand Down
42 changes: 0 additions & 42 deletions unit-test/utilities/cf_test_utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,45 +796,3 @@ uint8 Any_cf_chan_num(void)
{
return Any_uint8_LessThan(CF_NUM_CHANNELS);
}

CFE_SB_MsgId_t Any_MsgId(void)
{
size_t msg_id_size = sizeof(CFE_SB_MsgId_t);

switch (msg_id_size)
{
case 4:
return Any_uint32();

case 2:
return Any_uint16();

default:
UtAssert_Failed("Any_MsgId_ExceptThese unimplemented sizeof(CFE_SB_MsgId_t) = %lu", msg_id_size);
UtAssert_Abort(__func__);
}

return (CFE_SB_MsgId_t)UT_UINT_16_DEFAULT; /* default for switch(msg_id_size) will always assert, this should not be
able to happen, but removes warning on build */
}

CFE_SB_MsgId_t Any_MsgId_ExceptThese(CFE_SB_MsgId_t exceptions[], uint8 num_exceptions)
{
size_t msg_id_size = sizeof(CFE_SB_MsgId_t);

switch (msg_id_size)
{
case 4:
return Any_uint32_ExceptThese((uint32 *)exceptions, num_exceptions);

case 2:
return Any_uint16_ExceptThese((uint16 *)exceptions, num_exceptions);

default:
UtAssert_Failed("Any_MsgId_ExceptThese unimplemented sizeof(CFE_SB_MsgId_t) = %lu", msg_id_size);
UtAssert_Abort(__func__);
}

return (CFE_SB_MsgId_t)UT_UINT_16_DEFAULT; /* default for switch(msg_id_size) will always assert, this should not be
able to happen, but removes warning on build */
}
3 changes: 0 additions & 3 deletions unit-test/utilities/cf_test_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,4 @@ CFE_Status_t Any_CFE_Status_t_Except(CFE_Status_t exception);
CFE_MSG_Size_t Any_CFE_MSG_Size_t(void);
CFE_MSG_Size_t Any_CFE_MSG_Size_t_LessThan(size_t ceiling);

CFE_SB_MsgId_t Any_MsgId(void);
CFE_SB_MsgId_t Any_MsgId_ExceptThese(CFE_SB_MsgId_t exceptions[], uint8 num_exceptions);

#endif /* _cf_test_utils_h_ */

0 comments on commit cfe2881

Please sign in to comment.