Skip to content

Commit

Permalink
Fix #387, Update minor out-of-family naming/consistency issues in CF
Browse files Browse the repository at this point in the history
  • Loading branch information
thnkslprpt committed Sep 27, 2024
1 parent 7eac600 commit f928452
Show file tree
Hide file tree
Showing 26 changed files with 487 additions and 521 deletions.
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion config/default_cf_internal_cfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
* @par Limits:
* The minimum size of this parameter is 1
* The maximum size dictated by cFE platform configuration
* parameter is CFE_SB_MAX_PIPE_DEPTH
* parameter is OS_QUEUE_MAX_DEPTH
*/
#define CF_PIPE_DEPTH (32)

Expand Down
4 changes: 2 additions & 2 deletions config/default_cf_msgstruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ typedef struct CF_DisableEngineCmd
*
* For command details see #CF_RESET_CC
*/
typedef struct CF_ResetCmd
typedef struct CF_ResetCountersCmd
{
CFE_MSG_CommandHeader_t CommandHeader; /**< \brief Command header */
CF_UnionArgs_Payload_t Payload; /**< \brief Generic command arguments */
} CF_ResetCmd_t;
} CF_ResetCountersCmd_t;

/**
* \brief Freeze command structure
Expand Down
13 changes: 12 additions & 1 deletion fsw/inc/cf_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
*
* Failure from create pipe call during engine channel initialization
*/
#define CF_CR_PIPE_ERR_EID (31)
#define CF_CR_CHANNEL_PIPE_ERR_EID (31)

/**
* \brief CF Channel Message Subscription Failed Event ID
Expand Down Expand Up @@ -212,6 +212,17 @@
*/
#define CF_INIT_OUTGOING_SIZE_ERR_EID (35)

/**
* \brief CF Create SB Command Pipe at Initialization Failed Event ID
*
* \par Type: ERROR
*
* \par Cause:
*
* Failure from create command pipe call during application initialization
*/
#define CF_CR_PIPE_ERR_EID (36)

/**************************************************************************
* CF_PDU event IDs - Protocol data unit
*/
Expand Down
54 changes: 23 additions & 31 deletions fsw/src/cf_app.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,23 @@ void CF_CheckTables(void)
{
CFE_EVS_SendEvent(CF_INIT_TBL_CHECK_REL_ERR_EID, CFE_EVS_EventType_ERROR,
"CF: error in CFE_TBL_ReleaseAddress (check), returned 0x%08lx", (unsigned long)status);
CF_AppData.run_status = CFE_ES_RunStatus_APP_ERROR;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_ERROR;
}

status = CFE_TBL_Manage(CF_AppData.config_handle);
if (status < CFE_SUCCESS)
{
CFE_EVS_SendEvent(CF_INIT_TBL_CHECK_MAN_ERR_EID, CFE_EVS_EventType_ERROR,
"CF: error in CFE_TBL_Manage (check), returned 0x%08lx", (unsigned long)status);
CF_AppData.run_status = CFE_ES_RunStatus_APP_ERROR;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_ERROR;
}

status = CFE_TBL_GetAddress((void *)&CF_AppData.config_table, CF_AppData.config_handle);
if (status < CFE_SUCCESS)
{
CFE_EVS_SendEvent(CF_INIT_TBL_CHECK_GA_ERR_EID, CFE_EVS_EventType_ERROR,
"CF: failed to get table address (check), returned 0x%08lx", (unsigned long)status);
CF_AppData.run_status = CFE_ES_RunStatus_APP_ERROR;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_ERROR;
}
}
}
Expand Down Expand Up @@ -186,13 +186,16 @@ CFE_Status_t CF_TableInit(void)
* See description in cf_app.h for argument/return detail
*
*-----------------------------------------------------------------*/
CFE_Status_t CF_Init(void)
CFE_Status_t CF_AppInit(void)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
CFE_Status_t status;
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;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_RUN;

/* Zero-out global data structure */
memset(&CF_AppData, 0, sizeof(CF_AppData));

This comment has been minimized.

Copy link
@RR-HSz

RR-HSz Oct 29, 2024

