Open thnkslprpt opened 1 year ago
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.
Checklist
Describe the contribution
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
).CF_HkCmd()
: UseCFE_SB_TimeStampMsg()
instead ofCFE_MSG_SetMsgTime()
and use theCFE_MSG_PTR
conversion macro - CF was the only app remaining to useCFE_MSG_SetMsgTime()
memset
at 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)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()
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
)Minor changes:
CF_CmdAbandon_Txn()
noted incorrect parameter cannot beNULL
- corrected thistlm_header
toTelemetryHeader
run_status
toRunStatus
cmd_pipe
toCmdPipe
CFE_SB_Buffer_t
types) frommsg
toBufPtr
msg_id
toMessageID
CF_CmdNoop()
changed toCF_NoopCmd()
)CF_CmdReset()
toCF_ResetCountersCmd()
- more clear and specific, and in line with vast majority of cFS incl. cFE (also amended the matching typeCF_ResetCmd_t
toCF_ResetCountersCmd_t
)CF_Init()
toCF_AppInit()
#include
s - cf_verify.h in cf_clist.c andNote: CF does not verify length for non-command MIDs received in the app pipe (
CF_WAKE_UP_MID
andCF_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