Merge puill request #1975 from jphickey/fix-245-caelum-opaque-msgid

Fix #245, #1944, Message ID type improvements

modules/cfe_testcase/src/message_id_test.c

@@ -42,7 +42,7 @@ void TestMsgId(void)
 
     UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, expectedmsgid), CFE_SUCCESS);
     UtAssert_INT32_EQ(CFE_MSG_GetMsgId(&msg, &msgid), CFE_SUCCESS);
-    UtAssert_UINT32_EQ(msgid, expectedmsgid);
+    CFE_Assert_MSGID_EQ(msgid, expectedmsgid);
 
     UtAssert_INT32_EQ(CFE_MSG_SetMsgId(NULL, msgid), CFE_MSG_BAD_ARGUMENT);
 
@@ -93,4 +93,4 @@ void MessageIdTestSetup(void)
 {
     UtTest_Add(TestMsgId, NULL, NULL, "Test Set/Get Message ID");
     UtTest_Add(TestGetTypeFromMsgId, NULL, NULL, "Test Get Type From Message ID");
-}
+}

modules/cfe_testcase/src/msg_api_test.c

@@ -50,12 +50,12 @@ void TestMsgApiBasic(void)
     bool                    _returned = false;
 
     memset(&cmd, 0xFF, sizeof(cmd));
-    msgId = CFE_SB_ValueToMsgId(0);
+    msgId = CFE_SB_ValueToMsgId(1);
 
     /* test msg-init */
-    UtAssert_INT32_EQ(CFE_MSG_Init(NULL, CFE_SB_ValueToMsgId(0), sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
+    UtAssert_INT32_EQ(CFE_MSG_Init(NULL, msgId, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
     UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, msgId, 0), CFE_MSG_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1, sizeof(cmd)),
+    UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(cmd)),
                       CFE_MSG_BAD_ARGUMENT);
 
     UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, msgId, sizeof(cmd)), CFE_SUCCESS);

modules/cfe_testcase/src/sb_subscription_test.c

@@ -54,7 +54,6 @@ void TestSubscribeUnsubscribe(void)
 
     /* Subscribe - Confirm Bad MsgId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT);
 
     /* Subscribe - Confirm Bad PipeId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT);
@@ -73,7 +72,6 @@ void TestSubscribeUnsubscribe(void)
 
     /* Unsubscribe - Confirm Bad MsgId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT);
 
     /* Unsubscribe - Confirm Bad PipeId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT);
@@ -108,7 +106,6 @@ void TestSubscribeUnsubscribeLocal(void)
 
     /* Subscribe - Confirm Bad MsgId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_SB_INVALID_MSG_ID, PipeId1, 2), CFE_SB_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_SB_MSGID_RESERVED, PipeId2, 2), CFE_SB_BAD_ARGUMENT);
 
     /* Subscribe - Confirm Bad PipeId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE, 2), CFE_SB_BAD_ARGUMENT);
@@ -127,7 +124,6 @@ void TestSubscribeUnsubscribeLocal(void)
 
     /* Unsubscribe - Confirm Bad MsgId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT);
 
     /* Unsubscribe - Confirm Bad PipeId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT);
@@ -174,7 +170,6 @@ void TestSubscribeEx(void)
 
     /* Subscribe - Confirm Bad MsgId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_SB_INVALID_MSG_ID, PipeId1, CFE_SB_DEFAULT_QOS, 2), CFE_SB_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_SB_MSGID_RESERVED, PipeId2, CFE_SB_DEFAULT_QOS, 2), CFE_SB_BAD_ARGUMENT);
 
     /* Subscribe - Confirm Bad PipeId Arg Rejection */
     UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE, CFE_SB_DEFAULT_QOS, 2),