Hi, this memset clears the RunStatus, that you have set in the previous line.
This will result in RunStatus = CFE_ES_RunStatus_UNDEFINED. CF_AppInit will be successful, but in CF_AppMain the CFE_ES_RunLoop will return false due to invalid status, and CF app will exit.
I suggest setting the RunStatus after doing the zero-out of the structure.


CFE_MSG_Init(CFE_MSG_PTR(CF_AppData.hk.TelemetryHeader), CFE_SB_ValueToMsgId(CF_HK_TLM_MID), sizeof(CF_AppData.hk));

Expand All @@ -203,19 +206,19 @@ CFE_Status_t CF_Init(void)
}
else
{
status = CFE_SB_CreatePipe(&CF_AppData.cmd_pipe, CF_PIPE_DEPTH, CF_PIPE_NAME);
status = CFE_SB_CreatePipe(&CF_AppData.CmdPipe, CF_PIPE_DEPTH, CF_PIPE_NAME);
if (status != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("CF app: error creating pipe %s, returned 0x%08lx", CF_PIPE_NAME,
(unsigned long)status);
CFE_EVS_SendEvent(CF_CR_PIPE_ERR_EID, CFE_EVS_EventType_ERROR,
"CF app: error creating pipe %s, returned 0x%08lx", CF_PIPE_NAME, (unsigned long)status);
}
}

if (status == CFE_SUCCESS)
{
for (i = 0; i < (sizeof(MID_VALUES) / sizeof(MID_VALUES[0])); ++i)
{
status = CFE_SB_Subscribe(CFE_SB_ValueToMsgId(MID_VALUES[i]), CF_AppData.cmd_pipe);
status = CFE_SB_Subscribe(CFE_SB_ValueToMsgId(MID_VALUES[i]), CF_AppData.CmdPipe);
if (status != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("CF app: failed to subscribe to MID 0x%04lx, returned 0x%08lx",
Expand All @@ -237,13 +240,8 @@ CFE_Status_t CF_Init(void)

if (status == CFE_SUCCESS)
{
status =
CFE_EVS_SendEvent(CF_INIT_INF_EID, CFE_EVS_EventType_INFORMATION, "CF Initialized. Version %d.%d.%d.%d",
CF_MAJOR_VERSION, CF_MINOR_VERSION, CF_REVISION, CF_MISSION_REV);
if (status != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("CF: error sending init event, returned 0x%08lx", (unsigned long)status);
}
CFE_EVS_SendEvent(CF_INIT_INF_EID, CFE_EVS_EventType_INFORMATION, "CF Initialized. Version %d.%d.%d.%d",
CF_MAJOR_VERSION, CF_MINOR_VERSION, CF_REVISION, CF_MISSION_REV);
}

return status;
Expand All @@ -258,38 +256,32 @@ CFE_Status_t CF_Init(void)
void CF_AppMain(void)
{
int32 status;
CFE_SB_Buffer_t *msg;
CFE_SB_Buffer_t *BufPtr = NULL;

CFE_ES_PerfLogEntry(CF_PERF_ID_APPMAIN);

status = CF_Init();
status = CF_AppInit();
if (status != CFE_SUCCESS)
{
CF_AppData.run_status = CFE_ES_RunStatus_APP_ERROR;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_ERROR;
}

msg = NULL;

while (CFE_ES_RunLoop(&CF_AppData.run_status))
while (CFE_ES_RunLoop(&CF_AppData.RunStatus))

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
CFE_ES_PerfLogExit(CF_PERF_ID_APPMAIN);

status = CFE_SB_ReceiveBuffer(&msg, CF_AppData.cmd_pipe, CF_RCVMSG_TIMEOUT);
status = CFE_SB_ReceiveBuffer(&BufPtr, CF_AppData.CmdPipe, CF_RCVMSG_TIMEOUT);
CFE_ES_PerfLogEntry(CF_PERF_ID_APPMAIN);

/*
* note that CFE_SB_ReceiveBuffer() guarantees that a CFE_SUCCESS status is accompanied by
* a valid (non-NULL) output message pointer. However the unit test can force this condition.
*/
if (status == CFE_SUCCESS && msg != NULL)
if (status == CFE_SUCCESS)
{
CF_AppPipe(msg);
CF_AppPipe(BufPtr);
}
else if (status != CFE_SB_TIME_OUT && status != CFE_SB_NO_MESSAGE)
{
CFE_EVS_SendEvent(CF_INIT_MSG_RECV_ERR_EID, CFE_EVS_EventType_ERROR,
"CF: exiting due to CFE_SB_ReceiveBuffer error 0x%08lx", (unsigned long)status);
CF_AppData.run_status = CFE_ES_RunStatus_APP_ERROR;
CF_AppData.RunStatus = CFE_ES_RunStatus_APP_ERROR;
}
else
{
Expand All @@ -298,5 +290,5 @@ void CF_AppMain(void)
}

Check warning

Code scanning / CodeQL

Unbounded loop Warning

This loop does not have a fixed bound.

CFE_ES_PerfLogExit(CF_PERF_ID_APPMAIN);
CFE_ES_ExitApp(CF_AppData.run_status);
CFE_ES_ExitApp(CF_AppData.RunStatus);
}
6 changes: 3 additions & 3 deletions fsw/src/cf_app.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ typedef struct
{
CF_HkPacket_t hk;

uint32 run_status;
uint32 RunStatus;

CFE_SB_PipeId_t cmd_pipe;
CFE_SB_PipeId_t CmdPipe;

CFE_TBL_Handle_t config_handle;
CF_ConfigTable_t *config_table;
Expand Down Expand Up @@ -163,7 +163,7 @@ CFE_Status_t CF_TableInit(void);
* @retval Returns anything else on error.
*
*/
CFE_Status_t CF_Init(void);
CFE_Status_t CF_AppInit(void);

/************************************************************************/
/** @brief CF app entry point
Expand Down
2 changes: 1 addition & 1 deletion fsw/src/cf_cfdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ CFE_Status_t CF_CFDP_InitEngine(void)
nbuf);
if (ret != CFE_SUCCESS)
{
CFE_EVS_SendEvent(CF_CR_PIPE_ERR_EID, CFE_EVS_EventType_ERROR,
CFE_EVS_SendEvent(CF_CR_CHANNEL_PIPE_ERR_EID, CFE_EVS_EventType_ERROR,
"CF: failed to create pipe %s, returned 0x%08lx", nbuf, (unsigned long)ret);
break;
}
Expand Down
1 change: 0 additions & 1 deletion fsw/src/cf_clist.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* This file is intended to be a generic class that can be used in other apps.
*/

#include "cf_verify.h"
#include "cf_clist.h"
#include "cf_assert.h"

Expand Down
20 changes: 10 additions & 10 deletions fsw/src/cf_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ CFE_Status_t CF_NoopCmd(const CF_NoopCmd_t *msg)
* See description in cf_cmd.h for argument/return detail
*
*-----------------------------------------------------------------*/
CFE_Status_t CF_ResetCmd(const CF_ResetCmd_t *msg)
CFE_Status_t CF_ResetCountersCmd(const CF_ResetCountersCmd_t *msg)

Check notice

Code scanning / CodeQL

Function too long Note

CF_ResetCountersCmd has too many lines (63, while 60 are allowed).

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
const CF_UnionArgs_Payload_t *data = &msg->Payload;
static const char * names[5] = {"all", "cmd", "fault", "up", "down"};
Expand Down Expand Up @@ -488,7 +488,7 @@ CFE_Status_t CF_ResumeCmd(const CF_ResumeCmd_t *msg)
* See description in cf_cmd.h for argument/return detail
*
*-----------------------------------------------------------------*/
void CF_CmdCancel_Txn(CF_Transaction_t *txn, void *ignored)
void CF_Cancel_TxnCmd(CF_Transaction_t *txn, void *ignored)
{
CF_CFDP_CancelTransaction(txn);
}
Expand All @@ -501,7 +501,7 @@ void CF_CmdCancel_Txn(CF_Transaction_t *txn, void *ignored)
*-----------------------------------------------------------------*/
CFE_Status_t CF_CancelCmd(const CF_CancelCmd_t *msg)
{
if (CF_TsnChanAction(&msg->Payload, "cancel", CF_CmdCancel_Txn, NULL) > 0)
if (CF_TsnChanAction(&msg->Payload, "cancel", CF_Cancel_TxnCmd, NULL) > 0)

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
CFE_EVS_SendEvent(CF_CMD_CANCEL_INF_EID, CFE_EVS_EventType_INFORMATION,
"CF: cancel transaction successfully initiated");
Expand All @@ -523,7 +523,7 @@ CFE_Status_t CF_CancelCmd(const CF_CancelCmd_t *msg)
* See description in cf_cmd.h for argument/return detail
*
*-----------------------------------------------------------------*/
void CF_CmdAbandon_Txn(CF_Transaction_t *txn, void *ignored)
void CF_Abandon_TxnCmd(CF_Transaction_t *txn, void *ignored)
{
CF_CFDP_ResetTransaction(txn, 0);
}
Expand All @@ -536,7 +536,7 @@ void CF_CmdAbandon_Txn(CF_Transaction_t *txn, void *ignored)
*-----------------------------------------------------------------*/
CFE_Status_t CF_AbandonCmd(const CF_AbandonCmd_t *msg)
{
if (CF_TsnChanAction(&msg->Payload, "abandon", CF_CmdAbandon_Txn, NULL) > 0)
if (CF_TsnChanAction(&msg->Payload, "abandon", CF_Abandon_TxnCmd, NULL) > 0)

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
CFE_EVS_SendEvent(CF_CMD_ABANDON_INF_EID, CFE_EVS_EventType_INFORMATION, "CF: abandon successful");
++CF_AppData.hk.Payload.counters.cmd;
Expand Down Expand Up @@ -946,7 +946,7 @@ CFE_Status_t CF_WriteQueueCmd(const CF_WriteQueueCmd_t *msg)
* See description in cf_cmd.h for argument/return detail
*
*-----------------------------------------------------------------*/
CF_ChanAction_Status_t CF_CmdValidateChunkSize(CF_ChunkSize_t val, uint8 chan_num /* ignored */)
CF_ChanAction_Status_t CF_ValidateChunkSizeCmd(CF_ChunkSize_t val, uint8 chan_num /* ignored */)
{
CF_ChanAction_Status_t ret = CF_ChanAction_Status_SUCCESS;
if (val > sizeof(CF_CFDP_PduFileDataContent_t))
Expand All @@ -962,7 +962,7 @@ CF_ChanAction_Status_t CF_CmdValidateChunkSize(CF_ChunkSize_t val, uint8 chan_nu
* See description in cf_cmd.h for argument/return detail
*
*-----------------------------------------------------------------*/
CF_ChanAction_Status_t CF_CmdValidateMaxOutgoing(uint32 val, uint8 chan_num)
CF_ChanAction_Status_t CF_ValidateMaxOutgoingCmd(uint32 val, uint8 chan_num)
{
CF_ChanAction_Status_t ret = CF_ChanAction_Status_SUCCESS;

Expand Down Expand Up @@ -1022,7 +1022,7 @@ void CF_GetSetParamCmd(uint8 is_set, CF_GetSet_ValueID_t param_id, uint32 value,
case CF_GetSet_ValueID_outgoing_file_chunk_size:
item.ptr = &config->outgoing_file_chunk_size;
item.size = sizeof(config->outgoing_file_chunk_size);
item.fn = CF_CmdValidateChunkSize;
item.fn = CF_ValidateChunkSizeCmd;
break;
case CF_GetSet_ValueID_ack_limit:
item.ptr = &config->chan[chan_num].ack_limit;
Expand All @@ -1039,7 +1039,7 @@ void CF_GetSetParamCmd(uint8 is_set, CF_GetSet_ValueID_t param_id, uint32 value,
case CF_GetSet_ValueID_chan_max_outgoing_messages_per_wakeup:
item.ptr = &config->chan[chan_num].max_outgoing_messages_per_wakeup;
item.size = sizeof(config->chan[chan_num].max_outgoing_messages_per_wakeup);
item.fn = CF_CmdValidateMaxOutgoing;
item.fn = CF_ValidateMaxOutgoingCmd;
break;
default:
break;
Expand Down Expand Up @@ -1224,7 +1224,7 @@ CFE_Status_t CF_DisableEngineCmd(const CF_DisableEngineCmd_t *msg)
*-----------------------------------------------------------------*/
CFE_Status_t CF_SendHkCmd(const CF_SendHkCmd_t *msg)
{
CFE_MSG_SetMsgTime(CFE_MSG_PTR(CF_AppData.hk.TelemetryHeader), CFE_TIME_GetTime());
CFE_SB_TimeStampMsg(CFE_MSG_PTR(CF_AppData.hk.TelemetryHeader));
/* return value ignored */ CFE_SB_TransmitMsg(CFE_MSG_PTR(CF_AppData.hk.TelemetryHeader), true);

/* This is also used to check tables */
Expand Down
12 changes: 6 additions & 6 deletions fsw/src/cf_cmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ CFE_Status_t CF_NoopCmd(const CF_NoopCmd_t *msg);
*
* @param msg Pointer to command message
*/
CFE_Status_t CF_ResetCmd(const CF_ResetCmd_t *msg);
CFE_Status_t CF_ResetCountersCmd(const CF_ResetCountersCmd_t *msg);

/************************************************************************/
/** @brief Ground command to start a file transfer.
Expand Down Expand Up @@ -337,7 +337,7 @@ CFE_Status_t CF_ResumeCmd(const CF_ResumeCmd_t *msg);
* @param txn Pointer to transaction object
* @param ignored Not used by this function
*/
void CF_CmdCancel_Txn(CF_Transaction_t *txn, void *ignored);
void CF_Cancel_TxnCmd(CF_Transaction_t *txn, void *ignored);

/************************************************************************/
/** @brief Handle a cancel ground command.
Expand All @@ -355,12 +355,12 @@ CFE_Status_t CF_CancelCmd(const CF_CancelCmd_t *msg);
* This helper function is used with CF_TsnChanAction() to abandon matched transactions
*
* @par Assumptions, External Events, and Notes:
* msg must not be NULL.
* txn must not be NULL.
*
* @param txn Pointer to transaction object
* @param ignored Not used by this function
*/
void CF_CmdAbandon_Txn(CF_Transaction_t *txn, void *ignored);
void CF_Abandon_TxnCmd(CF_Transaction_t *txn, void *ignored);

/************************************************************************/
/** @brief Handle an abandon ground command.
Expand Down Expand Up @@ -524,7 +524,7 @@ CFE_Status_t CF_WriteQueueCmd(const CF_WriteQueueCmd_t *msg);
* @retval CF_ChanAction_Status_ERROR if failed (val is greater than max PDU)
*
*/
CF_ChanAction_Status_t CF_CmdValidateChunkSize(CF_ChunkSize_t val, uint8 chan_num);
CF_ChanAction_Status_t CF_ValidateChunkSizeCmd(CF_ChunkSize_t val, uint8 chan_num);

/************************************************************************/
/** @brief Checks if the value is within allowable range as outgoing packets per wakeup
Expand All @@ -540,7 +540,7 @@ CF_ChanAction_Status_t CF_CmdValidateChunkSize(CF_ChunkSize_t val, uint8 chan_nu
* @retval CF_ChanAction_Status_ERROR if failed (val is not allowed)
*
*/
CF_ChanAction_Status_t CF_CmdValidateMaxOutgoing(uint32 val, uint8 chan_num);
CF_ChanAction_Status_t CF_ValidateMaxOutgoingCmd(uint32 val, uint8 chan_num);

/************************************************************************/
/** @brief Perform a configuration get/set operation.
Expand Down
2 changes: 1 addition & 1 deletion fsw/src/cf_codec.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static const CF_Codec_BitField_t CF_CFDP_PduMd_CHECKSUM_TYPE = CF_INIT_FIELD
static const CF_Codec_BitField_t CF_CFDP_PduFileData_RECORD_CONTINUATION_STATE = CF_INIT_FIELD(2, 6);
static const CF_Codec_BitField_t CF_CFDP_PduFileData_SEGMENT_METADATA_LENGTH = CF_INIT_FIELD(6, 0);

/* NOTE: get/set will handle endianess */
/* NOTE: get/set will handle endianness */
/*
* ALSO NOTE: These store/set inline functions/macros are used with
* literal integers as well as variables. So they operate on value, where
Expand Down
Loading

0 comments on commit f928452

Please sign in to comment.