Skip to content

Commit

Permalink
Fix #34: Follow recommended patterns for message handling
Browse files Browse the repository at this point in the history 
Make the patterns in CI better match the patterns used
in other modules (CFE core, Sample App, etc)

- Separate each command into a separate handler function
- Each command handler accepts a const pointer to the full message
- Put Telemetry payload into a separate "Payload" sub-structure
- Use naming conventions defined in conventions document

Note the payload name changes only affect the FSW internal
usage, the payload format for the ground system is not changed.
  • Loading branch information
@jphickey
jphickey committed Feb 8, 2020
1 parent 1f1c178 commit e1072c0
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 34 deletions.
96 changes: 70 additions & 26 deletions fsw/src/ci_lab_app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ static CFE_EVS_BinFilter_t CI_EventFilters[] = {/* Event ID mask */
{CI_COMMANDNOP_INF_EID, 0x0000}, {CI_COMMANDRST_INF_EID, 0x0000},
{CI_INGEST_INF_EID, 0x0000}, {CI_INGEST_ERR_EID, 0x0000}};

/*
* Individual message handler function prototypes
*
* Per the recommended code pattern, these should accept a const pointer
* to a structure type which matches the message, and return an int32
* where CFE_SUCCESS (0) indicates successful handling of the message.
*/
int32 CI_NoopCmd(const CI_NoopCmd_t *data);
int32 CI_ResetCountersCmd(const CI_ResetCountersCmd_t *data);
int32 CI_ReportHousekeeping(const CCSDS_CommandPacket_t *data);

/** * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* CI_Lab_AppMain() -- Application entry point and main process loop */
/* Purpose: This is the Main task event loop for the Command Ingest Task */
Expand Down Expand Up @@ -201,11 +212,11 @@ void CI_ProcessCommandPacket(void)
break;

case CI_LAB_SEND_HK_MID:
CI_ReportHousekeeping();
CI_ReportHousekeeping((const CCSDS_CommandPacket_t *)CIMsgPtr);
break;

default:
CI_HkTelemetryPkt.ci_command_error_count++;
CI_HkTelemetryPkt.Payload.CommandErrorCounter++;
CFE_EVS_SendEvent(CI_COMMAND_ERR_EID, CFE_EVS_EventType_ERROR, "CI: invalid command packet,MID = 0x%x",
MsgId);
break;
Expand All @@ -231,12 +242,11 @@ void CI_ProcessGroundCommand(void)
switch (CommandCode)
{
case CI_NOOP_CC:
CI_HkTelemetryPkt.ci_command_count++;
CFE_EVS_SendEvent(CI_COMMANDNOP_INF_EID, CFE_EVS_EventType_INFORMATION, "CI: NOOP command");
CI_NoopCmd((const CI_NoopCmd_t *)CIMsgPtr);
break;

case CI_RESET_COUNTERS_CC:
CI_ResetCounters();
CI_ResetCountersCmd((const CI_ResetCountersCmd_t *)CIMsgPtr);
break;

/* default case already found during FC vs length test */
Expand All @@ -248,6 +258,37 @@ void CI_ProcessGroundCommand(void)

} /* End of CI_ProcessGroundCommand() */

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* Name: CI_NoopCmd */
/* */
/* Purpose: */
/* Handle NOOP command packets */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
int32 CI_NoopCmd(const CI_NoopCmd_t *data)
{
/* Does everything the name implies */
CI_HkTelemetryPkt.Payload.CommandCounter++;

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

return CFE_SUCCESS;
}


/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* Name: CI_ResetCountersCmd */
/* */
/* Purpose: */
/* Handle ResetCounters command packets */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
int32 CI_ResetCountersCmd(const CI_ResetCountersCmd_t *data)
{
CI_ResetCounters();
return CFE_SUCCESS;
}

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * **/
/* Name: CI_ReportHousekeeping */
/* */
Expand All @@ -257,12 +298,12 @@ void CI_ProcessGroundCommand(void)
/* telemetry, packetize it and send it to the housekeeping task via */
/* the software bus */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
void CI_ReportHousekeeping(void)
int32 CI_ReportHousekeeping(const CCSDS_CommandPacket_t *data)
{
CI_HkTelemetryPkt.SocketConnected = CI_SocketConnected;
CI_HkTelemetryPkt.Payload.SocketConnected = CI_SocketConnected;
CFE_SB_TimeStampMsg((CFE_SB_Msg_t *)&CI_HkTelemetryPkt);
CFE_SB_SendMsg((CFE_SB_Msg_t *)&CI_HkTelemetryPkt);
return;
return CFE_SUCCESS;

} /* End of CI_ReportHousekeeping() */