@@ -214,7 +209,7 @@ void TestSBMaxSubscriptions(void)
     while (NumSubs <= CFE_PLATFORM_SB_MAX_MSG_IDS)
     {
         /* fabricate a msgid to subscribe to (this may overlap real msgids) */
-        TestMsgId = CFE_SB_MSGID_WRAP_VALUE(CFE_PLATFORM_CMD_MID_BASE + NumSubs);
+        TestMsgId = CFE_SB_ValueToMsgId(CFE_PLATFORM_CMD_MID_BASE + NumSubs);
 
         Status = CFE_SB_Subscribe(TestMsgId, PipeId);
         if (Status != CFE_SUCCESS)

modules/cfe_testcase/src/tbl_information_test.c

@@ -78,7 +78,7 @@ void TestNotifyByMessage(void)
     UtPrintf("Testing: CFE_TBL_NotifyByMessage");
     CFE_TBL_Handle_t  SharedTblHandle;
     const char *      SharedTblName = "SAMPLE_APP.SampleAppTable";
-    CFE_SB_MsgId_t    TestMsgId     = CFE_TEST_CMD_MID;
+    CFE_SB_MsgId_t    TestMsgId     = CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID);
     CFE_MSG_FcnCode_t TestCmdCode   = 0;
     uint32            TestParameter = 0;
 

modules/cfe_testcase/src/tbl_registration_test.c

