From c43981bde6e276defa70296c6c6c833a1bff2756 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 6 Jan 2022 14:49:25 -0500 Subject: [PATCH] Fix #148, use proper types for MsgId and mid values Ensures use of the proper SB-provided types for MsgIDs and MsgId values (aka CFE_SB_MsgId_Atom_t). This ensures that the values are used and converted properly. --- fsw/platform_inc/cf_tbldefs.h | 4 +-- fsw/src/cf_app.c | 26 +++++++++--------- fsw/src/cf_cfdp.c | 8 +++--- fsw/src/cf_cfdp_sbintf.c | 3 ++- unit-test/cf_app_tests.c | 29 ++++++++++++-------- unit-test/utilities/cf_test_utils.c | 42 ----------------------------- unit-test/utilities/cf_test_utils.h | 3 --- 7 files changed, 38 insertions(+), 77 deletions(-) diff --git a/fsw/platform_inc/cf_tbldefs.h b/fsw/platform_inc/cf_tbldefs.h index 11353896..ed12dbf9 100644 --- a/fsw/platform_inc/cf_tbldefs.h +++ b/fsw/platform_inc/cf_tbldefs.h @@ -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 */ diff --git a/fsw/src/cf_app.c b/fsw/src/cf_app.c index 9f83b35a..20890d56 100644 --- a/fsw/src/cf_app.c +++ b/fsw/src/cf_app.c @@ -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) @@ -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; } } @@ -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); @@ -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; } } diff --git a/fsw/src/cf_cfdp.c b/fsw/src/cf_cfdp.c index 4196a4db..be6df85f 100644 --- a/fsw/src/cf_cfdp.c +++ b/fsw/src/cf_cfdp.c @@ -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; } diff --git a/fsw/src/cf_cfdp_sbintf.c b/fsw/src/cf_cfdp_sbintf.c index 6655d109..ca695b10 100644 --- a/fsw/src/cf_cfdp_sbintf.c +++ b/fsw/src/cf_cfdp_sbintf.c @@ -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 */ diff --git a/unit-test/cf_app_tests.c b/unit-test/cf_app_tests.c index facacc68..6e3bb7ec 100644 --- a/unit-test/cf_app_tests.c +++ b/unit-test/cf_app_tests.c @@ -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; @@ -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 */ @@ -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 */ @@ -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)); @@ -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 */ diff --git a/unit-test/utilities/cf_test_utils.c b/unit-test/utilities/cf_test_utils.c index 01bd2ff6..90584098 100644 --- a/unit-test/utilities/cf_test_utils.c +++ b/unit-test/utilities/cf_test_utils.c @@ -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 */ -} diff --git a/unit-test/utilities/cf_test_utils.h b/unit-test/utilities/cf_test_utils.h index 0a33883c..dbfd3241 100644 --- a/unit-test/utilities/cf_test_utils.h +++ b/unit-test/utilities/cf_test_utils.h @@ -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_ */