nasa / cFE

The Core Flight System (cFS) Core Flight Executive (cFE)
Apache License 2.0
413 stars 204 forks source link

SB subscription reporting request messages out of family #523

Closed skliper closed 4 years ago

skliper commented 4 years ago

Is your feature request related to a problem? Please describe. CFE_SB_EnableSubReportingCmd, CFE_SB_DisableSubReportingCmd, CFE_SB_SendPrevSubsCmd are processed like commands but don't increment the command counter.

Typical pattern is for non-ground, inter-app messages to have separate message IDs from ground commands.

Describe the solution you'd like Make consistent with standard pattern

Describe alternatives you've considered None

Additional context See Hk message processing, or the message processing in Time services that don't increment command counter.

Requester Info Jacob Hageman - NASA/GSFC

skliper commented 4 years ago

@CDKnightNASA @tngo67 Does SBN or SBNg use these? Since they are from another app, preference would be to not use the ground command MID. Can we coordinate a change?

jwilmot commented 4 years ago

As I recall these were for SBN to use when interfacing with SB. They are not ground commands. SBN is designed to be (as best we could) a plug-in just like any other apps. We wanted to avoid call backs or other compile/link time hooks. That said, the internal MID are in the same name space as "regular" commands so need to be allocated and managed but not in the ground system database. They should not increment the command counter +/- as those are for operators, but should send an event if an error occurs.

skliper commented 4 years ago

I think you are saying they should be a separate MID from the ground commands? Want them to use the same MID (CFE_SB_SUBSCRIPTION_RPT_CONTROL_MID or whatever), or each use their own?

CDKnightNASA commented 4 years ago

As I recall these were for SBN to use when interfacing with SB. They are not ground commands. SBN is designed to be (as best we could) a plug-in just like any other apps. We wanted to avoid call backs or other compile/link time hooks. That said, the internal MID are in the same name space as "regular" commands so need to be allocated and managed but not in the ground system database. They should not increment the command counter +/- as those are for operators, but should send an event if an error occurs.

Nope, SBN does not use this API. SBN listens for events that are generated by subscription activity:

    Status = CFE_SB_SubscribeLocal(CFE_SB_ALLSUBS_TLM_MID, SBN.SubPipe,
...
    Status = CFE_SB_SubscribeLocal(CFE_SB_ONESUB_TLM_MID, SBN.SubPipe,
skliper commented 4 years ago

So who/what sends the referenced commands?

CDKnightNASA commented 4 years ago

So who/what sends the referenced commands?

Sorry, looking through the code, SBN does send the relevant commands. I was looking for direct calls to the API.

sbn_sub.c:

void SBN_SendSubsRequests(void)
{
    CFE_SB_CmdHdr_t     SBCmdMsg;

    /* Turn on SB subscription reporting */
    CFE_SB_InitMsg(&SBCmdMsg, CFE_SB_CMD_MID, sizeof(CFE_SB_CmdHdr_t),
        TRUE);
    CFE_SB_SetCmdCode((CFE_SB_MsgPtr_t) &SBCmdMsg,
        CFE_SB_ENABLE_SUB_REPORTING_CC);
    CFE_SB_SendMsg((CFE_SB_MsgPtr_t) &SBCmdMsg);

    /* Request a list of previous subscriptions from SB */
    CFE_SB_SetCmdCode((CFE_SB_MsgPtr_t) &SBCmdMsg, CFE_SB_SEND_PREV_SUBS_CC);
    CFE_SB_SendMsg((CFE_SB_MsgPtr_t) &SBCmdMsg);
}/* end SBN_SendSubsRequests */
jwilmot commented 4 years ago

The required SBN behavior is to requests subscriptions when it is done initializing the subnetwork and discovered peers. This can happen any time after local system init or later due to fault management restarting SBN. This decouples SBN from the local system. The internal commands CFE_SB_EnableSubReportingCmd, CFE_SB_DisableSubReportingCmd, CFE_SB_SendPrevSubsCmd support this. Like other internal commands, the counters should not increment but error events should be sent. Sorry I am being dense, but it's still not clear what problem this ticket is trying to solve.

skliper commented 4 years ago

Discussed w/ @jwilmot and got concurrence with the following approach.

Inter-app messages/MIDs: CFE_SB_SEND_HK_MID - For scheduler to request HK message CFE_SB_SUBSCRIPTION_RPT_CONTROL_MID - Use for these three commands

Ground command MID: CFE_SB_CMD_MID - ground commands only

So it'll require a 1 line change in SBN, but provides separation between the inter-app and ground commands (preferred model).

juliejohn015 commented 3 years ago

Subscription Reporting and Analytics Software