Skip to content

Commit

Permalink
Fix #1419, Resolve sequence count auto-increment rollover bug
Browse files Browse the repository at this point in the history
  • Loading branch information
@skliper
skliper committed May 4, 2021
1 parent c8b5e00 commit 0a0b9cb
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 18 deletions.
16 changes: 16 additions & 0 deletions modules/core_api/fsw/inc/cfe_msg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,22 @@ CFE_Status_t CFE_MSG_GetSequenceCount(const CFE_MSG_Message_t *MsgPtr, CFE_MSG_S
*/
CFE_Status_t CFE_MSG_SetSequenceCount(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt);

/*****************************************************************************/
/**
* \brief Gets the next sequence count value (rolls over if appropriate)
*
* \par Description
* Abstract method to get the next valid sequence count value.
* Will roll over to zero for any input value greater than or
* equal to the maximum possible sequence count value given
* the field in the header.
*
* \param[in] SeqCnt Sequence count
*
* \return The next valid sequence count value
*/
CFE_MSG_SequenceCount_t CFE_MSG_GetNextSequenceCount(CFE_MSG_SequenceCount_t SeqCnt);

/*****************************************************************************/
/**
* \brief Gets the message EDS version
Expand Down
12 changes: 12 additions & 0 deletions modules/core_api/ut-stubs/src/ut_msg_stubs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,3 +703,15 @@ int32 CFE_MSG_ValidateChecksum(const CFE_MSG_Message_t *MsgPtr, bool *IsValid)

return status;
}

/*
* -----------------------------------------------------------
* Stub implementation of CFE_MSG_GetNextSequenceCount
* -----------------------------------------------------------
*/
CFE_MSG_SequenceCount_t CFE_MSG_GetNextSequenceCount(CFE_MSG_SequenceCount_t SeqCnt)
{
UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_MSG_GetNextSequenceCount), SeqCnt);

return UT_DEFAULT_IMPL(CFE_MSG_GetNextSequenceCount);
}
15 changes: 15 additions & 0 deletions modules/msg/fsw/src/cfe_msg_ccsdspri.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,21 @@ int32 CFE_MSG_SetSequenceCount(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_
return CFE_SUCCESS;
}

/******************************************************************************
* Get next sequence count - See API and header file for details
*/
CFE_MSG_SequenceCount_t CFE_MSG_GetNextSequenceCount(CFE_MSG_SequenceCount_t SeqCnt)
{
SeqCnt++;

if (SeqCnt > CFE_MSG_SEQCNT_MASK)
{
SeqCnt = 0;
}

return SeqCnt;
}

/******************************************************************************
* Get message size - See API and header file for details
*/
Expand Down
19 changes: 14 additions & 5 deletions modules/msg/ut-coverage/test_cfe_msg_ccsdspri.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,13 @@ void Test_MSG_SegmentationFlag(void)

void Test_MSG_SequenceCount(void)
{
CFE_MSG_Message_t msg;
CFE_MSG_ApId_t input[] = {0, TEST_SEQUENCE_MAX / 2, TEST_SEQUENCE_MAX};
CFE_MSG_ApId_t actual = TEST_SEQUENCE_MAX;
int i;
CFE_MSG_Message_t msg;
const CFE_MSG_SequenceCount_t input[] = {0, TEST_SEQUENCE_MAX / 2, TEST_SEQUENCE_MAX};
CFE_MSG_SequenceCount_t actual = TEST_SEQUENCE_MAX;
CFE_MSG_SequenceCount_t maxsc;
int i;

memset(&maxsc, 0xFF, sizeof(maxsc));

UtPrintf("Bad parameter tests, Null pointers and invalid (max valid + 1, max)");
memset(&msg, 0, sizeof(msg));
Expand All @@ -410,7 +413,7 @@ void Test_MSG_SequenceCount(void)
ASSERT_EQ(CFE_MSG_SetSequenceCount(NULL, input[0]), CFE_MSG_BAD_ARGUMENT);
ASSERT_EQ(CFE_MSG_SetSequenceCount(&msg, TEST_SEQUENCE_MAX + 1), CFE_MSG_BAD_ARGUMENT);
ASSERT_EQ(Test_MSG_NotZero(&msg), 0);
ASSERT_EQ(CFE_MSG_SetSequenceCount(&msg, 0xFFFF), CFE_MSG_BAD_ARGUMENT);
ASSERT_EQ(CFE_MSG_SetSequenceCount(&msg, maxsc), CFE_MSG_BAD_ARGUMENT);
ASSERT_EQ(Test_MSG_NotZero(&msg), 0);

UtPrintf("Set to all F's, various valid inputs");
Expand Down Expand Up @@ -452,6 +455,12 @@ void Test_MSG_SequenceCount(void)
ASSERT_EQ(Test_MSG_NotZero(&msg), MSG_SEQUENCE_FLAG);
}
}

UtPrintf("Fully exercise getting next sequence count");
ASSERT_EQ(CFE_MSG_GetNextSequenceCount(0), 1);
ASSERT_EQ(CFE_MSG_GetNextSequenceCount(TEST_SEQUENCE_MAX / 2), (TEST_SEQUENCE_MAX / 2) + 1);
ASSERT_EQ(CFE_MSG_GetNextSequenceCount(TEST_SEQUENCE_MAX), 0);
ASSERT_EQ(CFE_MSG_GetNextSequenceCount(maxsc), 0);
}

/*
Expand Down
25 changes: 21 additions & 4 deletions modules/sb/ut-coverage/sb_UT.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2820,7 +2820,7 @@ void Test_TransmitMsg_BasicSend(void)

} /* end Test_TransmitMsg_BasicSend */

/* Sequence count hook */
/* Set sequence count hook */
static int32 UT_CheckSetSequenceCount(void *UserObj, int32 StubRetcode, uint32 CallCount,
const UT_StubContext_t *Context)
{
Expand All @@ -2843,54 +2843,70 @@ void Test_TransmitMsg_SequenceCount(void)
CFE_MSG_Type_t Type = CFE_MSG_Type_Tlm;
uint32 PipeDepth = 10;
CFE_MSG_SequenceCount_t SeqCnt;
CFE_MSG_SequenceCount_t SeqCntExpected;

/* Set up hook for checking CFE_MSG_SetSequenceCount calls */
UT_SetHookFunction(UT_KEY(CFE_MSG_SetSequenceCount), UT_CheckSetSequenceCount, &SeqCnt);

SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "SeqCntTestPipe"));
SETUP(CFE_SB_Subscribe(MsgId, PipeId));

/* Note the Sequence count value doesn't really matter, just set unique to confirm use */
SeqCntExpected = 1;
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), SeqCntExpected);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);

SETUP(CFE_SB_TransmitMsg(&TlmPkt.Hdr.Msg, true));
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_SetSequenceCount)), 1);
ASSERT_EQ(SeqCnt, 1);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 1);
ASSERT_EQ(SeqCnt, SeqCntExpected);

UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);
ASSERT(CFE_SB_TransmitMsg(&TlmPkt.Hdr.Msg, false));

/* Assert sequence count wasn't set */
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 1);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_SetSequenceCount)), 1);

SeqCntExpected = 2;
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), SeqCntExpected);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);
ASSERT(CFE_SB_TransmitMsg(&TlmPkt.Hdr.Msg, true));
ASSERT_EQ(SeqCnt, 2);
ASSERT_EQ(SeqCnt, SeqCntExpected);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_SetSequenceCount)), 2);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 2);

EVTCNT(2);
EVTSENT(CFE_SB_SUBSCRIPTION_RCVD_EID);

SETUP(CFE_SB_Unsubscribe(MsgId, PipeId)); /* should have no subscribers now */

SeqCntExpected = 3;
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), SeqCntExpected);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);
SETUP(CFE_SB_TransmitMsg(&TlmPkt.Hdr.Msg, true)); /* increment to 3 */
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_SetSequenceCount)), 3);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 3);

SETUP(CFE_SB_Subscribe(MsgId, PipeId)); /* resubscribe so we can receive a msg */

SeqCntExpected = 4;
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), SeqCntExpected);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);
SETUP(CFE_SB_TransmitMsg(&TlmPkt.Hdr.Msg, true)); /* increment to 4 */
ASSERT_EQ(SeqCnt, 4);
ASSERT_EQ(SeqCnt, SeqCntExpected);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_SetSequenceCount)), 4);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 4);

TEARDOWN(CFE_SB_DeletePipe(PipeId));

Expand Down Expand Up @@ -3135,6 +3151,7 @@ void Test_TransmitBuffer_IncrementSeqCnt(void)
UtAssert_Failed("Unexpected NULL pointer returned from ZeroCopyGetPtr");
}

UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), 1);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &MsgId, sizeof(MsgId), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &Size, sizeof(Size), false);
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false);
Expand Down
5 changes: 4 additions & 1 deletion modules/sbr/fsw/src/cfe_sbr_route_unsorted.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <string.h>

#include "cfe_sb.h"
#include "cfe_msg.h"

/******************************************************************************
* Type Definitions
Expand Down Expand Up @@ -158,9 +159,11 @@ void CFE_SBR_SetDestListHeadPtr(CFE_SBR_RouteId_t RouteId, CFE_SB_DestinationD_t
*/
void CFE_SBR_IncrementSequenceCounter(CFE_SBR_RouteId_t RouteId)
{
CFE_MSG_SequenceCount_t *cnt = &CFE_SBR_RDATA.RoutingTbl[CFE_SBR_RouteIdToValue(RouteId)].SeqCnt;

if (CFE_SBR_IsValidRouteId(RouteId))
{
CFE_SBR_RDATA.RoutingTbl[CFE_SBR_RouteIdToValue(RouteId)].SeqCnt++;
*cnt = CFE_MSG_GetNextSequenceCount(*cnt);
}
}

Expand Down
21 changes: 13 additions & 8 deletions modules/sbr/ut-coverage/test_cfe_sbr_route_unsorted.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,13 @@ void Test_SBR_Route_Unsort_General(void)
void Test_SBR_Route_Unsort_GetSet(void)
{

CFE_SB_RouteId_Atom_t routeidx;
CFE_SB_MsgId_t msgid[3];
CFE_SBR_RouteId_t routeid[3];
CFE_SB_DestinationD_t dest[2];
uint32 count;
uint32 i;
CFE_SB_RouteId_Atom_t routeidx;
CFE_SB_MsgId_t msgid[3];
CFE_SBR_RouteId_t routeid[3];
CFE_SB_DestinationD_t dest[2];
CFE_MSG_SequenceCount_t seqcntexpected[] = {1, 2};
uint32 count;
uint32 i;

UtPrintf("Invalid route ID checks");
routeid[0] = CFE_SBR_INVALID_ROUTE_ID;
Expand Down Expand Up @@ -167,20 +168,24 @@ void Test_SBR_Route_Unsort_GetSet(void)
}

/* Check the msgid matches and increment a sequence counter */
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), seqcntexpected[0]);
for (i = 0; i < 3; i++)
{
ASSERT_TRUE(CFE_SB_MsgId_Equal(msgid[i], CFE_SBR_GetMsgId(routeid[i])));
CFE_SBR_IncrementSequenceCounter(routeid[0]);
}
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 3);

/* Increment route 1 once and set dest pointers */
UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetNextSequenceCount), seqcntexpected[1]);
CFE_SBR_IncrementSequenceCounter(routeid[1]);
ASSERT_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_GetNextSequenceCount)), 4);
CFE_SBR_SetDestListHeadPtr(routeid[1], &dest[1]);
CFE_SBR_SetDestListHeadPtr(routeid[2], &dest[0]);

UtPrintf("Verify remaining set values");
ASSERT_EQ(CFE_SBR_GetSequenceCounter(routeid[0]), 3);
ASSERT_EQ(CFE_SBR_GetSequenceCounter(routeid[1]), 1);
ASSERT_EQ(CFE_SBR_GetSequenceCounter(routeid[0]), seqcntexpected[0]);
ASSERT_EQ(CFE_SBR_GetSequenceCounter(routeid[1]), seqcntexpected[1]);
ASSERT_EQ(CFE_SBR_GetSequenceCounter(routeid[2]), 0);
UtAssert_ADDRESS_EQ(CFE_SBR_GetDestListHeadPtr(routeid[0]), NULL);
UtAssert_ADDRESS_EQ(CFE_SBR_GetDestListHeadPtr(routeid[1]), &dest[1]);
Expand Down

0 comments on commit 0a0b9cb

Please sign in to comment.