@@ -220,7 +220,7 @@ void TestTblNonAppContext(void)
                       CFE_ES_ERR_RESOURCEID_NOT_VALID);
     UtAssert_INT32_EQ(CFE_TBL_Manage(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID);
     UtAssert_INT32_EQ(CFE_TBL_Modified(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID);
-    UtAssert_INT32_EQ(CFE_TBL_NotifyByMessage(CFE_FT_Global.TblHandle, CFE_TEST_CMD_MID, 0, 0),
+    UtAssert_INT32_EQ(CFE_TBL_NotifyByMessage(CFE_FT_Global.TblHandle, CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID), 0, 0),
                       CFE_ES_ERR_RESOURCEID_NOT_VALID);
     UtAssert_INT32_EQ(CFE_TBL_ReleaseAddress(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID);
     UtAssert_INT32_EQ(CFE_TBL_Share(&Handle, CFE_FT_Global.TblName), CFE_ES_ERR_RESOURCEID_NOT_VALID);

modules/core_api/fsw/inc/cfe_sb.h

@@ -827,7 +827,7 @@ static inline CFE_SB_MsgId_Atom_t CFE_SB_MsgIdToValue(CFE_SB_MsgId_t MsgId)
  */
 static inline CFE_SB_MsgId_t CFE_SB_ValueToMsgId(CFE_SB_MsgId_Atom_t MsgIdValue)
 {
-    CFE_SB_MsgId_t Result = CFE_SB_MSGID_WRAP_VALUE(MsgIdValue);
+    CFE_SB_MsgId_t Result = CFE_SB_MSGID_C(MsgIdValue);
     return Result;
 }
 /** @} */

modules/core_api/fsw/inc/cfe_sb_api_typedefs.h

@@ -63,7 +63,23 @@
  *
  * \sa CFE_SB_ValueToMsgId()
  */
-#define CFE_SB_MSGID_WRAP_VALUE(val) ((CFE_SB_MsgId_t)(val))
+#define CFE_SB_MSGID_WRAP_VALUE(val) \
+    {                                \
+        val                          \
+    }
+
+/**
+ * \brief Translation macro to convert to MsgId integer values from a literal
+ *
+ * This ensures that the literal is interpreted as the CFE_SB_MsgId_t type, rather
+ * than the default type associated with that literal (e.g. int/unsigned int).
+ *
+ * \note Due to constraints in C99 this style of initializer can only be used
+ * at runtime, not for static/compile-time initializers.
+ *
+ * \sa CFE_SB_ValueToMsgId()
+ */
+#define CFE_SB_MSGID_C(val) ((CFE_SB_MsgId_t)CFE_SB_MSGID_WRAP_VALUE(val))
 
 /**
  * \brief Translation macro to convert to MsgId integer values from opaque/abstract API values
@@ -75,15 +91,15 @@
  *
  * \sa CFE_SB_MsgIdToValue()
  */
-#define CFE_SB_MSGID_UNWRAP_VALUE(mid) ((CFE_SB_MsgId_Atom_t)(mid))
+#define CFE_SB_MSGID_UNWRAP_VALUE(mid) ((mid).Value)
 
 /**
  * \brief Reserved value for CFE_SB_MsgId_t that will not match any valid MsgId
  *
  * This rvalue macro can be used for static/compile-time data initialization to ensure that
  * the initialized value does not alias to a valid MsgId object.
  */
-#define CFE_SB_MSGID_RESERVED CFE_SB_MSGID_WRAP_VALUE(-1)
+#define CFE_SB_MSGID_RESERVED CFE_SB_MSGID_WRAP_VALUE(0)
 
 /**
  * \brief A literal of the CFE_SB_MsgId_t type representing an invalid ID
@@ -96,7 +112,7 @@
  * purposes (rvalue), #CFE_SB_MSGID_RESERVED should be used instead.
  * However, in the current implementation, they are equivalent.
  */
-#define CFE_SB_INVALID_MSG_ID CFE_SB_MSGID_RESERVED
+#define CFE_SB_INVALID_MSG_ID CFE_SB_MSGID_C(0)
 
 /**
  * \brief Cast/Convert a generic CFE_ResourceId_t to a CFE_SB_PipeId_t

modules/core_api/fsw/inc/cfe_sb_extern_typedefs.h

@@ -115,7 +115,10 @@ typedef uint32 CFE_SB_MsgId_Atom_t;
  * @note In a future version it could become a type-safe wrapper similar to the route index,
  * to avoid message IDs getting mixed between other integer values.
  */
-typedef CFE_SB_MsgId_Atom_t CFE_SB_MsgId_t;
+typedef struct
+{
+    CFE_SB_MsgId_Atom_t Value;
+} CFE_SB_MsgId_t;
 
 /** \brief  CFE_SB_PipeId_t to primitive type definition
  *

modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c

@@ -46,7 +46,7 @@ void Test_MSG_GetTypeFromMsgId(void)
     UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
 
     UtPrintf("Bad parameter tests, Invalid message ID");
-    UtAssert_INT32_EQ(CFE_MSG_GetTypeFromMsgId(CFE_SB_INVALID_MSG_ID, &actual), CFE_MSG_BAD_ARGUMENT);
+    UtAssert_INT32_EQ(CFE_MSG_GetTypeFromMsgId(CFE_SB_ValueToMsgId(-1), &actual), CFE_MSG_BAD_ARGUMENT);
 
     UtPrintf("Set to all F's, test cmd and tlm");
     memset(&msg, 0xFF, sizeof(msg));

modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c

@@ -48,28 +48,29 @@ void Test_MSG_MsgId(void)
     UtAssert_INT32_EQ(CFE_MSG_GetMsgId(&msg, NULL), CFE_MSG_BAD_ARGUMENT);
     UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
     UtAssert_INT32_EQ(CFE_MSG_SetMsgId(NULL, msgid), CFE_MSG_BAD_ARGUMENT);
-    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_INVALID_MSG_ID), CFE_MSG_BAD_ARGUMENT);
+    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(-1)), CFE_MSG_BAD_ARGUMENT);
     UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
-    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), CFE_MSG_BAD_ARGUMENT);
+    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1)),
+                      CFE_MSG_BAD_ARGUMENT);
     UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
-    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, 0xFFFF), CFE_MSG_BAD_ARGUMENT);
+    UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(0xFFFF)), CFE_MSG_BAD_ARGUMENT);
     UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
 
-    UtPrintf("Set msg to all F's, set msgid to 0 and verify");
+    UtPrintf("Set msg to all F's, set msgid to 1 and verify");
     memset(&msg, 0xFF, sizeof(msg));
     CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
     UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0xFFFF);
-    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, 0));
+    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(1)));
     UT_DisplayPkt(&msg, sizeof(msg));
     CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
-    UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0);
+    UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 1);
     UtAssert_INT32_EQ(Test_MSG_NotF(&msg), MSG_HDRVER_FLAG | MSG_APID_FLAG | MSG_TYPE_FLAG | MSG_HASSEC_FLAG);
 
     UtPrintf("Set msg to all 0, set msgid to max and verify");
     memset(&msg, 0, sizeof(msg));
     CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
     UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0);
-    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID));
+    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID)));
     UT_DisplayPkt(&msg, sizeof(msg));
     CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
     UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), CFE_PLATFORM_SB_HIGHEST_VALID_MSGID);

modules/sb/ut-coverage/sb_UT.c

@@ -91,7 +91,7 @@ const CFE_SB_MsgId_t SB_UT_LAST_VALID_MID = CFE_SB_MSGID_WRAP_VALUE(CFE_PLATFORM
  * This is a  "borderline" value to test the limits of the validity checking
  * The specific value depends on how MsgId is actually defined internally
  */
-const CFE_SB_MsgId_t SB_UT_FIRST_VALID_MID = CFE_SB_MSGID_WRAP_VALUE(0);
+const CFE_SB_MsgId_t SB_UT_FIRST_VALID_MID = CFE_SB_MSGID_WRAP_VALUE(1);
 
 /*
  * A MsgId value which is in the middle of the valid range
@@ -1480,15 +1480,15 @@ void Test_SB_Cmds_SendPrevSubs(void)
     NumEvts = 2; /* one for each pipe create */
 
     /* Two full pkts to be sent plus five entries in a partial pkt, skipping MSGID 0x0D */
-    for (i = 0; i < CFE_SB_SUB_ENTRIES_PER_PKT * 2 + 5; i++)
+    for (i = 1; i < (CFE_SB_SUB_ENTRIES_PER_PKT * 2) + 6; i++)
     {
         /* Skip subscribing to ALLSUBS mid. This is the one we are testing.
          * MsgID for this in CCSDS v.1 was 0x180d so this MID did not appear in the
          * SB sub list. This results in multiple NO_SUBS_EID sent which we are not
          * testing here so it is irrelevant
          * For CCSDS v.2 CFE_SB_ALLSUBS_TLM_MID (0x0d) now appears in the
          * SB subscription list and thus we must skip adding 0x0D to the list
-         * as we were going from MSGID 0-45 (0x00-0x2D)
+         * as we were going from MSGID 1-46 (0x01-0x2E)
          * */
         if (i != CFE_SB_ALLSUBS_TLM_MID)
         {
@@ -1533,7 +1533,7 @@ void Test_SB_Cmds_SendPrevSubs(void)
     /* Round out the number to three full pkts in order to test branch path
      * coverage, MSGID 0x0D was skipped in previous subscription loop
      */
-    for (; i < CFE_SB_SUB_ENTRIES_PER_PKT * 3; i++)
+    for (; i < (CFE_SB_SUB_ENTRIES_PER_PKT * 3) + 1; i++)
     {
         CFE_UtAssert_SETUP(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(i), PipeId1));
         NumEvts += 1;
@@ -2571,18 +2571,18 @@ void Test_Subscribe_MaxMsgIdCount(void)
     {
         if (i < CFE_PLATFORM_SB_MAX_MSG_IDS)
         {
-            CFE_UtAssert_SETUP(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(i), PipeId2));
+            CFE_UtAssert_SETUP(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(1 + i), PipeId2));
         }
         else
         {
-            UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(i), PipeId2), CFE_SB_MAX_MSGS_MET);
+            UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(1 + i), PipeId2), CFE_SB_MAX_MSGS_MET);
         }
     }
 
     UtAssert_UINT32_EQ(CFE_SB_Global.StatTlmMsg.Payload.PeakMsgIdsInUse, CFE_PLATFORM_SB_MAX_MSG_IDS);
     CFE_UtAssert_EVENTSENT(CFE_SB_MAX_MSGS_MET_EID);
 
-    CFE_UtAssert_SUCCESS(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(0), PipeId1));
+    CFE_UtAssert_SUCCESS(CFE_SB_Subscribe(CFE_SB_ValueToMsgId(1), PipeId1));
     UtAssert_UINT32_EQ(CFE_SB_Global.StatTlmMsg.Payload.PeakMsgIdsInUse, CFE_PLATFORM_SB_MAX_MSG_IDS);
 
     CFE_UtAssert_TEARDOWN(CFE_SB_DeletePipe(PipeId0));