Expand All @@ -277,12 +318,12 @@ void CI_ReportHousekeeping(void)
void CI_ResetCounters(void)
{
/* Status of commands processed by CI task */
CI_HkTelemetryPkt.ci_command_count = 0;
CI_HkTelemetryPkt.ci_command_error_count = 0;
CI_HkTelemetryPkt.Payload.CommandCounter = 0;
CI_HkTelemetryPkt.Payload.CommandErrorCounter = 0;

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

CFE_EVS_SendEvent(CI_COMMANDRST_INF_EID, CFE_EVS_EventType_INFORMATION, "CI: RESET command");
return;
Expand All @@ -302,21 +343,24 @@ void CI_ReadUpLink(void)
for (i = 0; i <= 10; i++)
{
status = OS_SocketRecvFrom(CI_SocketID, CI_IngestBuffer.bytes, sizeof(CI_IngestBuffer), &CI_SocketAddress, OS_CHECK);
if (status < 0)
break; /* no (more) messages */
if (status >= ((int32)CFE_SB_CMD_HDR_SIZE) &&
status <= ((int32)CI_MAX_INGEST))
{
CFE_ES_PerfLogEntry(CI_SOCKET_RCV_PERF_ID);
CI_HkTelemetryPkt.Payload.IngestPackets++;
status = CFE_SB_SendMsg(&CI_IngestBuffer.MsgHdr);
CFE_ES_PerfLogExit(CI_SOCKET_RCV_PERF_ID);
}
else if (status > 0)
{
/* bad size, report as ingest error */
CI_HkTelemetryPkt.Payload.IngestErrors++;
CFE_EVS_SendEvent(CI_INGEST_ERR_EID,CFE_EVS_EventType_ERROR, "CI: L%d, cmd %0x %0x dropped, bad length=%d\n", __LINE__,
CI_IngestBuffer.hwords[0], CI_IngestBuffer.hwords[1], (int)status);
}
else
{
if (status <= CI_MAX_INGEST)
{
CFE_ES_PerfLogEntry(CI_SOCKET_RCV_PERF_ID);
CI_HkTelemetryPkt.IngestPackets++;
CFE_SB_SendMsg(&CI_IngestBuffer.MsgHdr);
CFE_ES_PerfLogExit(CI_SOCKET_RCV_PERF_ID);
}
else
{
CI_HkTelemetryPkt.IngestErrors++;
}
break; /* no (more) messages */
}
}

Expand Down Expand Up @@ -346,7 +390,7 @@ bool CI_VerifyCmdLength(CFE_SB_MsgPtr_t msg, uint16 ExpectedLength)
"Invalid msg length: ID = 0x%X, CC = %d, Len = %d, Expected = %d", MessageID, CommandCode,
ActualLength, ExpectedLength);
result = false;
CI_HkTelemetryPkt.ci_command_error_count++;
CI_HkTelemetryPkt.Payload.CommandErrorCounter++;
}

return (result);
Expand Down
5 changes: 2 additions & 3 deletions fsw/src/ci_lab_app.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,13 @@
/*
** Local function prototypes...
**
** Note: Except for the entry point (CI_task_main), these
** Note: Except for the entry point (CI_Lab_AppMain), these
** functions are not called from any other source module.
*/
void CI_task_main(void);
void CI_Lab_AppMain(void);
void CI_TaskInit(void);
void CI_ProcessCommandPacket(void);
void CI_ProcessGroundCommand(void);
void CI_ReportHousekeeping(void);
void CI_ResetCounters(void);
void CI_ReadUpLink(void);

Expand Down
25 changes: 20 additions & 5 deletions fsw/src/ci_lab_msg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,38 @@ typedef struct

} CI_NoArgsCmd_t;

/*
* Neither the Noop nor ResetCounters command
* have any payload, but should still "reserve" a unique
* structure type to employ a consistent handler pattern.
*
* This matches the pattern in CFE core and other modules.
*/
typedef CI_NoArgsCmd_t CI_NoopCmd_t;
typedef CI_NoArgsCmd_t CI_ResetCountersCmd_t;


/*************************************************************************/
/*
** Type definition (CI_Lab housekeeping)...
*/
typedef struct
{

uint8 TlmHeader[CFE_SB_TLM_HDR_SIZE];
uint8 ci_command_error_count;
uint8 ci_command_count;
uint8 ci_xsums_enabled;
uint8 CommandErrorCounter;
uint8 CommandCounter;
uint8 EnableChecksums;
uint8 SocketConnected;
uint8 Spare1[8];
uint32 IngestPackets;
uint32 IngestErrors;
uint32 Spare2;

} CI_HkTlm_Payload_t;

typedef struct
{
uint8 TlmHeader[CFE_SB_TLM_HDR_SIZE];
CI_HkTlm_Payload_t Payload;
} OS_PACK ci_hk_tlm_t;

#define CI_LAB_HK_TLM_LNGTH sizeof(ci_hk_tlm_t)
Expand Down

0 comments on commit e1072c0

Please sign in to comment.