expose CFE_SB_GetPktType (and add stub)

[skliper]

Is your feature request related to a problem? Please describe.
Requests to expose packet type as an API (helps SBN, testing)

Describe the solution you'd like
Add requirement, expose in API header, add stub, etc.

Describe alternatives you've considered
N/A

Additional context
Slightly related to #263

Requester Info
Jacob Hageman - NASA/GSFC

[skliper]

@tngo67 - FYI

[skliper]

GetPktType seems more appropriate relative to the message (defined by standard) vs MsgId (really an internal Key for SB). What about changing the API (and possibly internal use) to be based on standards?

[jphickey]

GetPktType seems more appropriate relative to the message (defined by standard) vs MsgId (really an internal Key for SB). What about changing the API (and possibly internal use) to be based on standards?

The CFE_SB_MsgId_t type is intended as an abstract value that uniquely identifies a specific type of message. This is a public API type. It is not used for routing, at least not directly - There is a separate internal type called CFE_SB_MsgKey_t which is the internal type used for looking up into the routing tables, and there is a conversion function to get the key value from the CFE_SB_MsgId_t value. But that is all inside CFE SB and the "keys" for routing are not exposed to applications.

I see nothing wrong with use of the CFE_SB_MsgId_t type here; it is (potentially) much safer to pass around this abstract value than a raw pointer - the validation opportunities are much better.

[skliper]

But MsgId is not a standard, and abstracting MsgKey from MsgId does nothing in the current implementation. MsgId could have been directly used as the index into MsgMap (like it used to)... seems like folks are trying to overload the use of MsgId and building in a lot of dependencies on something that isn't standard (as illustrated by the current differences between when the extended header is used or not)

[jphickey]

Yes, in today's implementation the MsgKey and MsgId are similar - but the whole point is that they can evolve in the future and become more different. "MsgKey" was invented as part of the extended header work to decouple the routing logic from the generic/abstract representation of the MsgId.

However IIRC the final implementation basically did a simple translation between the two, with the intent of giving this some "soak time" without making fundamental changes to the way SB routing works in the first pass. That doesn't mean we should rip it out and go back to the way it was.

I just don't see the downside of allowing the SB routing logic to be decoupled from the public representation of the MsgId type that apps use. IMO this is a good thing. Yes it can be a simple translation between them, and when it is the cost is negligible. MsgKey is not a public type and shouldn't appear anywhere in the API and should need any documentation aside from the internal comments within the implementation.

FWIW, as a matter of principle, we should ALWAYS be using a separate/distinct identifier type in the public APIs, and translating this value to the internal index/lookup upon entry from external API calls. That translation can then apply all range checking/validation and enforcement, returning only the local index/key value to be used in internal lookup tables. This model is just good practice, and helps ensure that input values are always properly vetted before use, and helps maintain API stability by decoupling the internal tables from the external identifiers. I'm surprised that this isn't explicitly required in the coding standards, actually.

[skliper]

I'm leaning towards exposing GetPktTypeFromHdr. From CCSDS this is well defined and always in the same place. Could be redefined by non-CCSDS systems, but definitely not the norm for us.

If people feel strongly about having a GetPktTypeFromMsgId in the framework, could wrap cFE_SB_SetMsgId/GetPktTypeFromHdr to avoid requiring customization (SetMsgId is the implementation mapping, best to keep it in one place).

[skliper]

I don't think MsgId to MsgKey should be implementation defined other than the max limit on MsgKey to size MsgMap. That's probably a better framing of my concern. The wrapping to make MsgId of zero invalid, or to enforce limits and correct type definitely makes sense (and I agree is good practice).

[tngo67]

For Gateway, we need GetPktType() from both MID & msg header.

[skliper]

Thanks @tngo67! Out of curiosity, is there a specific use case for the API using the MID (when would the header not be available, but you need the packet type)?

[tngo67]

As an example only, you might have an app that uses a table describing the mapping between MID and destID for off-board forwarding, where priority goes to command before telemetry.

[astrogeco]

CCB 2020-04-01 - Discussed, will go ahead with implementation and use MsgID