From cb7f988966f01c371328d1786810b5a2b04b8965 Mon Sep 17 00:00:00 2001 From: Avi Weiss Date: Wed, 17 Jan 2024 15:26:48 +1000 Subject: [PATCH] Fix #1360, Limit HK Commands to 1 in pipe at a time --- .github/pull_request_template.md | 2 +- docs/cFE Application Developers Guide.md | 2 +- .../core_private/ut-stubs/src/ut_osprintf_stubs.c | 2 +- modules/es/fsw/src/cfe_es_api.c | 4 ++-- modules/es/fsw/src/cfe_es_start.c | 10 +++++----- modules/es/fsw/src/cfe_es_task.c | 15 ++++++++------- modules/es/ut-coverage/es_UT.c | 2 +- modules/sb/eds/cfe_sb.xml | 2 +- modules/sb/fsw/src/cfe_sb_api.c | 4 ++-- 9 files changed, 22 insertions(+), 21 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 81c10a07e..3d7679b6f 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -10,7 +10,7 @@ A clear and concise description of what the contribution is. **Testing performed** Steps taken to test the contribution: 1. Build steps '...' -1. Execution steps '...' +2. Execution steps '...' **Expected behavior changes** A clear and concise description of how this contribution will change behavior and level of impact. diff --git a/docs/cFE Application Developers Guide.md b/docs/cFE Application Developers Guide.md index 743fe2ef9..ee06f44b9 100644 --- a/docs/cFE Application Developers Guide.md +++ b/docs/cFE Application Developers Guide.md @@ -1251,7 +1251,7 @@ with the allocated memory. The intention is for an Application to use a working copy of the CDS data during Application execution. Periodically, the Application is then responsible for calling CFE_ES_CopyToCDS API to copy the working image -back into the CDS The cFE then computes a data integrity value for the +back into the CDS. The cFE then computes a data integrity value for the block of data and stores it in the allocated CDS block. It should be noted that although the cFE will validate the integrity of the contents of the Application's CDS, the Application is responsible for determining diff --git a/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c b/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c index 7a3f048b5..a5659670b 100644 --- a/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c +++ b/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c @@ -79,7 +79,7 @@ const char *UT_OSP_MESSAGES[] = { [UT_OSP_APP_CREATE] = "%s: AppCreate Error: TaskCreate %s Failed. EC = %ld!\n", [UT_OSP_CREATE_VOLATILE] = "%s: Error Creating Volatile(RAM) Volume. EC = %ld\n", [UT_OSP_MODULE_UNLOAD_FAILED] = "%s: Failed to unload: %s. EC = %ld\n", - [UT_OSP_POR_COMMANDED] = "%s: POWERON RESET called from CFE_ES_ResetCFE (Commanded).\n", + [UT_OSP_POR_COMMANDED] = "POWERON RESET called from %s (Commanded).\n", [UT_OSP_REMOUNT_VOLATILE] = "%s: Error Re-Mounting Volatile(RAM) Volume. EC = %ld\n", [UT_OSP_CORE_APP_EXIT] = "%s: Cannot Exit CORE Application %s\n", [UT_OSP_ES_APP_STARTUP_OPEN] = "%s: Opened ES App Startup file: %s\n", diff --git a/modules/es/fsw/src/cfe_es_api.c b/modules/es/fsw/src/cfe_es_api.c index 6b460ec6b..b5eb167ad 100644 --- a/modules/es/fsw/src/cfe_es_api.c +++ b/modules/es/fsw/src/cfe_es_api.c @@ -95,7 +95,7 @@ CFE_Status_t CFE_ES_ResetCFE(uint32 ResetType) } else { - CFE_ES_WriteToSysLog("%s: PROCESSOR RESET called from CFE_ES_ResetCFE (Commanded).\n", __func__); + CFE_ES_WriteToSysLog("PROCESSOR RESET called from %s (Commanded).\n", __func__); /* ** Update the reset variables @@ -121,7 +121,7 @@ CFE_Status_t CFE_ES_ResetCFE(uint32 ResetType) } else if (ResetType == CFE_PSP_RST_TYPE_POWERON) { - CFE_ES_WriteToSysLog("%s: POWERON RESET called from CFE_ES_ResetCFE (Commanded).\n", __func__); + CFE_ES_WriteToSysLog("POWERON RESET called from %s (Commanded).\n", __func__); /* ** Log the reset in the ER Log. The log will be wiped out, but it's good to have diff --git a/modules/es/fsw/src/cfe_es_start.c b/modules/es/fsw/src/cfe_es_start.c index 91cadfdc6..d5c296d0c 100644 --- a/modules/es/fsw/src/cfe_es_start.c +++ b/modules/es/fsw/src/cfe_es_start.c @@ -154,7 +154,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha /* ** Announce the startup */ - CFE_ES_WriteToSysLog("%s: CFE_ES_Main in EARLY_INIT state\n", __func__); + CFE_ES_WriteToSysLog("%s: In EARLY_INIT state\n", __func__); /* ** Create and Mount the filesystems needed @@ -177,7 +177,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha /* ** Indicate that the CFE core is now starting up / going multi-threaded */ - CFE_ES_WriteToSysLog("%s: CFE_ES_Main entering CORE_STARTUP state\n", __func__); + CFE_ES_WriteToSysLog("%s: Entering CORE_STARTUP state\n", __func__); CFE_ES_Global.SystemState = CFE_ES_SystemState_CORE_STARTUP; /* @@ -188,7 +188,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha /* ** Indicate that the CFE core is ready */ - CFE_ES_WriteToSysLog("%s: CFE_ES_Main entering CORE_READY state\n", __func__); + CFE_ES_WriteToSysLog("%s: Entering CORE_READY state\n", __func__); CFE_ES_Global.SystemState = CFE_ES_SystemState_CORE_READY; /* @@ -210,7 +210,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha CFE_ES_WriteToSysLog("%s: Startup Sync failed - Applications may not have all initialized\n", __func__); } - CFE_ES_WriteToSysLog("%s: CFE_ES_Main entering APPS_INIT state\n", __func__); + CFE_ES_WriteToSysLog("%s: Entering APPS_INIT state\n", __func__); CFE_ES_Global.SystemState = CFE_ES_SystemState_APPS_INIT; /* @@ -228,7 +228,7 @@ void CFE_ES_Main(uint32 StartType, uint32 StartSubtype, uint32 ModeId, const cha /* ** Startup is fully complete */ - CFE_ES_WriteToSysLog("%s: CFE_ES_Main entering OPERATIONAL state\n", __func__); + CFE_ES_WriteToSysLog("%s: Entering OPERATIONAL state\n", __func__); CFE_ES_Global.SystemState = CFE_ES_SystemState_OPERATIONAL; } diff --git a/modules/es/fsw/src/cfe_es_task.c b/modules/es/fsw/src/cfe_es_task.c index a862ffabf..7deeca70f 100644 --- a/modules/es/fsw/src/cfe_es_task.c +++ b/modules/es/fsw/src/cfe_es_task.c @@ -335,9 +335,10 @@ int32 CFE_ES_TaskInit(void) } /* - ** Subscribe to Housekeeping request commands + ** Subscribe to Housekeeping request commands (limit to 1 in the pipe at a time) */ - Status = CFE_SB_Subscribe(CFE_SB_ValueToMsgId(CFE_ES_SEND_HK_MID), CFE_ES_Global.TaskData.CmdPipe); + Status = CFE_SB_SubscribeEx(CFE_SB_ValueToMsgId(CFE_ES_SEND_HK_MID), CFE_ES_Global.TaskData.CmdPipe, + CFE_SB_DEFAULT_QOS, 1); if (Status != CFE_SUCCESS) { CFE_ES_WriteToSysLog("%s: Cannot Subscribe to HK packet, RC = 0x%08X\n", __func__, (unsigned int)Status); @@ -655,32 +656,32 @@ int32 CFE_ES_StartAppCmd(const CFE_ES_StartAppCmd_t *data) { CFE_ES_Global.TaskData.CommandErrorCounter++; CFE_EVS_SendEvent(CFE_ES_START_INVALID_FILENAME_ERR_EID, CFE_EVS_EventType_ERROR, - "CFE_ES_StartAppCmd: invalid filename, status=%lx", (unsigned long)Result); + "%s: invalid filename, status=%lx", __func__, (unsigned long)Result); } else if (AppEntryLen <= 0) { CFE_ES_Global.TaskData.CommandErrorCounter++; CFE_EVS_SendEvent(CFE_ES_START_INVALID_ENTRY_POINT_ERR_EID, CFE_EVS_EventType_ERROR, - "CFE_ES_StartAppCmd: App Entry Point is empty."); + "%s: App Entry Point is empty.", __func__); } else if (AppNameLen <= 0) { CFE_ES_Global.TaskData.CommandErrorCounter++; CFE_EVS_SendEvent(CFE_ES_START_NULL_APP_NAME_ERR_EID, CFE_EVS_EventType_ERROR, - "CFE_ES_StartAppCmd: App Name is empty."); + "%s: App Name is empty.", __func__); } else if (cmd->Priority > OS_MAX_PRIORITY) { CFE_ES_Global.TaskData.CommandErrorCounter++; CFE_EVS_SendEvent(CFE_ES_START_PRIORITY_ERR_EID, CFE_EVS_EventType_ERROR, - "CFE_ES_StartAppCmd: Priority is too large: %d.", (int)cmd->Priority); + "%s: Priority is too large: %d.", __func__, (int)cmd->Priority); } else if ((cmd->ExceptionAction != CFE_ES_ExceptionAction_RESTART_APP) && (cmd->ExceptionAction != CFE_ES_ExceptionAction_PROC_RESTART)) { CFE_ES_Global.TaskData.CommandErrorCounter++; CFE_EVS_SendEvent(CFE_ES_START_EXC_ACTION_ERR_EID, CFE_EVS_EventType_ERROR, - "CFE_ES_StartAppCmd: Invalid Exception Action: %d.", (int)cmd->ExceptionAction); + "%s: Invalid Exception Action: %d.", __func__, (int)cmd->ExceptionAction); } else { diff --git a/modules/es/ut-coverage/es_UT.c b/modules/es/ut-coverage/es_UT.c index 215a818b3..674e116b7 100644 --- a/modules/es/ut-coverage/es_UT.c +++ b/modules/es/ut-coverage/es_UT.c @@ -2573,7 +2573,7 @@ void TestTask(void) /* Test task main process loop with a ground command subscribe failure */ ES_ResetUnitTest(); - UT_SetDeferredRetcode(UT_KEY(CFE_SB_Subscribe), 2, -4); + UT_SetDeferredRetcode(UT_KEY(CFE_SB_SubscribeEx), 1, -4); UtAssert_INT32_EQ(CFE_ES_TaskInit(), -4); /* Test task main process loop with an init event send failure */ diff --git a/modules/sb/eds/cfe_sb.xml b/modules/sb/eds/cfe_sb.xml index 02284d5ea..f35349d14 100644 --- a/modules/sb/eds/cfe_sb.xml +++ b/modules/sb/eds/cfe_sb.xml @@ -329,7 +329,7 @@ \cfetlmmnemonic \SB_SMPSBBIU - + \cfetlmmnemonic \SB_SMMPDALW diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index bc4280ece..0de1c955a 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -1897,7 +1897,7 @@ CFE_Status_t CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t Pipe if (CFE_SB_PipeDescIsMatch(PipeDscPtr, PipeId)) { /* - ** Load the pipe tables 'CurrentBuff' with the buffer descriptor + ** Load the pipe table's 'LastBuffer' with the buffer descriptor ** ptr corresponding to the message just read. This is done so that ** the buffer can be released on the next receive call for this pipe. ** @@ -1907,7 +1907,7 @@ CFE_Status_t CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t Pipe PipeDscPtr->LastBuffer = BufDscPtr; /* - * Also set the Receivers pointer to the address of the actual message + * Also set the Receiver's pointer to the address of the actual message * (currently this is "borrowing" the ref above, not its own ref) */ *BufPtr = &BufDscPtr->Content;