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.

Sign up for GitHub

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

Jump to bottom

Fix #62, Apply message alignment pattern #63

Merged
merged 2 commits into from
Dec 1, 2020
Merged

Fix #62, Apply message alignment pattern #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6d740a
Fix #62, Apply message alignment pattern
skliper Nov 16, 2020
3ed636d
Fix #62, Removed hwords and replaced deprecated elements
skliper Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 53 additions & 51 deletions fsw/src/ci_lab_app.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,18 @@
*/
typedef union
{
CFE_MSG_Message_t MsgHdr;
CFE_MSG_Message_t Msg;
uint8 bytes[CI_LAB_MAX_INGEST];
uint16 hwords[2];
} CI_LAB_IngestBuffer_t;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "hwords" member here really should not be needed. AFAIK it served two purposes - to align the buffer to 16 bits (not needed if using CFE_MSG_Message_t) and in the event the message didn't validate it would actually report the value of the first two words in an error message. But that message is fundamentally flawed because if the length check doesn't work the values probably are undefined/uninitialized and thereby meaningless.


typedef union
{
CFE_MSG_Message_t MsgHdr;
CI_LAB_HkTlm_t HkTlm;
} CI_LAB_HkTlm_Buffer_t;

typedef struct
{
bool SocketConnected;
CFE_SB_PipeId_t CommandPipe;
CFE_MSG_Message_t *MsgPtr;
osal_id_t SocketID;
OS_SockAddr_t SocketAddress;
bool SocketConnected;
CFE_SB_PipeId_t CommandPipe;
osal_id_t SocketID;
OS_SockAddr_t SocketAddress;

CI_LAB_HkTlm_Buffer_t HkBuffer;
CI_LAB_HkTlm_t HkTlm;
CI_LAB_IngestBuffer_t IngestBuffer;
} CI_LAB_GlobalData_t;

Expand All @@ -85,9 +77,11 @@ static CFE_EVS_BinFilter_t CI_LAB_EventFilters[] =
* to a structure type which matches the message, and return an int32
* where CFE_SUCCESS (0) indicates successful handling of the message.
*/
int32 CI_LAB_Noop(const CI_LAB_Noop_t *data);
int32 CI_LAB_ResetCounters(const CI_LAB_ResetCounters_t *data);
int32 CI_LAB_ReportHousekeeping(const CFE_SB_CmdHdr_t *data);
int32 CI_LAB_Noop(const CI_LAB_NoopCmd_t *data);
int32 CI_LAB_ResetCounters(const CI_LAB_ResetCountersCmd_t *data);

/* Housekeeping message handler */
int32 CI_LAB_ReportHousekeeping(const CFE_MSG_CommandHeader_t *data);

/** * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* CI_Lab_AppMain() -- Application entry point and main process loop */
Expand All @@ -101,8 +95,9 @@ int32 CI_LAB_ReportHousekeeping(const CFE_SB_CmdHdr_t *data);
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * **/
void CI_Lab_AppMain(void)
{
int32 status;
uint32 RunStatus = CFE_ES_RunStatus_APP_RUN;
int32 status;
uint32 RunStatus = CFE_ES_RunStatus_APP_RUN;
CFE_SB_Buffer_t *SBBufPtr;

CFE_ES_PerfLogEntry(CI_LAB_MAIN_TASK_PERF_ID);

Expand All @@ -116,13 +111,13 @@ void CI_Lab_AppMain(void)
CFE_ES_PerfLogExit(CI_LAB_MAIN_TASK_PERF_ID);

/* Pend on receipt of command packet -- timeout set to 500 millisecs */
status = CFE_SB_RcvMsg(&CI_LAB_Global.MsgPtr, CI_LAB_Global.CommandPipe, 500);
status = CFE_SB_ReceiveBuffer(&SBBufPtr, CI_LAB_Global.CommandPipe, 500);

CFE_ES_PerfLogEntry(CI_LAB_MAIN_TASK_PERF_ID);

if (status == CFE_SUCCESS)
{
CI_LAB_ProcessCommandPacket();
CI_LAB_ProcessCommandPacket(SBBufPtr);
}

/* Regardless of packet vs timeout, always process uplink queue */
Expand Down Expand Up @@ -201,7 +196,7 @@ void CI_LAB_TaskInit(void)
*/
OS_TaskInstallDeleteHandler(&CI_LAB_delete_callback);

CFE_MSG_Init(&CI_LAB_Global.HkBuffer.HkTlm.TlmHeader.BaseMsg, CI_LAB_HK_TLM_MID, CI_LAB_HK_TLM_LNGTH);
CFE_MSG_Init(&CI_LAB_Global.HkTlm.TlmHeader.Msg, CI_LAB_HK_TLM_MID, sizeof(CI_LAB_Global.HkTlm));

CFE_EVS_SendEvent(CI_LAB_STARTUP_INF_EID, CFE_EVS_EventType_INFORMATION, "CI Lab Initialized.%s",
CI_LAB_VERSION_STRING);
Expand All @@ -220,24 +215,24 @@ void CI_LAB_TaskInit(void)
/* 3. Request for housekeeping telemetry packet (from HS task) */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
void CI_LAB_ProcessCommandPacket(void)
void CI_LAB_ProcessCommandPacket(CFE_SB_Buffer_t *SBBufPtr)
{
CFE_SB_MsgId_t MsgId = CFE_SB_INVALID_MSG_ID;

CFE_MSG_GetMsgId(CI_LAB_Global.MsgPtr, &MsgId);
CFE_MSG_GetMsgId(&SBBufPtr->Msg, &MsgId);

switch (CFE_SB_MsgIdToValue(MsgId))
{
case CI_LAB_CMD_MID:
CI_LAB_ProcessGroundCommand();
CI_LAB_ProcessGroundCommand(SBBufPtr);
break;

case CI_LAB_SEND_HK_MID:
CI_LAB_ReportHousekeeping((const CFE_SB_CmdHdr_t *)CI_LAB_Global.MsgPtr);
CI_LAB_ReportHousekeeping((const CFE_MSG_CommandHeader_t *)SBBufPtr);
break;

default:
CI_LAB_Global.HkBuffer.HkTlm.Payload.CommandErrorCounter++;
CI_LAB_Global.HkTlm.Payload.CommandErrorCounter++;
CFE_EVS_SendEvent(CI_LAB_COMMAND_ERR_EID, CFE_EVS_EventType_ERROR, "CI: invalid command packet,MID = 0x%x",
(unsigned int)CFE_SB_MsgIdToValue(MsgId));
break;
Expand All @@ -253,21 +248,27 @@ void CI_LAB_ProcessCommandPacket(void)
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * **/

void CI_LAB_ProcessGroundCommand(void)
void CI_LAB_ProcessGroundCommand(CFE_SB_Buffer_t *SBBufPtr)
{
CFE_MSG_FcnCode_t CommandCode = 0;

CFE_MSG_GetFcnCode(CI_LAB_Global.MsgPtr, &CommandCode);
CFE_MSG_GetFcnCode(&SBBufPtr->Msg, &CommandCode);

/* Process "known" CI task ground commands */
switch (CommandCode)
{
case CI_LAB_NOOP_CC:
CI_LAB_Noop((const CI_LAB_Noop_t *)CI_LAB_Global.MsgPtr);
if (CI_LAB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CI_LAB_NoopCmd_t)))
{
CI_LAB_Noop((const CI_LAB_NoopCmd_t *)SBBufPtr);
}
break;

case CI_LAB_RESET_COUNTERS_CC:
CI_LAB_ResetCounters((const CI_LAB_ResetCounters_t *)CI_LAB_Global.MsgPtr);
if (CI_LAB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CI_LAB_ResetCountersCmd_t)))
{
CI_LAB_ResetCounters((const CI_LAB_ResetCountersCmd_t *)SBBufPtr);
}
break;

/* default case already found during FC vs length test */
Expand All @@ -286,10 +287,10 @@ void CI_LAB_ProcessGroundCommand(void)
/* Handle NOOP command packets */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
int32 CI_LAB_Noop(const CI_LAB_Noop_t *data)
int32 CI_LAB_Noop(const CI_LAB_NoopCmd_t *data)
{
/* Does everything the name implies */
CI_LAB_Global.HkBuffer.HkTlm.Payload.CommandCounter++;
CI_LAB_Global.HkTlm.Payload.CommandCounter++;

CFE_EVS_SendEvent(CI_LAB_COMMANDNOP_INF_EID, CFE_EVS_EventType_INFORMATION, "CI: NOOP command");

Expand All @@ -303,7 +304,7 @@ int32 CI_LAB_Noop(const CI_LAB_Noop_t *data)
/* Handle ResetCounters command packets */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
int32 CI_LAB_ResetCounters(const CI_LAB_ResetCounters_t *data)
int32 CI_LAB_ResetCounters(const CI_LAB_ResetCountersCmd_t *data)
{
CFE_EVS_SendEvent(CI_LAB_COMMANDRST_INF_EID, CFE_EVS_EventType_INFORMATION, "CI: RESET command");
CI_LAB_ResetCounters_Internal();
Expand All @@ -319,11 +320,11 @@ int32 CI_LAB_ResetCounters(const CI_LAB_ResetCounters_t *data)
/* telemetry, packetize it and send it to the housekeeping task via */
/* the software bus */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
int32 CI_LAB_ReportHousekeeping(const CFE_SB_CmdHdr_t *data)
int32 CI_LAB_ReportHousekeeping(const CFE_MSG_CommandHeader_t *data)
{
CI_LAB_Global.HkBuffer.HkTlm.Payload.SocketConnected = CI_LAB_Global.SocketConnected;
CFE_SB_TimeStampMsg(&CI_LAB_Global.HkBuffer.MsgHdr);
CFE_SB_SendMsg(&CI_LAB_Global.HkBuffer.MsgHdr);
CI_LAB_Global.HkTlm.Payload.SocketConnected = CI_LAB_Global.SocketConnected;
CFE_SB_TimeStampMsg(&CI_LAB_Global.HkTlm.TlmHeader.Msg);
CFE_SB_TransmitMsg(&CI_LAB_Global.HkTlm.TlmHeader.Msg, true);
return CFE_SUCCESS;

} /* End of CI_LAB_ReportHousekeeping() */
Expand All @@ -339,12 +340,12 @@ int32 CI_LAB_ReportHousekeeping(const CFE_SB_CmdHdr_t *data)
void CI_LAB_ResetCounters_Internal(void)
{
/* Status of commands processed by CI task */
CI_LAB_Global.HkBuffer.HkTlm.Payload.CommandCounter = 0;
CI_LAB_Global.HkBuffer.HkTlm.Payload.CommandErrorCounter = 0;
CI_LAB_Global.HkTlm.Payload.CommandCounter = 0;
CI_LAB_Global.HkTlm.Payload.CommandErrorCounter = 0;

/* Status of packets ingested by CI task */
CI_LAB_Global.HkBuffer.HkTlm.Payload.IngestPackets = 0;
CI_LAB_Global.HkBuffer.HkTlm.Payload.IngestErrors = 0;
CI_LAB_Global.HkTlm.Payload.IngestPackets = 0;
CI_LAB_Global.HkTlm.Payload.IngestErrors = 0;

return;

Expand All @@ -364,20 +365,21 @@ void CI_LAB_ReadUpLink(void)
{
status = OS_SocketRecvFrom(CI_LAB_Global.SocketID, CI_LAB_Global.IngestBuffer.bytes,
sizeof(CI_LAB_Global.IngestBuffer), &CI_LAB_Global.SocketAddress, OS_CHECK);
if (status >= ((int32)CFE_SB_CMD_HDR_SIZE) && status <= ((int32)CI_LAB_MAX_INGEST))
if (status >= sizeof(CFE_MSG_CommandHeader_t) && status <= ((int32)CI_LAB_MAX_INGEST))
{
CFE_ES_PerfLogEntry(CI_LAB_SOCKET_RCV_PERF_ID);
CI_LAB_Global.HkBuffer.HkTlm.Payload.IngestPackets++;
status = CFE_SB_SendMsg(&CI_LAB_Global.IngestBuffer.MsgHdr);
CI_LAB_Global.HkTlm.Payload.IngestPackets++;
status = CFE_SB_TransmitMsg(&CI_LAB_Global.IngestBuffer.Msg, false);
CFE_ES_PerfLogExit(CI_LAB_SOCKET_RCV_PERF_ID);
}
else if (status > 0)
{
/* bad size, report as ingest error */
CI_LAB_Global.HkBuffer.HkTlm.Payload.IngestErrors++;
CI_LAB_Global.HkTlm.Payload.IngestErrors++;
CFE_EVS_SendEvent(CI_LAB_INGEST_ERR_EID, CFE_EVS_EventType_ERROR,
"CI: L%d, cmd %0x %0x dropped, bad length=%d\n", __LINE__,
CI_LAB_Global.IngestBuffer.hwords[0], CI_LAB_Global.IngestBuffer.hwords[1], (int)status);
"CI: L%d, cmd %0x%0x %0x%0x dropped, bad length=%d\n", __LINE__,
CI_LAB_Global.IngestBuffer.bytes[0], CI_LAB_Global.IngestBuffer.bytes[1],
CI_LAB_Global.IngestBuffer.bytes[2], CI_LAB_Global.IngestBuffer.bytes[3], (int)status);
}
else
{
Expand All @@ -394,10 +396,10 @@ void CI_LAB_ReadUpLink(void)
/* CI_LAB_VerifyCmdLength() -- Verify command packet length */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * **/
bool CI_LAB_VerifyCmdLength(CFE_MSG_Message_t *MsgPtr, CFE_MSG_Size_t ExpectedLength)
bool CI_LAB_VerifyCmdLength(CFE_MSG_Message_t *MsgPtr, size_t ExpectedLength)
{
bool result = true;
CFE_MSG_Size_t ActualLength = 0;
size_t ActualLength = 0;
CFE_MSG_FcnCode_t FcnCode = 0;
CFE_SB_MsgId_t MsgId = CFE_SB_INVALID_MSG_ID;

Expand All @@ -416,7 +418,7 @@ bool CI_LAB_VerifyCmdLength(CFE_MSG_Message_t *MsgPtr, CFE_MSG_Size_t ExpectedLe
(unsigned int)CFE_SB_MsgIdToValue(MsgId), (unsigned int)FcnCode, (unsigned int)ActualLength,
(unsigned int)ExpectedLength);
result = false;
CI_LAB_Global.HkBuffer.HkTlm.Payload.CommandErrorCounter++;
CI_LAB_Global.HkTlm.Payload.CommandErrorCounter++;
}

return (result);
Expand Down
6 changes: 3 additions & 3 deletions fsw/src/ci_lab_app.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@
*/
void CI_Lab_AppMain(void);
void CI_LAB_TaskInit(void);
void CI_LAB_ProcessCommandPacket(void);
void CI_LAB_ProcessGroundCommand(void);
void CI_LAB_ProcessCommandPacket(CFE_SB_Buffer_t *SBBufPtr);
void CI_LAB_ProcessGroundCommand(CFE_SB_Buffer_t *SBBufPtr);
void CI_LAB_ResetCounters_Internal(void);
void CI_LAB_ReadUpLink(void);

bool CI_LAB_VerifyCmdLength(CFE_MSG_Message_t *MsgPtr, CFE_MSG_Size_t ExpectedLength);
bool CI_LAB_VerifyCmdLength(CFE_MSG_Message_t *MsgPtr, size_t ExpectedLength);

#endif /* _ci_lab_app_h_ */
12 changes: 5 additions & 7 deletions fsw/src/ci_lab_msg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
*/
typedef struct
{
uint8 CmdHeader[CFE_SB_CMD_HDR_SIZE];
CFE_MSG_CommandHeader_t CmdHeader;

} CI_LAB_NoArgsCmd_t;

Expand All @@ -52,8 +52,8 @@ typedef struct
*
* This matches the pattern in CFE core and other modules.
*/
typedef CI_LAB_NoArgsCmd_t CI_LAB_Noop_t;
typedef CI_LAB_NoArgsCmd_t CI_LAB_ResetCounters_t;
typedef CI_LAB_NoArgsCmd_t CI_LAB_NoopCmd_t;
typedef CI_LAB_NoArgsCmd_t CI_LAB_ResetCountersCmd_t;

/*************************************************************************/
/*
Expand All @@ -74,12 +74,10 @@ typedef struct

typedef struct
{
CFE_SB_TlmHdr_t TlmHeader;
CI_LAB_HkTlm_Payload_t Payload;
CFE_MSG_TelemetryHeader_t TlmHeader;
CI_LAB_HkTlm_Payload_t Payload;
} CI_LAB_HkTlm_t;

#define CI_LAB_HK_TLM_LNGTH sizeof(CI_LAB_HkTlm_t)

#endif /* _ci_lab_msg_h_ */

/************************/
Expand Down