Fix #263, expose IsValidMsgID and partially address #245 Consistent use of MsgId types

[jphickey]

Describe the contribution

Fixes #263

This exposes the CFE_SB_IsValidMsgId() in the CFE SB public API, rather than confining it to the private/internal API.

Partially addresses #245

This also includes a number of internal changes to make CFE core apps consistent in their use of the CFE_SB_MsgId type and with the CFE SB API. This employs the appropriate CFE SB abstraction API whenever any of the following needs to happen:

• Use of a CFE_SB_MsgId_t value within a printf (event, syslog, etc).
• Initialization of a CFE_SB_MsgId_t from an integer value
• Comparison of two CFE_SB_MsgId_t values
• Checking if a CFE_SB_MsgId_t value is within the valid set

Notable exception to integer conversion rules is for the MID #define values, which are not changed for backward compatibility reasons. These are still defined as integers, and used directly in the code.

Also worth noting - this replaces the switch/case constructs used in message dispatch with a nested if/else based on CFE_SB_MsgId_Equal(), as the switch statement can only be used with an integer.

A few new macros are introduced, mainly because the inline functions that already existed for this purpose cannot be used where evaluation must be done at compile time (e.g. constants, struct initialization). These are initially just typecasts, but could become more interesting
in future revisions.

Testing performed
Build with SIMULATION=native ENABLE_UNIT_TESTS=TRUE

• Run all unit tests and confirm passing
• Run sanity check on CFE, boot up and issue commands to each core app using cmdUtil. Ensure no changes to message routing/processing for every core app.

Expected behavior changes
This exposes the CFE_SB_IsValidMsgId() for application usage.
No other API or behavior changes. All other changes are transparent.

System(s) tested on
Ubuntu 18.04 LTS 64 bit

Additional context
This is a step in the direction of related issue #245 but does not do anything that would affect backward compatibility with the way the MID integers are currently defined and used. It does introduce a few of the macros that will become more relevant/necessary (i.e. so they can evolve beyond a simple typecast/passthrough).

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

EDIT added reference for MsgId handling

[skliper]

I'd rather thse be in a separate file if it's not intended for application use, since this file is pulled into the API by doxygen.

[jphickey]

Yeah, the problem is the same logic is needed by CFE_SB_MsgIdToValue/ValueToMsgId and these are inline functions within the same file, so the macro needs to be available.

By "application code' really I mean runtime -- the reason these need to exist in macro form is because one cannot use the inline function when initializing data at compile-time. I want to encourage use of the inline functions whenever possible because its more type safe, but occasionally one does need an expression that evaluates at compile-time, and its those times that the macro needs to be used instead.

[skliper]

Still could be in different file and just include, which provides a clear separation.

[jphickey]

But there is a possible valid case for using it externally, where one actually needs a value that evaluates at compile time.

The preference should always be to use the inline function, but there are some places (e.g. global variable init) where they don't work.

As there are no applications using this and the decision isn't made yet as to how MsgIds will be defined in the future, I suppose I could put this into a new file in the private/ subdir if that helps, but that seems like overkill to make a new file for just this.

But No matter how we define MID's in the future, I would think we'd need something that can be evaluated at compile time....

[jphickey]

What if the documentation was clarified to indicate "application runtime code" rather than just "application code"?

[skliper]

Eh, from a naive user point of view I wouldn't know the difference. No biggie, I just don't like huge header files to begin with, especially containing things that have different uses. Wouldn't have to be in private, just prefer separation.

[jphickey]

NOTE: commit 67eaf73 is only for a minor cleanup in unit test file sb_UT.c. Didn't realize this was re-reviewed since my last push, but it is largely the same thing. I had just mistakenly pushed to my fork a little earlier than I should have.

At any rate, this version basically assumes that the #define MID values will always be integers in nature, not necessarily CFE_SB_MsgId_t values. This means that the old switch statements can stay with minor update, because the case labels will be integers. However, it also means that the CFE_SB_ValueToMsgId function or CFE_SB_MSGID_WRAP_VALUE macro needs to be applied in a lot more places than it did in the previous proposal.

Nonetheless, this should be (hopefully) good to go now.

[skliper]

Split out exposing CFE_SB_IsValidMsgId.

[skliper]

I recommend changing the issue reference to #245 (partial fix) related to the MsgId changes, and at minimum a separate commit for #263

[jphickey]

Same basic changeset, in two commits now.

[astrogeco]

CCB 20200415 - APPROVED

[astrogeco]

@jphickey can you work with @dmknutsen and resolve the conflicts between this #578 and #635 ?

[jphickey]

Manually merged PR #635 and #602, resolved conflicts, and pushed to integration-candidate