From bad8af3511cd9a0ba0e16747a1df63005c9c95f9 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 8 Sep 2023 15:03:34 -0400 Subject: [PATCH] Fix #101, add header update platform config This adds a new SC platform configuration option: SC_PLATFORM_SC_ENABLE_HEADER_UPDATE This option is translated to the UpdateHeader option of CFE_SB_TransmitMsg() - If set to "false" (default) this replicates current behavior. In this mode, commands within ATS/RTS tables are expected to be fully initialized, including all headers and checksums, and the message will be passed verbatim to the software bus. - If set to "true" (new option) this permits the commands to be timestamped and sequenced according to the real time system state. The headers will be updated as part of sending the message on the bus, and a valid checksum will be computed in real time. Note - There is currently no way to perform a checksum computation during table generation, so inserting a valid checksum within in ATS/RTS table entries is difficult when a robust checksum algorithm is used. --- fsw/inc/sc_platform_cfg.h | 32 ++++++++++++++++++++ fsw/src/sc_app.c | 2 ++ fsw/src/sc_app.h | 3 +- fsw/src/sc_cmds.c | 35 ++++++++++++++-------- unit-test/sc_cmds_tests.c | 62 +++++++++++++-------------------------- 5 files changed, 80 insertions(+), 54 deletions(-) diff --git a/fsw/inc/sc_platform_cfg.h b/fsw/inc/sc_platform_cfg.h index 42a5927..fa925d2 100644 --- a/fsw/inc/sc_platform_cfg.h +++ b/fsw/inc/sc_platform_cfg.h @@ -30,6 +30,38 @@ * \{ */ +/** + * \brief Whether to update headers on commands when sending on software bus + * + * \par Description: + * + * This controls whether to update the command header on the outgoing messages + * produced by the SC app. + * + * - If the messages in the ATS/RTS tables are expected to be completely + * initialized, including the sequence number and any required checksum + * field, then this should be configured as "false" such that these fields + * will not be modified during message transmission. The UpdateHeader + * parameter to CFE_SB_TransmitMsg() will be passed as false, and the message + * will be broadcast on the software bus verbatim. This replicates historical + * behavior of SC. + * + * - If the messages in the ATS/RTS tables need a real-time header update as + * part of the sending process, this should be configured as "true". Any + * checksum embedded within the ATS/RTS will be ignored. The UpdateHeader + * parameter to CFE_SB_TransmitMsg() will be passed as true, such that the + * command will be sequenced and timestamped with current values. A correct + * checksum will be computed based on the updated header values, and the + * resulting message is broadcast on the software bus. + * + * \sa + * CFE_SB_TransmitMsg() - UpdateHeader parameter + * + * \par Limits: + * Must be true or false + */ +#define SC_PLATFORM_ENABLE_HEADER_UPDATE false + /** * \brief Max number of commands per second * diff --git a/fsw/src/sc_app.c b/fsw/src/sc_app.c index 2bdd900..9750212 100644 --- a/fsw/src/sc_app.c +++ b/fsw/src/sc_app.c @@ -150,6 +150,8 @@ CFE_Status_t SC_AppInit(void) /* Continue ATS execution if ATS command checksum fails */ SC_OperData.HkPacket.Payload.ContinueAtsOnFailureFlag = SC_CONT_ON_FAILURE_START; + SC_AppData.EnableHeaderUpdate = SC_PLATFORM_ENABLE_HEADER_UPDATE; + /* Make sure nothing is running */ SC_AppData.NextProcNumber = SC_NONE; SC_AppData.NextCmdTime[SC_ATP] = SC_MAX_TIME; diff --git a/fsw/src/sc_app.h b/fsw/src/sc_app.h index 90baaba..a031180 100644 --- a/fsw/src/sc_app.h +++ b/fsw/src/sc_app.h @@ -280,10 +280,11 @@ typedef struct These offsets correspond to the addresses of ATS commands located in the ATS table. The index used is the ATS command index with values from 0 to SC_MAX_ATS_CMDS-1 */ + bool EnableHeaderUpdate; /**< \brief whether to update headers in outgoing messages */ + uint8 NextProcNumber; /**< \brief the next command processor number */ SC_AbsTimeTag_t NextCmdTime[2]; /**< \brief The overall next command time 0 - ATP, 1- RTP*/ SC_AbsTimeTag_t CurrentTime; /**< \brief this is the current time for SC */ - uint16 Unused; /**< \brief Unused */ uint16 AutoStartRTS; /**< \brief Start selected auto-exec RTS after init */ uint16 AppendWordCount; /**< \brief Size of cmd entries in current Append ATS table */ } SC_AppData_t; diff --git a/fsw/src/sc_cmds.c b/fsw/src/sc_cmds.c index 621c983..b54b4e3 100644 --- a/fsw/src/sc_cmds.c +++ b/fsw/src/sc_cmds.c @@ -63,9 +63,9 @@ void SC_ProcessAtpCmd(void) CFE_Status_t Result; bool AbortATS = false; SC_AtsEntry_t * EntryPtr; - CFE_SB_MsgId_t MessageID = CFE_SB_INVALID_MSG_ID; - CFE_MSG_FcnCode_t CommandCode = 0; - bool ChecksumValid = 0; + CFE_SB_MsgId_t MessageID = CFE_SB_INVALID_MSG_ID; + CFE_MSG_FcnCode_t CommandCode = 0; + bool ChecksumValid; /* ** The following conditions must be met before the ATS command will be @@ -98,11 +98,17 @@ void SC_ProcessAtpCmd(void) if (EntryPtr->Header.CmdNumber == (SC_ATS_CMD_INDEX_TO_NUM(CmdIndex))) { /* - ** Check the checksum on the command - ** + * Check the checksum on the command + * If this feature is disabled, just skip and assume valid + * Note that the checksum will be re-computed when the message is sent. */ - CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid); - if (ChecksumValid == true) + ChecksumValid = SC_AppData.EnableHeaderUpdate; + if (!SC_AppData.EnableHeaderUpdate) + { + /* If header update is NOT enabled, confirm this table entry has a valid checksum already */ + CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid); + } + if (ChecksumValid) { /* ** Count the command for the rate limiter @@ -151,7 +157,7 @@ void SC_ProcessAtpCmd(void) } else { - Result = CFE_SB_TransmitMsg(&EntryPtr->Msg, true); + Result = CFE_SB_TransmitMsg(&EntryPtr->Msg, SC_AppData.EnableHeaderUpdate); if (Result == CFE_SUCCESS) { @@ -294,7 +300,7 @@ void SC_ProcessRtpCommand(void) uint16 RtsIndex; /* the RTS index for the cmd */ uint16 CmdOffset; /* the location of the cmd */ CFE_Status_t Result; - bool ChecksumValid = false; + bool ChecksumValid; /* ** The following conditions must be met before a RTS command is executed: @@ -329,14 +335,19 @@ void SC_ProcessRtpCommand(void) */ EntryPtr = (SC_RtsEntry_t *)&SC_OperData.RtsTblAddr[RtsIndex][CmdOffset]; - CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid); - if (ChecksumValid == true) + ChecksumValid = SC_AppData.EnableHeaderUpdate; + if (!SC_AppData.EnableHeaderUpdate) + { + /* If header update is NOT enabled, confirm this table entry has a valid checksum already */ + CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid); + } + if (ChecksumValid) { /* ** Try Sending the command on the Software Bus */ - Result = CFE_SB_TransmitMsg(&EntryPtr->Msg, true); + Result = CFE_SB_TransmitMsg(&EntryPtr->Msg, SC_AppData.EnableHeaderUpdate); if (Result == CFE_SUCCESS) { diff --git a/unit-test/sc_cmds_tests.c b/unit-test/sc_cmds_tests.c index f0733ae..86ac61d 100644 --- a/unit-test/sc_cmds_tests.c +++ b/unit-test/sc_cmds_tests.c @@ -64,7 +64,6 @@ void SC_ProcessAtpCmd_Test_SwitchCmd(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_SWITCH_ATS_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[0][0]; Entry->CmdNumber = 1; @@ -78,14 +77,12 @@ void SC_ProcessAtpCmd_Test_SwitchCmd(void) SC_OperData.AtsCmdStatusTblAddr[0][0] = SC_LOADED; SC_AppData.AtsCmdIndexBuffer[0][0] = 0; + SC_AppData.EnableHeaderUpdate = true; + /* Set return value for CFE_TIME_Compare to make SC_CompareAbsTime return false, to satisfy first if-statement of * SC_ProcessAtpCmd, and for all other calls to CFE_TIME_Compare called from subfunctions reached by this test */ UT_SetHookFunction(UT_KEY(CFE_TIME_Compare), Ut_CFE_TIME_CompareHookAlessthanB, NULL); - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); - /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); @@ -111,7 +108,6 @@ void SC_ProcessAtpCmd_Test_NonSwitchCmd(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_NOOP_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[0][0]; Entry->CmdNumber = 1; @@ -125,14 +121,12 @@ void SC_ProcessAtpCmd_Test_NonSwitchCmd(void) SC_OperData.AtsCmdStatusTblAddr[0][0] = SC_LOADED; SC_AppData.AtsCmdIndexBuffer[0][0] = 0; + SC_AppData.EnableHeaderUpdate = true; + /* Set return value for CFE_TIME_Compare to make SC_CompareAbsTime return false, to satisfy first if-statement of * SC_ProcessAtpCmd, and for all other calls to CFE_TIME_Compare called from subfunctions reached by this test */ UT_SetHookFunction(UT_KEY(CFE_TIME_Compare), Ut_CFE_TIME_CompareHookAlessthanB, NULL); - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); - /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); @@ -158,7 +152,6 @@ void SC_ProcessAtpCmd_Test_InlineSwitchError(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_SWITCH_ATS_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[0][0]; Entry->CmdNumber = 1; @@ -177,8 +170,7 @@ void SC_ProcessAtpCmd_Test_InlineSwitchError(void) UT_SetHookFunction(UT_KEY(CFE_TIME_Compare), Ut_CFE_TIME_CompareHookAlessthanB, NULL); /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); + SC_AppData.EnableHeaderUpdate = true; /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ @@ -207,7 +199,6 @@ void SC_ProcessAtpCmd_Test_SBErrorAtsA(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_NOOP_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[0][0]; Entry->CmdNumber = 1; @@ -223,9 +214,7 @@ void SC_ProcessAtpCmd_Test_SBErrorAtsA(void) SC_OperData.AtsCmdStatusTblAddr[0][0] = SC_LOADED; SC_AppData.AtsCmdIndexBuffer[0][0] = 0; - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); + SC_AppData.EnableHeaderUpdate = true; /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ @@ -257,7 +246,6 @@ void SC_ProcessAtpCmd_Test_SBErrorAtsB(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_NOOP_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[1][0]; Entry->CmdNumber = 1; @@ -273,9 +261,7 @@ void SC_ProcessAtpCmd_Test_SBErrorAtsB(void) SC_OperData.AtsCmdStatusTblAddr[1][0] = SC_LOADED; SC_AppData.AtsCmdIndexBuffer[1][0] = 0; - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); + SC_AppData.EnableHeaderUpdate = true; /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ @@ -325,6 +311,8 @@ void SC_ProcessAtpCmd_Test_ChecksumFailedAtsA(void) SC_OperData.HkPacket.Payload.ContinueAtsOnFailureFlag = false; + SC_AppData.EnableHeaderUpdate = false; + /* Set to return false in order to generate error message SC_ATS_CHKSUM_ERR_EID */ ChecksumValid = false; UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); @@ -375,6 +363,8 @@ void SC_ProcessAtpCmd_Test_ChecksumFailedAtsB(void) SC_OperData.HkPacket.Payload.ContinueAtsOnFailureFlag = false; + SC_AppData.EnableHeaderUpdate = false; + /* Set to return false in order to generate error message SC_ATS_CHKSUM_ERR_EID */ ChecksumValid = false; UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); @@ -425,6 +415,8 @@ void SC_ProcessAtpCmd_Test_ChecksumFailedAtsAContinue(void) SC_OperData.HkPacket.Payload.ContinueAtsOnFailureFlag = true; + SC_AppData.EnableHeaderUpdate = false; + /* Set to return false in order to generate error message SC_ATS_CHKSUM_ERR_EID */ ChecksumValid = false; UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); @@ -634,7 +626,6 @@ void SC_ProcessAtpCmd_Test_CmdMid(void) SC_AtsEntryHeader_t *Entry; CFE_SB_MsgId_t TestMsgId = CFE_SB_INVALID_MSG_ID; CFE_MSG_FcnCode_t FcnCode = SC_SWITCH_ATS_CC; - bool ChecksumValid; Entry = (SC_AtsEntryHeader_t *)&SC_OperData.AtsTblAddr[0][0]; Entry->CmdNumber = 1; @@ -648,14 +639,12 @@ void SC_ProcessAtpCmd_Test_CmdMid(void) SC_OperData.AtsCmdStatusTblAddr[0][0] = SC_LOADED; SC_AppData.AtsCmdIndexBuffer[0][0] = 0; + SC_AppData.EnableHeaderUpdate = true; + /* Set return value for CFE_TIME_Compare to make SC_CompareAbsTime return false, to satisfy first if-statement of * SC_ProcessAtpCmd, and for all other calls to CFE_TIME_Compare called from subfunctions reached by this test */ UT_SetHookFunction(UT_KEY(CFE_TIME_Compare), Ut_CFE_TIME_CompareHookAlessthanB, NULL); - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); - /* Set these two functions to return these values in order to statisfy the if-statement from which they are both * called */ UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); @@ -677,17 +666,13 @@ void SC_ProcessAtpCmd_Test_CmdMid(void) void SC_ProcessRtpCommand_Test_Nominal(void) { - bool ChecksumValid; - SC_AppData.NextCmdTime[SC_RTP] = 0; SC_AppData.CurrentTime = 1; SC_AppData.NextProcNumber = SC_RTP; SC_OperData.RtsCtrlBlckAddr->RtsNumber = 1; SC_OperData.RtsInfoTblAddr[SC_OperData.RtsCtrlBlckAddr->RtsNumber - 1].RtsStatus = SC_EXECUTING; - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); + SC_AppData.EnableHeaderUpdate = true; /* Execute the function being tested */ UtAssert_VOIDCALL(SC_ProcessRtpCommand()); @@ -704,20 +689,16 @@ void SC_ProcessRtpCommand_Test_Nominal(void) void SC_ProcessRtpCommand_Test_BadSoftwareBusReturn(void) { - bool ChecksumValid; - SC_AppData.NextCmdTime[SC_RTP] = 0; SC_AppData.CurrentTime = 1; SC_AppData.NextProcNumber = SC_RTP; SC_OperData.RtsCtrlBlckAddr->RtsNumber = 1; SC_OperData.RtsInfoTblAddr[SC_OperData.RtsCtrlBlckAddr->RtsNumber - 1].RtsStatus = SC_EXECUTING; - /* Set to return true in order to satisfy the if-statement from which the function is called */ - ChecksumValid = true; + SC_AppData.EnableHeaderUpdate = true; /* Set to return -1 in order to generate error message SC_RTS_DIST_ERR_EID */ UT_SetDeferredRetcode(UT_KEY(CFE_SB_TransmitMsg), 1, -1); - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); /* Execute the function being tested */ UtAssert_VOIDCALL(SC_ProcessRtpCommand()); @@ -745,6 +726,8 @@ void SC_ProcessRtpCommand_Test_BadChecksum(void) SC_OperData.RtsCtrlBlckAddr->RtsNumber = 1; SC_OperData.RtsInfoTblAddr[SC_OperData.RtsCtrlBlckAddr->RtsNumber - 1].RtsStatus = SC_EXECUTING; + SC_AppData.EnableHeaderUpdate = false; + /* Set to return false in order to generate error message SC_RTS_CHKSUM_ERR_EID */ ChecksumValid = false; UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); @@ -1170,18 +1153,15 @@ void SC_ProcessRequest_Test_1HzWakeupRtpExecutionTime(void) void SC_ProcessRequest_Test_1HzWakeupRtpExecutionTimeTooManyCmds(void) { - bool ChecksumValid; - /** ** Test case: SC_1HZ_WAKEUP_MID with a pending RTP command that needs to execute immediately, but too many *commands are being sent at once **/ CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_1HZ_WAKEUP_MID); - UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); + SC_AppData.EnableHeaderUpdate = true; - ChecksumValid = true; - UT_SetDataBuffer(UT_KEY(CFE_MSG_ValidateChecksum), &ChecksumValid, sizeof(ChecksumValid), false); + UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); UT_SetDeferredRetcode(UT_KEY(SC_VerifyCmdLength), 1, true);