Consolidate similar commands

[skliper]

Several command handler functions are nearly identical. Could be consolidated.

CS_DisableAppCmd/CS_EnableAppCmd CS_DisableNameAppCmd/CS_EnableNameAppCmd CS_DisableEepromCmd/CS_EnableEepromCmd CS_DisableMemoryCmd/CS_EnableMemoryCmd CS_DisableEntryIDMemoryCmd/CS_EnableEntryIDMemoryCmd CS_DisableTablesCmd/CS_EnableTablesCmd CS_DisableNameTablesCmd/CS_EnableNameTablesCmd

Imported from GSFCCFS-1322

[thnkslprpt]

@skliper Do you mean combine these (opposing) functions together, or separate out the common logic into another function?

[dzbaker]

@skliper Do you mean combine these (opposing) functions together, or separate out the common logic into another function?

I'm thinking the latter. @dmknutsen, what do you think?

[skliper]

I think this issue comes from a code review, where the issue author is suggesting a patter like CF. Basically implement one common function that takes the parameter and the selection as inputs, then each individual handler just wraps that function with the appropriate inputs.

See where it starts in CF here: https://github.com/nasa/CF/blob/281a94188cd7d885a5aed01ee041f3ece2b0486f/fsw/src/cf_cmd.c#L542-L557

I think it's worth a careful trade before going as far as CF though... confirm it really is simpler and there's sufficient consolidation possible.

[dmknutsen]

I think the latter makes sense...but can't say I have a strong opinion either way. The one thing I would caution is that this change would also likely require requirements updates, BVT test updates, and documentation updates in addition to the code changes (we are currently updating all the CS BVTs to be portable for the January release...).

[thnkslprpt]

These pairs of functions are ~90% identical. There seems to be significant de-duplication possible using something like these first couple that I mocked up from the list:

void CS_DoEnableDisableAppCmd(const CS_NoArgsCmd_t *CmdPtr, uint16 State, uint16 EventId)
{
    /* command verification variables */
    size_t ExpectedLength = sizeof(CS_NoArgsCmd_t);

    /* Verify command packet length */
    if (CS_VerifyCmdLength(&CmdPtr->CmdHeader.Msg, ExpectedLength))
    {
        if (CS_CheckRecomputeOneshot() == false)
        {
            CS_AppData.HkPacket.AppCSState = State;

            if (State == CS_STATE_DISABLED)
            {
                CS_ZeroAppTempValues();
            }

#if (CS_PRESERVE_STATES_ON_PROCESSOR_RESET == true)
            CS_UpdateCDS();
#endif

            CFE_EVS_SendEvent(EventId, CFE_EVS_EventType_INFORMATION, "Checksumming of App is %s",
                              State == CS_STATE_ENABLED ? "Enabled" : "Disabled");
            CS_AppData.HkPacket.CmdCounter++;
        }
    }
}

void CS_DisableAppCmd(const CS_NoArgsCmd_t *CmdPtr)
{
    CS_DoEnableDisableAppCmd(CmdPtr, CS_STATE_DISABLED, CS_DISABLE_APP_INF_EID);
}

void CS_EnableAppCmd(const CS_NoArgsCmd_t *CmdPtr)
{
    CS_DoEnableDisableAppCmd(CmdPtr, CS_STATE_ENABLED, CS_ENABLE_APP_INF_EID);
}

static void CS_DoEnableDisableNameAppCmd(const CS_AppNameCmd_t *CmdPtr, uint16 NewState, uint16 EventID)
{
    /* command verification variables */
    size_t ExpectedLength = sizeof(CS_AppNameCmd_t);

    CS_Res_App_Table_Entry_t *ResultsEntry;
    CS_Def_App_Table_Entry_t *DefinitionEntry;
    char                      Name[OS_MAX_API_NAME];

    /* Verify command packet length */
    if (CS_VerifyCmdLength(&CmdPtr->CmdHeader.Msg, ExpectedLength))
    {
        if (CS_CheckRecomputeOneshot() == false)
        {
            strncpy(Name, CmdPtr->Name, sizeof(Name) - 1);
            Name[sizeof(Name) - 1] = '\0';

            if (CS_GetAppResTblEntryByName(&ResultsEntry, Name))
            {
                ResultsEntry->State = NewState;

                if (NewState == CS_STATE_DISABLED)
                {
                    ResultsEntry->TempChecksumValue = 0;
                    ResultsEntry->ByteOffset        = 0;
                }

                CFE_EVS_SendEvent(EventID, CFE_EVS_EventType_INFORMATION, "Checksumming of app %s is %s", Name,
                                  NewState == CS_STATE_ENABLED ? "Enabled" : "Disabled");

                if (CS_GetAppDefTblEntryByName(&DefinitionEntry, Name))
                {
                    DefinitionEntry->State = NewState;
                    CS_ResetTablesTblResultEntry(CS_AppData.AppResTablesTblPtr);
                    CFE_TBL_Modified(CS_AppData.DefAppTableHandle);
                }
                else
                {
                    CFE_EVS_SendEvent(NewState == CS_STATE_ENABLED ? CS_ENABLE_APP_DEF_NOT_FOUND_DBG_EID
                                                                   : CS_DISABLE_APP_DEF_NOT_FOUND_DBG_EID,
                                      CFE_EVS_EventType_DEBUG, "CS unable to update apps definition table for entry %s",
                                      Name);
                }

                CS_AppData.HkPacket.CmdCounter++;
            }
            else
            {
                CFE_EVS_SendEvent(NewState == CS_STATE_ENABLED ? CS_ENABLE_APP_UNKNOWN_NAME_ERR_EID
                                                               : CS_DISABLE_APP_UNKNOWN_NAME_ERR_EID,
                                  CFE_EVS_EventType_ERROR, "App %s app command failed, app %s not found",
                                  NewState == CS_STATE_ENABLED ? "enable" : "disable", Name);
                CS_AppData.HkPacket.CmdErrCounter++;
            }
        } /* end InProgress if */
    }
}

void CS_DisableNameAppCmd(const CS_AppNameCmd_t *CmdPtr)
{
    CS_DoEnableDisableNameAppCmd(CmdPtr, CS_STATE_DISABLED, CS_DISABLE_APP_NAME_INF_EID);
}

void CS_EnableNameAppCmd(const CS_AppNameCmd_t *CmdPtr)
{
    CS_DoEnableDisableNameAppCmd(CmdPtr, CS_STATE_ENABLED, CS_ENABLE_APP_NAME_INF_EID);
}