Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #245, #1944, Message ID type improvements #1975

Merged
merged 3 commits into from
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/cfe_testcase/src/message_id_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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");
}
}
6 changes: 3 additions & 3 deletions modules/cfe_testcase/src/msg_api_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 1 addition & 6 deletions modules/cfe_testcase/src/sb_subscription_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/tbl_information_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/tbl_registration_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion modules/core_api/fsw/inc/cfe_sb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
/** @} */
Expand Down
24 changes: 20 additions & 4 deletions modules/core_api/fsw/inc/cfe_sb_api_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion modules/core_api/fsw/inc/cfe_sb_extern_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
2 changes: 1 addition & 1 deletion modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
15 changes: 8 additions & 7 deletions modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions modules/sb/ut-coverage/sb_UT.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down