Standardization of Command Responses

[dmknutsen]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I reviewed the README file to see if the feature is in the major future work.
• I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
cFS should have standardized command responses for commands that set a state/mode like enable/disable, on/off, true/false, start/stop/pause/resume, etc.

For those commands, the app should respond as describe below:

• If the current setting is not in that commanded state,

  1. If the commanded state can be set successfully,
     a. Increment the command counter
     b. Send an INFO event that says command successfully executed
  2. otherwise,
     a. Increment the command error counter
     b. Send an ERROR event that says failed to execute the command

• If the current setting is already in that commanded state,

  1. Increment the command counter, not command error counter
  2. Send an INFO event that says already in that state

The following event IDs issued for successful commands are debug type - which does not align with the standard:

HS_RESET_DBG_EID
HS_DISABLE_APPMON_DBG_EID
HS_ENABLE_EVENTMON_DBG_EID
HS_DISABLE_EVENTMON_DBG_EID
HS_ENABLE_ALIVENESS_DBG_EID
HS_DISABLE_ALIVENESS_DBG_EID
HS_ENABLE_CPUHOG_DBG_EID
HS_DISABLE_CPUHOG_DBG_EID
HS_RESET_RESETS_DBG_EID
HS_SET_MAX_RESETS_DBG_EID

Describe the solution you'd like
Update logic to align with the standard.

Requester Info
Dan Knutsen
NASA Goddard

[thnkslprpt]

There's one more that wasn't listed: HS_ENABLE_APPMON_DBG_EID

[dmknutsen]

Thanks for the heads up!

[thnkslprpt]

@dmknutsen Something like this?:

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/*                                                                 */
/* Disable applications monitor command                            */
/*                                                                 */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
CFE_Status_t HS_DisableAppMonCmd(const HS_DisableAppMonCmd_t *BufPtr)
{
    HS_AppData.CmdCount++;

    if (HS_AppData.CurrentAppMonState == HS_STATE_DISABLED)
    {
        CFE_EVS_SendEvent(HS_DISABLE_APPMON_INF_EID, CFE_EVS_EventType_INFORMATION, "Application Monitoring *already* Disabled");
    }
    else
    {
        HS_AppData.CurrentAppMonState = HS_STATE_DISABLED;
        CFE_EVS_SendEvent(HS_DISABLE_APPMON_INF_EID, CFE_EVS_EventType_INFORMATION, "Application Monitoring Disabled");
    }

    return CFE_SUCCESS;
}

A separate question - create a new EID for these 'already in that state' events or re-use the successful command EIDs? (as per the above code snippet)

[skliper]

Consider setting a string based on state and just have one SendEvent call (with the same EID) for slightly less duplication.

[thnkslprpt]

OK, so how's this?
ping @dmknutsen and/or @skliper

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/*                                                                 */
/* Disable applications monitor command                            */
/*                                                                 */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
CFE_Status_t HS_DisableAppMonCmd(const HS_DisableAppMonCmd_t *BufPtr)
{
    char EventText[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH];

    HS_AppData.CmdCount++; /* Currently always successful*/

    /* Check if already in the commanded state */
    if (HS_AppData.CurrentAppMonState == HS_STATE_DISABLED)
    {
        snprintf(EventText, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH, "Application Monitoring *already* Disabled");
    }
    else
    {
        HS_AppData.CurrentAppMonState = HS_STATE_DISABLED;
        snprintf(EventText, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH, "Application Monitoring Disabled");
    }

    CFE_EVS_SendEvent(HS_DISABLE_APPMON_INF_EID, CFE_EVS_EventType_INFORMATION, EventText);

    return CFE_SUCCESS; /* Currently always returns success*/
}

In the end the 'less duplicative' version is longer...

[dmknutsen]

Either look good to me!

[thnkslprpt]

Either look good to me!

Went with option 1.