From 6cd5bd0ee7287cef712c3929074cc5ee6a5ad57a Mon Sep 17 00:00:00 2001 From: Avi Weiss Date: Wed, 12 Jul 2023 21:06:05 +1000 Subject: [PATCH] Fix #288, Remove unnecessary `CF_UnionArgs_Payload_t` union --- fsw/inc/cf_msg.h | 12 +---- fsw/src/cf_cmd.c | 22 ++++----- unit-test/cf_cmd_tests.c | 98 ++++++++++++++++++++-------------------- 3 files changed, 61 insertions(+), 71 deletions(-) diff --git a/fsw/inc/cf_msg.h b/fsw/inc/cf_msg.h index 27587c51..3e8ec9d5 100644 --- a/fsw/inc/cf_msg.h +++ b/fsw/inc/cf_msg.h @@ -870,16 +870,6 @@ typedef struct CF_NoArgsCmd CFE_MSG_CommandHeader_t cmd_header; /**< \brief Command header */ } CF_NoArgsCmd_t; -/** - * \brief Command payload argument union to support 4 uint8's, 2 uint16's or 1 uint32 - */ -typedef union CF_UnionArgs_Payload -{ - uint32 dword; /**< \brief Generic uint32 argument */ - uint16 hword[2]; /**< \brief Generic uint16 array of arguments */ - uint8 byte[4]; /**< \brief Generic uint8 array of arguments */ -} CF_UnionArgs_Payload_t; - /** * \brief Generic command structure with arguments supports common handling on multiple command types * @@ -889,7 +879,7 @@ typedef union CF_UnionArgs_Payload typedef struct { CFE_MSG_CommandHeader_t cmd_header; /**< \brief Command header */ - CF_UnionArgs_Payload_t data; /**< \brief Generic command arguments */ + uint8 byte[4]; /**< \brief Generic uint8 array of arguments */ } CF_UnionArgsCmd_t; /** diff --git a/fsw/src/cf_cmd.c b/fsw/src/cf_cmd.c index 024a2d90..4d039fe8 100644 --- a/fsw/src/cf_cmd.c +++ b/fsw/src/cf_cmd.c @@ -62,7 +62,7 @@ void CF_CmdReset(CFE_SB_Buffer_t *msg) CF_UnionArgsCmd_t *cmd = (CF_UnionArgsCmd_t *)msg; static const char *names[5] = {"all", "cmd", "fault", "up", "down"}; /* 0=all, 1=cmd, 2=fault 3=up 4=down */ - uint8 param = cmd->data.byte[0]; + uint8 param = cmd->byte[0]; int i; int acc = 1; @@ -220,21 +220,21 @@ CFE_Status_t CF_DoChanAction(CF_UnionArgsCmd_t *cmd, const char *errstr, CF_Chan /* this function is generic for any ground command that takes a single channel * argument which must be less than CF_NUM_CHANNELS or 255 which is a special * value that means apply command to all channels */ - if (cmd->data.byte[0] == CF_ALL_CHANNELS) + if (cmd->byte[0] == CF_ALL_CHANNELS) { /* apply to all channels */ for (i = 0; i < CF_NUM_CHANNELS; ++i) ret |= fn(i, context); } - else if (cmd->data.byte[0] < CF_NUM_CHANNELS) + else if (cmd->byte[0] < CF_NUM_CHANNELS) { - ret = fn(cmd->data.byte[0], context); + ret = fn(cmd->byte[0], context); } else { /* bad parameter */ CFE_EVS_SendEvent(CF_EID_ERR_CMD_CHAN_PARAM, CFE_EVS_EventType_ERROR, - "CF: %s: channel parameter out of range. received %d", errstr, cmd->data.byte[0]); + "CF: %s: channel parameter out of range. received %d", errstr, cmd->byte[0]); ret = -1; } @@ -591,20 +591,20 @@ CFE_Status_t CF_DoEnableDisablePolldir(uint8 chan_num, const CF_ChanAction_BoolM int i; CFE_Status_t ret = CFE_SUCCESS; /* no need to bounds check chan_num, done in caller */ - if (context->msg->data.byte[1] == CF_ALL_POLLDIRS) + if (context->msg->byte[1] == CF_ALL_POLLDIRS) { /* all polldirs in channel */ for (i = 0; i < CF_MAX_POLLING_DIR_PER_CHAN; ++i) CF_AppData.config_table->chan[chan_num].polldir[i].enabled = context->barg; } - else if (context->msg->data.byte[1] < CF_MAX_POLLING_DIR_PER_CHAN) + else if (context->msg->byte[1] < CF_MAX_POLLING_DIR_PER_CHAN) { - CF_AppData.config_table->chan[chan_num].polldir[context->msg->data.byte[1]].enabled = context->barg; + CF_AppData.config_table->chan[chan_num].polldir[context->msg->byte[1]].enabled = context->barg; } else { CFE_EVS_SendEvent(CF_EID_ERR_CMD_POLLDIR_INVALID, CFE_EVS_EventType_ERROR, - "CF: enable/disable polldir: invalid polldir %d on channel %d", context->msg->data.byte[1], + "CF: enable/disable polldir: invalid polldir %d on channel %d", context->msg->byte[1], chan_num); ret = CF_ERROR; } @@ -703,7 +703,7 @@ CFE_Status_t CF_DoPurgeQueue(uint8 chan_num, CF_UnionArgsCmd_t *cmd) int pend = 0; int hist = 0; - switch (cmd->data.byte[1]) + switch (cmd->byte[1]) { case 0: /* pend */ pend = 1; @@ -720,7 +720,7 @@ CFE_Status_t CF_DoPurgeQueue(uint8 chan_num, CF_UnionArgsCmd_t *cmd) default: CFE_EVS_SendEvent(CF_EID_ERR_CMD_PURGE_ARG, CFE_EVS_EventType_ERROR, "CF: purge queue invalid arg %d", - cmd->data.byte[1]); + cmd->byte[1]); ret = -1; break; } diff --git a/unit-test/cf_cmd_tests.c b/unit-test/cf_cmd_tests.c index 852c90b6..9eb2300d 100644 --- a/unit-test/cf_cmd_tests.c +++ b/unit-test/cf_cmd_tests.c @@ -205,7 +205,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIsEqTo_5_SendEventAndRejectCommand(vo memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = 5; /* 5 is size of 'names' */ + msg->byte[0] = 5; /* 5 is size of 'names' */ CF_AppData.hk.counters.err = initial_hk_err_counter; @@ -229,7 +229,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIsGreaterThan_5_SendEventAndRejectCom memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = Any_uint8_GreaterThan(5); /* 5 is size of 'names' */ + msg->byte[0] = Any_uint8_GreaterThan(5); /* 5 is size of 'names' */ CF_AppData.hk.counters.err = initial_hk_err_counter; @@ -252,7 +252,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIs_command_AndResetHkCmdAndErrCountSe memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = CF_Reset_command; + msg->byte[0] = CF_Reset_command; CF_AppData.hk.counters.cmd = Any_uint16_Except(0); CF_AppData.hk.counters.err = Any_uint16_Except(0); @@ -277,7 +277,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIs_fault_ResetAllHkFaultCountSendEven memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = CF_Reset_fault; + msg->byte[0] = CF_Reset_fault; for (i = 0; i < CF_NUM_CHANNELS; ++i) { @@ -337,7 +337,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIs_up_AndResetAllHkRecvCountSendEvent memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = CF_Reset_up; + msg->byte[0] = CF_Reset_up; for (i = 0; i < CF_NUM_CHANNELS; ++i) { @@ -385,7 +385,7 @@ void Test_CF_CmdReset_tests_SWhenCommandByteIs_down_AndResetAllHkSentCountendEve memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = CF_Reset_down; + msg->byte[0] = CF_Reset_down; for (i = 0; i < CF_NUM_CHANNELS; ++i) { @@ -426,7 +426,7 @@ void Test_CF_CmdReset_tests_WhenCommandByteIs_all_AndResetAllMemValuesSendEvent( memset(&utbuf, 0, sizeof(utbuf)); - msg->data.byte[0] = CF_Reset_all; + msg->byte[0] = CF_Reset_all; CF_AppData.hk.counters.cmd = Any_uint16_Except(0); CF_AppData.hk.counters.err = Any_uint16_Except(0); @@ -651,7 +651,7 @@ void Test_CF_DoChanAction_CF_ALL_CHANNELS_WhenAny_fn_returns_1_Return_1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = CF_ALL_CHANNELS; + arg_cmd->byte[0] = CF_ALL_CHANNELS; UT_SetDeferredRetcode(UT_KEY(Chan_action_fn_t), random_fn_call, 1); @@ -680,7 +680,7 @@ void Test_CF_DoChanAction_CF_ALL_CHANNELS_WhenAll_fn_return_1_Return_1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = CF_ALL_CHANNELS; + arg_cmd->byte[0] = CF_ALL_CHANNELS; UT_SetDefaultReturnValue(UT_KEY(Chan_action_fn_t), 1); @@ -709,7 +709,7 @@ void Test_CF_DoChanAction_CF_ALL_CHANNELS_WhenNo_fn_returns_0_Return_0(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = CF_ALL_CHANNELS; + arg_cmd->byte[0] = CF_ALL_CHANNELS; UT_SetDefaultReturnValue(UT_KEY(Chan_action_fn_t), 0); @@ -738,7 +738,7 @@ void Test_CF_DoChanAction_WhenChannel_fn_ActionReturns_1_Return_1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = Any_cf_channel(); + arg_cmd->byte[0] = Any_cf_channel(); UT_SetDefaultReturnValue(UT_KEY(Chan_action_fn_t), 1); @@ -767,7 +767,7 @@ void Test_CF_DoChanAction_WhenChannel_fn_ActionReturns_0_Return_1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = Any_cf_channel(); + arg_cmd->byte[0] = Any_cf_channel(); UT_SetDefaultReturnValue(UT_KEY(Chan_action_fn_t), 0); @@ -796,7 +796,7 @@ void Test_CF_DoChanAction_WhenChanNumberEq_CF_NUM_CHANNELS_Return_neg1_And_SendE memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[0] = CF_NUM_CHANNELS; + arg_cmd->byte[0] = CF_NUM_CHANNELS; /* Act */ local_result = CF_DoChanAction(arg_cmd, arg_errstr, arg_fn, arg_context); @@ -809,8 +809,8 @@ void Test_CF_DoChanAction_WhenChanNumberEq_CF_NUM_CHANNELS_Return_neg1_And_SendE UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); UT_CF_AssertEventID(CF_EID_ERR_CMD_CHAN_PARAM); - UtAssert_True(local_result == -1, - "CF_DoChanAction returned %d and should be -1 (cmd->data.byte[0] >= CF_NUM_CHANNELS)", local_result); + UtAssert_True(local_result == -1, "CF_DoChanAction returned %d and should be -1 (cmd->byte[0] >= CF_NUM_CHANNELS)", + local_result); } void Test_CF_DoChanAction_WhenBadChannelNumber_Return_neg1_And_SendEvent(void) @@ -828,16 +828,16 @@ void Test_CF_DoChanAction_WhenBadChannelNumber_Return_neg1_And_SendEvent(void) memset(&utbuf, 0, sizeof(utbuf)); /* force CF_ALL_CHANNELS to not be a selection possibility */ - arg_cmd->data.byte[0] = CF_ALL_CHANNELS; - while (arg_cmd->data.byte[0] == CF_ALL_CHANNELS) + arg_cmd->byte[0] = CF_ALL_CHANNELS; + while (arg_cmd->byte[0] == CF_ALL_CHANNELS) { if (catastrophe_count == 10) /* 10 is arbitrary */ { UtAssert_Message(UTASSERT_CASETYPE_ABORT, __FILE__, __LINE__, - "CANNOT make arg_cmd->data.byte[0] != CF_ALL_CHANNELS in 10 tries"); + "CANNOT make arg_cmd->byte[0] != CF_ALL_CHANNELS in 10 tries"); } - arg_cmd->data.byte[0] = Any_uint8_GreaterThan_or_EqualTo(CF_NUM_CHANNELS); + arg_cmd->byte[0] = Any_uint8_GreaterThan_or_EqualTo(CF_NUM_CHANNELS); ++catastrophe_count; } @@ -852,8 +852,8 @@ void Test_CF_DoChanAction_WhenBadChannelNumber_Return_neg1_And_SendEvent(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); UT_CF_AssertEventID(CF_EID_ERR_CMD_CHAN_PARAM); - UtAssert_True(local_result == -1, - "CF_DoChanAction returned %d and should be -1 (cmd->data.byte[0] >= CF_NUM_CHANNELS)", local_result); + UtAssert_True(local_result == -1, "CF_DoChanAction returned %d and should be -1 (cmd->byte[0] >= CF_NUM_CHANNELS)", + local_result); } /******************************************************************************* @@ -909,7 +909,7 @@ void Test_CF_CmdFreeze_Set_frozen_To_1_AndAcceptCommand(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = chan_num; + msg->byte[0] = chan_num; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -939,7 +939,7 @@ void Test_CF_CmdFreeze_Set_frozen_To_1_AndRejectCommand(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = CF_NUM_CHANNELS + 1; + msg->byte[0] = CF_NUM_CHANNELS + 1; CF_AppData.hk.counters.cmd = 0; @@ -972,7 +972,7 @@ void Test_CF_CmdThaw_Set_frozen_To_0_AndAcceptCommand(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = chan_num; + msg->byte[0] = chan_num; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -1002,7 +1002,7 @@ void Test_CF_CmdThaw_Set_frozen_To_0_AndRejectCommand(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = CF_NUM_CHANNELS + 1; + msg->byte[0] = CF_NUM_CHANNELS + 1; CF_AppData.hk.counters.cmd = 0; @@ -1627,7 +1627,7 @@ void Test_CF_CmdEnableDequeue_Success(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = chan_num; + msg->byte[0] = chan_num; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -1664,7 +1664,7 @@ void Test_CF_CmdEnableDequeue_Failure(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = CF_NUM_CHANNELS + 1; + msg->byte[0] = CF_NUM_CHANNELS + 1; CF_AppData.hk.counters.err = 0; @@ -1701,7 +1701,7 @@ void Test_CF_CmdDisableDequeue_Success(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = chan_num; + msg->byte[0] = chan_num; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -1737,7 +1737,7 @@ void Test_CF_CmdDisableDequeue_Failure(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = CF_NUM_CHANNELS + 1; + msg->byte[0] = CF_NUM_CHANNELS + 1; CF_AppData.hk.counters.err = 0; @@ -1774,7 +1774,7 @@ void Test_CF_DoEnableDisablePolldir_When_CF_ALL_CHANNELS_SetAllPolldirsInChannel CF_AppData.config_table = &config_table; - msg->data.byte[1] = CF_ALL_CHANNELS; + msg->byte[1] = CF_ALL_CHANNELS; context.msg = msg; context.barg = Any_bool_arg_t_barg(); @@ -1813,7 +1813,7 @@ void Test_CF_DoEnableDisablePolldir_WhenSetToSpecificPolldirSetPolldirFrom_conte CF_AppData.config_table = &config_table; - msg->data.byte[1] = polldir; + msg->byte[1] = polldir; context.msg = msg; context.barg = Any_bool_arg_t_barg(); @@ -1845,7 +1845,7 @@ void Test_CF_DoEnableDisablePolldir_FailPolldirEq_CF_MAX_POLLING_DIR_PER_CHAN_An CF_AppData.config_table = &config_table; - msg->data.byte[1] = CF_MAX_POLLING_DIR_PER_CHAN; + msg->byte[1] = CF_MAX_POLLING_DIR_PER_CHAN; context.msg = msg; context.barg = Any_bool_arg_t_barg(); @@ -1875,7 +1875,7 @@ void Test_CF_DoEnableDisablePolldir_FailAnyBadPolldirSendEvent(void) CF_AppData.config_table = &config_table; - msg->data.byte[1] = CF_MAX_POLLING_DIR_PER_CHAN; + msg->byte[1] = CF_MAX_POLLING_DIR_PER_CHAN; context.msg = msg; context.barg = Any_bool_arg_t_barg(); @@ -1913,10 +1913,10 @@ void Test_CF_CmdEnablePolldir_SuccessWhenActionSuccess(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; /* Arrange unstubbable: CF_DoEnableDisablePolldir */ - msg->data.byte[1] = polldir; + msg->byte[1] = polldir; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -1948,10 +1948,10 @@ void Test_CF_CmdEnablePolldir_FailWhenActionFail(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; /* Arrange unstubbable: CF_DoEnableDisablePolldir */ - msg->data.byte[1] = error_polldir; + msg->byte[1] = error_polldir; CF_AppData.hk.counters.err = initial_hk_err_counter; /* Act */ @@ -1989,10 +1989,10 @@ void Test_CF_CmdDisablePolldir_SuccessWhenActionSuccess(void) CF_AppData.config_table = &config_table; /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; /* Arrange unstubbable: CF_DoEnableDisablePolldir */ - msg->data.byte[1] = polldir; + msg->byte[1] = polldir; CF_AppData.hk.counters.cmd = initial_hk_cmd_counter; @@ -2024,10 +2024,10 @@ void Test_CF_CmdDisablePolldir_FailWhenActionFail(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; /* Arrange unstubbable: CF_DoEnableDisablePolldir */ - msg->data.byte[1] = error_polldir; + msg->byte[1] = error_polldir; CF_AppData.hk.counters.err = initial_hk_err_counter; @@ -2123,7 +2123,7 @@ void Test_CF_DoPurgeQueue_PendOnly(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[1] = 0; /* pend */ + arg_cmd->byte[1] = 0; /* pend */ UT_SetHandlerFunction(UT_KEY(CF_CList_Traverse), UT_AltHandler_CF_CList_Traverse_POINTER, &context_CF_CList_Traverse); @@ -2159,7 +2159,7 @@ void Test_CF_DoPurgeQueue_HistoryOnly(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[1] = 1; /* history */ + arg_cmd->byte[1] = 1; /* history */ /* set correct context type for CF_CList_Traverse stub */ UT_SetHandlerFunction(UT_KEY(CF_CList_Traverse), UT_AltHandler_CF_CList_Traverse_POINTER, @@ -2199,7 +2199,7 @@ void Test_CF_DoPurgeQueue_Both(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[1] = 2; /* both */ + arg_cmd->byte[1] = 2; /* both */ /* set correct context type for CF_CList_Traverse stub */ /* this must use data buffer hack to pass multiple contexts */ @@ -2239,7 +2239,7 @@ void Test_CF_DoPurgeQueue_GivenBad_data_byte_1_SendEventAndReturn_neg1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[1] = 3; /* 3 is first default value */ + arg_cmd->byte[1] = 3; /* 3 is first default value */ /* Act */ local_result = CF_DoPurgeQueue(arg_chan_num, arg_cmd); @@ -2263,7 +2263,7 @@ void Test_CF_DoPurgeQueue_AnyGivenBad_data_byte_1_SendEventAndReturn_neg1(void) memset(&utbuf, 0, sizeof(utbuf)); - arg_cmd->data.byte[1] = Any_uint8_GreaterThan_or_EqualTo(3); + arg_cmd->byte[1] = Any_uint8_GreaterThan_or_EqualTo(3); /* Act */ local_result = CF_DoPurgeQueue(arg_chan_num, arg_cmd); @@ -2296,10 +2296,10 @@ void Test_CF_CmdPurgeQueue_FailWhenActionFail(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; /* Arrange unstubbable: CF_DoPurgeQueue */ - msg->data.byte[1] = error_purge; + msg->byte[1] = error_purge; CF_AppData.hk.counters.err = initial_hk_err_counter; @@ -2326,7 +2326,7 @@ void Test_CF_CmdPurgeQueue_SuccessWhenActionSuccess(void) memset(&utbuf, 0, sizeof(utbuf)); /* Arrange unstubbable: CF_DoChanAction */ - msg->data.byte[0] = channel; + msg->byte[0] = channel; CF_AppData.hk.counters.cmd = 0;