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

Documentation on CFE_SB_GetMsgTime()/SetMsgTime() needs updated. They do not set/get timestamps for commands (CFE_SB_CMD_HDR_SIZE). #499

Closed
pcooksey opened this issue Jan 28, 2020 · 2 comments · Fixed by #545 or #563
Assignees
Labels
Milestone

Comments

@pcooksey
Copy link

pcooksey commented Jan 28, 2020

Is your feature request related to a problem? Please describe.
Documentation does not clearly state that CFE_SB_GetMsgTime() and CFE_SB_SetMsgTime() are only useful if the message header is big enough. Basically, the CFE_SB_CMD_HDR_SIZE does not have this field and so it would return 0.

Describe the solution you'd like
Amend the documentation to let others know that those functions are not used when the cmdHeader[CFE_SB_CMD_HDR_SIZE]

Describe alternatives you've considered
Actually it would probably be better if there was some kind of assert or check to inform the programmer that the function probably shouldn't be used since returning 0 is really returning nothing.
Maybe?

CompileTimeAssert(sizeof(CFE_ES_ResetDataPtr->ERLog[0].Context) == CFE_PLATFORM_ES_ER_LOG_MAX_CONTEXT_SIZE, CfeEsErLogContextSizeError);

from
https://github.com/nasa/osal/blob/d0f1a397fa3559d3ac0c923b3c2ac7ed5004bd5a/src/os/inc/common_types.h#L44

Additional context
Documentation in cFS Application Developers Guide.doc pg. 57:

Before sending an SB Message to the SB, the Application can update the SB Message Header. The most common update is to put the current time in the SB Message. This is accomplished with one of two SB API functions. The most commonly used function would be CFE_SB_TimeStampMsg(). This API would insert the current time, in the mission defined format with the mission defined epoch, into the SB Message Header. The other SB API that can modify the SB Message Header time is CFE_SB_SetMsgTime(). This API call sets the time in the SB Message Header to the time specified during the call. This is useful when the Application wishes to time tag a series of SB Messages with the same time.

Other fields of the SB Message Header can be modified by an Application prior to sending the SB Message. These fields, and the associated APIs, are listed in the following table:

I didn't see anywhere that it said if the message is a Command Msg, i.e., CFE_SB_CMD_HDR_SIZE then it will not have a timestamp and therefore these functions return 0.

Code:

/******************************************************************************
** Function: CFE_SB_GetMsgTime()
**
** Purpose:
** Get the time field from a message.
**
** Arguments:
** MsgPtr - Pointer to a CFE_SB_Msg_t
**
** Return:
** Time field from message or
** Time value of zero for msgs that do not have a Time field in header
*/
CFE_TIME_SysTime_t CFE_SB_GetMsgTime(CFE_SB_MsgPtr_t MsgPtr)
{
CFE_TIME_SysTime_t TimeFromMsg;
uint32 LocalSecs32 = 0;
uint32 LocalSubs32 = 0;
#ifdef MESSAGE_FORMAT_IS_CCSDS
#if (CFE_MISSION_SB_PACKET_TIME_FORMAT == CFE_MISSION_SB_TIME_32_16_SUBS)
uint16 LocalSubs16;
#endif
CFE_SB_TlmHdr_t *TlmHdrPtr;
/* if msg type is a command or msg has no secondary hdr, time = 0 */
if ((CCSDS_RD_TYPE(MsgPtr->Hdr) != CCSDS_CMD) && (CCSDS_RD_SHDR(MsgPtr->Hdr) != 0)) {

Just went down a rabbit hole trying to figure out why the message header sizes didn't match in my test cases.

Requester Info
Philip Cooksey at NASA Ames

@skliper
Copy link
Contributor

skliper commented Jan 28, 2020

Please utilize the issue templates, more likely to get action on a ticket if all the information is provided.

@pcooksey
Copy link
Author

Updated to follow the feature template.

@ejtimmon ejtimmon self-assigned this Feb 19, 2020
skliper added a commit to skliper/cFE that referenced this issue Mar 13, 2020
Partial fix (see also nasa#545), comments/documentation only
@astrogeco astrogeco linked a pull request Mar 18, 2020 that will close this issue
lbleier-GSFC pushed a commit to lbleier-GSFC/cFE that referenced this issue Apr 6, 2020
Partial fix (see also nasa#545), comments/documentation only
@skliper skliper added this to the 6.8.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment