Remove CFE_SB_SendPrevSubs in favor of CFE_SBR_ForEachRouteId API

[skliper]

Is your feature request related to a problem? Please describe.
CFE_SB_SendPrevSubs holds locks that aren't that useful, sufficient logic required, creates bus traffic, etc.

Describe the solution you'd like
SBN could just use CFE_SBR_ForEachRouteId, not really a true "public" API... but seems like fair use in this case. Would be outside of SB lock, but likely OK based on design (enables subscription reporting first, then check all previous subs).

Describe alternatives you've considered
At least remove all the locking/unlocking. It doesn't help.

Additional context
Related to #947 work.

Requester Info
Jacob Hageman - NASA/GSFC

Ping @CDKnightNASA

[skliper]

@CDKnightNASA - thoughts on this? can we get rid of CFE_SB_SendPrevSubs?

[jwilmot]

Are the requirements/use cases still met? CFE_SB_SendPrevSubs was to inform SBN that subscriptions existed prior to SBN startup so that SBN could forward those to peers. SBN is started after cFE so subscriptions do already exist. The interface needs to be robust so that no subscriptions are lost and avoid any potential subscribe/unsubscribe race conditions. If CFE_SBR_ForEachRouteId does this then CFE_SB_SendPrevSubs is redundant and should be removed.

[skliper]

@jwilmot The implementation (as I understand it) enables reporting (to avoid missing new subscriptions), then gets all prev subs which would work with either method. If done in the opposite order, subscriptions can be missed either using CFE_SB_SendPrevSubs or using CFE_SBR_ForEachRouteId. Note that routes are never deleted (and there's no message to SBN for a message unsubscribe), so there's no race on that end as long as you are OK with SBN getting all the routes (even if not currently in use).

[jwilmot]

There was an unsubscribe "SBN_ProcessLocalUnsub" and "SBN_RemoveAllSubsFromPeer" in an older codebase. In the past, SBN would dynamically and reliably subscribe and unsubscribe to peers as informed by the local cFE SB. The idea was to have consistent state across all peers. Looks like something got lost over the years. The expected behavior is broken if there is no way for SBN to remove a subscription.

[skliper]

I'm not saying SBN doesn't have the capability to unsubscribe, there's just no message from SB that says a message was unsubscribed.

[CDKnightNASA]

Is there a SB housekeeping CC/Tlm struct that contains current (as of the HK response) subscription information? I'd think the ground would want a way to query what subscriptions were active. I suggest SBN could use a HK command/response...

[CDKnightNASA]

Is there a SB housekeeping CC/Tlm struct that contains current (as of the HK response) subscription information? I'd think the ground would want a way to query what subscriptions were active. I suggest SBN could use a HK command/response...

Ah, reviewing the code, looks like the model is that if GND wanted to see what the routing table contained, they dump it to a file and downlink the file. :\ Theoretically, SBN could use that route, although it's a bit gross...

The CFE_SBR_ForEachRouteId() API should be fine, a callback API is not much different than sending a message and handling the tlm messages. I'm wondering if this might be generalized to an API for a program being notified of sub/unsub rather than relying on messages on the bus? Seems odd to send bus status changes on the same bus where the changes are occurring.

[skliper]

Maybe add an API to register a callback function for new subscriptions? This isn't targeted for Caelum, but seems like a good path forward to reduce complexity in the core.

[CDKnightNASA]

So is the ForEachRouteId() ready for primetime? Sounds like I should switch SBN and you'll mark the SendPRevSubs and ALLSUBS bus messages deprecated, yes?

[skliper]

So is the ForEachRouteId() ready for primetime? Sounds like I should switch SBN and you'll mark the SendPRevSubs and ALLSUBS bus messages deprecated, yes?

Yes, ForEachRouteId is good to go.