Fix #387, Update minor out-of-family naming/consistency issues in CF
Checklist
- [x] I reviewed the Contributing Guide.
- [x] I signed and emailed the appropriate Contributor License Agreement to [email protected] and copied [email protected].
Describe the contribution
- Fixes #387
- Add event for
CreatePipe()failure during initialization (and created a new matching EID:CF_CR_PIPE_ERR_EID) + clarified different EIDs for pipe creation errors during initialization of the app, and separate EID for errors during CF Channel pipe creation (CF_CR_CHANNEL_PIPE_ERR_EID). - In
CF_HkCmd(): UseCFE_SB_TimeStampMsg()instead ofCFE_MSG_SetMsgTime()and use theCFE_MSG_PTRconversion macro - CF was the only app remaining to useCFE_MSG_SetMsgTime() - Add
memsetat the beginning of the initialization routine to zero-out the global data structure (defensive programming, and for consistency - almost all of the cFS modules/apps do this) - Remove check of the return value of
CFE_EVS_SendEvent()reporting initialization success - this is very out-of-family for cFS and also inconsistent - CF does not check the return value of any other calls toCFE_EVS_SendEvent() - Remove null check in
CF_AppMain()after successful call toCFE_SB_ReceiveBuffer()- out-of-family with cFS and also redundant (CFE_SB_ReceiveBuffer()with a successful return guarantees the returned pointer to be non-NULL)
- Add event for
Minor changes:
- Comment in
CF_CmdAbandon_Txn()noted incorrect parameter cannot beNULL- corrected this - Rename
tlm_headertoTelemetryHeader - Rename
run_statustoRunStatus - Rename
cmd_pipetoCmdPipe - Rename Software Bus command pipe message pointer variables (
CFE_SB_Buffer_ttypes) frommsgtoBufPtr - Rename
msg_idtoMessageID - Move 'Cmd' to end of command function names (e.g.
CF_CmdNoop()changed toCF_NoopCmd()) - Rename
CF_CmdReset()toCF_ResetCountersCmd()- more clear and specific, and in line with vast majority of cFS incl. cFE (also amended the matching typeCF_ResetCmd_ttoCF_ResetCountersCmd_t) - Rename
CF_Init()toCF_AppInit() - Removed a couple of unnessesasary header
#includes - cf_verify.h in cf_clist.c and
Note: CF does not verify length for non-command MIDs received in the app pipe (CF_WAKE_UP_MID and CF_SEND_HK_MID) - it would be worthwhile to rectify this at some point.
cFE and the other apps are generally inconsistent on this - some don't check non-command MIDs, some use a single VerifyLength function to check all MIDs arriving, some use separate VerifyLength functions for command and non-command MIDs...
Testing performed GitHub CI actions all passing successfully (incl. Build + Run, Unit/Functional Tests etc.).
Expected behavior changes Minor changes as noted above, no significant changes to behavior.
Aligning aberrant naming to the predominant patterns in cFS improves usability and eases future maintenance.
Contributor Info Avi Weiss @thnkslprpt
For length check and command handling inconsistencies, hopefully https://github.com/nasa/cFE/issues/2038 will eventually resolve this across all of cFS.
Concur with changes...
@chillfig Justin - slight change since your approval due to a merge conflict resolution. There are now 2 separate EIDs for the different pipe creation error locations: CF_CR_CHANNEL_PIPE_ERR_EID and CF_CR_PIPE_ERR_EID
I like this changeset, but it overlaps with other recent PRs. We should rebase it, but only after the other pending items.
No worries Joe @jphickey - I'll try to keep it up-to-date. Feel free to merge it whenever it suits.
Hi @thnkslprpt, apologies about the delay with this PR. I'd like to get it into the next round of PR reviews. Would you be able to resolve the merge conflict/unit test workflow failure?
Hi @thnkslprpt, apologies about the delay with this PR. I'd like to get it into the next round of PR reviews. Would you be able to resolve the merge conflict/unit test workflow failure?
I think it's good to go now @dzbaker