Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some minor out-of-family naming/consistency issues in CF could be updated #387

Closed
2 tasks done
thnkslprpt opened this issue Jun 6, 2023 · 0 comments · Fixed by #388
Closed
2 tasks done

Some minor out-of-family naming/consistency issues in CF could be updated #387

thnkslprpt opened this issue Jun 6, 2023 · 0 comments · Fixed by #388

Comments

@thnkslprpt
Copy link
Contributor

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
CF is the only app to still use CFE_MSG_SetMsgTime() to timestamp the HK packet, rather than CFE_SB_TimeStampMsg():

CF/fsw/src/cf_app.c

Lines 47 to 51 in 0f18ae4

void CF_HkCmd(void)
{
CFE_MSG_SetMsgTime(&CF_AppData.HkPacket.tlm_header.Msg, CFE_TIME_GetTime());
/* return value ignored */ CFE_SB_TransmitMsg(&CF_AppData.HkPacket.tlm_header.Msg, true);
}

No memset to zero-out the global data structure upon initialization.

CF checks the return value of the call to CFE_EVS_SendEvent() at the end of a successful initialization - not incorrect but unnecessary. Returns from CFE_EVS_SendEvent() are only checked a handful of times across cFS out of several thousand instances. Also, CF does not check returns from this function anywhere else in the source code.

CF/fsw/src/cf_app.c

Lines 251 to 257 in 0f18ae4

status =
CFE_EVS_SendEvent(CF_EID_INF_INIT, 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);
}

In CF_AppMain(), there is a check for CFE_SUCCESS and for a null pointer of the buffer passed in to CFE_SB_ReceiveBuffer() - this is guaranteed by CFE_SB_ReceiveBuffer() to not be possible and is therefore unnecessary. cFE and almost all other apps do not do this.

CF/fsw/src/cf_app.c

Lines 336 to 343 in 0f18ae4

status = CFE_SB_ReceiveBuffer(&msg, CF_AppData.cmd_pipe, 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)

Some other naming inconsistencies that could be updated to match standard cFS patterns such as common variables and function/command names also exist.

Expected behavior
Align with cFS where appropriate - consistency makes maintenance easier, and improves usability for consumers of cFS and the open-source apps.

Reporter Info
Avi Weiss @thnkslprpt

@dzbaker dzbaker closed this as completed in f928452 Oct 3, 2024
dzbaker added a commit that referenced this issue Oct 3, 2024
…nd-consistency-issues

Fix #387, Update minor out-of-family naming/consistency issues in CF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants