Skip to content

Commit

Permalink
Fix mSelfProcessedEvents init/cleanup bug (#12829)
Browse files Browse the repository at this point in the history
  • Loading branch information
yunhanw-google authored and pull[bot] committed Apr 3, 2023
1 parent 286d2a7 commit 4303697
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 16 deletions.
3 changes: 1 addition & 2 deletions src/app/EventManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,6 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD
{
EventLoadOutContext * const loadOutContext = static_cast<EventLoadOutContext *>(apContext);
CHIP_ERROR err = EventIterator(aReader, aDepth, loadOutContext);
loadOutContext->mCurrentEventNumber++;
if (err == CHIP_EVENT_ID_FOUND)
{
// checkpoint the writer
Expand All @@ -693,7 +692,7 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD
loadOutContext->mFirst = false;
loadOutContext->mEventCount++;
}

loadOutContext->mCurrentEventNumber++;
return err;
}

Expand Down
28 changes: 19 additions & 9 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ CHIP_ERROR ReadHandler::Init(Messaging::ExchangeManager * apExchangeMgr, Interac
mpAttributeClusterInfoList = nullptr;
mpEventClusterInfoList = nullptr;
mCurrentPriority = PriorityLevel::Invalid;
mIsPrimingReports = true;
for (size_t index = 0; index < kNumPriorityLevel; index++)
{
mSelfProcessedEvents[index] = 0;
mLastScheduledEventNumber[index] = 0;
}
mIsPrimingReports = true;
MoveToState(HandlerState::Initialized);
mpDelegate = apDelegate;
mSubscriptionId = 0;
Expand Down Expand Up @@ -103,14 +108,19 @@ void ReadHandler::Shutdown(ShutdownOptions aOptions)
mpAttributeClusterInfoList = nullptr;
mpEventClusterInfoList = nullptr;
mCurrentPriority = PriorityLevel::Invalid;
mIsPrimingReports = false;
mpDelegate = nullptr;
mHoldReport = false;
mDirty = false;
mActiveSubscription = false;
mIsChunkedReport = false;
mInitiatorNodeId = kUndefinedNodeId;
mHoldSync = false;
for (size_t index = 0; index < kNumPriorityLevel; index++)
{
mSelfProcessedEvents[index] = 0;
mLastScheduledEventNumber[index] = 0;
}
mIsPrimingReports = false;
mpDelegate = nullptr;
mHoldReport = false;
mDirty = false;
mActiveSubscription = false;
mIsChunkedReport = false;
mInitiatorNodeId = kUndefinedNodeId;
mHoldSync = false;
}

CHIP_ERROR ReadHandler::OnReadInitialRequest(System::PacketBufferHandle && aPayload)
Expand Down
8 changes: 4 additions & 4 deletions src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,12 @@ class ReadHandler : public Messaging::ExchangeDelegate
PriorityLevel mCurrentPriority = PriorityLevel::Invalid;

// The event number of the last processed event for each priority level
EventNumber mSelfProcessedEvents[kNumPriorityLevel];
EventNumber mSelfProcessedEvents[kNumPriorityLevel] = { 0 };

// The last schedule event number snapshoted in the beginning when preparing to fill new events to reports
EventNumber mLastScheduledEventNumber[kNumPriorityLevel];
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
InteractionModelDelegate * mpDelegate = nullptr;
EventNumber mLastScheduledEventNumber[kNumPriorityLevel] = { 0 };
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
InteractionModelDelegate * mpDelegate = nullptr;

// Tracks whether we're in the initial phase of receiving priming
// reports, which is always true for reads and true for subscriptions
Expand Down
17 changes: 16 additions & 1 deletion src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont
readPrepareParams.mEventPathParamsListSize = 1;
readPrepareParams.mpAttributePathParamsList = attributePathParams;
readPrepareParams.mAttributePathParamsListSize = 2;

err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, &delegate);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

Expand All @@ -703,6 +704,18 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont
NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2);
NL_TEST_ASSERT(apSuite, delegate.mGotReport);
NL_TEST_ASSERT(apSuite, !delegate.mReadError);

delegate.mGotEventResponse = false;
delegate.mNumAttributeResponse = 0;
delegate.mGotReport = false;
err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, &delegate);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
InteractionModelEngine::GetInstance()->GetReportingEngine().Run();
NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse);
NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2);
NL_TEST_ASSERT(apSuite, delegate.mGotReport);
NL_TEST_ASSERT(apSuite, !delegate.mReadError);

// By now we should have closed all exchanges and sent all pending acks, so
// there should be no queued-up things in the retransmit table.
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);
Expand Down Expand Up @@ -1029,6 +1042,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
delegate.mGotReport = false;
engine->GetReportingEngine().Run();
NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse);
NL_TEST_ASSERT(apSuite, delegate.mGotReport);
NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2);
NL_TEST_ASSERT(apSuite, delegate.mNumSubscriptions == 1);
Expand Down Expand Up @@ -1124,7 +1138,8 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a
engine->GetReportingEngine().Run();
NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0);

// Test multiple subscriptipn
// Test multiple subscription
delegate.mGotEventResponse = false;
delegate.mNumAttributeResponse = 0;
delegate.mGotReport = false;
ReadPrepareParams readPrepareParams1(ctx.GetSessionBobToAlice());
Expand Down

0 comments on commit 4303697

Please sign in to comment.