From 2cc09d8c7fae46a8edabc54097f6c85b879fb657 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 22 Oct 2022 08:38:17 -0400 Subject: [PATCH] Fix delivery of urgent events to actually work correctly. (#23240) (#23271) * Fix delivery of urgent events to actually work correctly. If an urgent event was emitted at a point when the ReadHandler subscribing for it had already gotten its "min interval has elapsed" callback, we would just mark the read handler dirty but not schedule a run of the reporting engine. This would cause us to not report the event until something _did_ trigger such a run (either the max interval being reached, or some other reading/reporting activity). The fix is to make sure ReadHandler always schedules a run when IsReportable() becomes true. * Address review comments. * Make TestSubscribeUrgentWildcardEvent slightly less random-failure-prone. --- src/app/ReadHandler.cpp | 62 ++++++++---- src/app/ReadHandler.h | 11 +- src/app/reporting/Engine.cpp | 10 +- src/app/reporting/Engine.h | 4 + src/app/tests/TestReadInteraction.cpp | 138 ++++++++++++++++++++------ 5 files changed, 166 insertions(+), 59 deletions(-) diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 64bcbb1a7e516b..7fd2436d645f87 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -51,7 +51,7 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon mLastWrittenEventsBytes = 0; mTransactionStartGeneration = InteractionModelEngine::GetInstance()->GetReportingEngine().GetDirtySetGeneration(); mFlags.ClearAll(); - mFlags.Set(ReadHandlerFlags::PrimingReports, true); + SetStateFlag(ReadHandlerFlags::PrimingReports); mSessionHandle.Grab(mExchangeCtx->GetSessionHandle()); } @@ -115,7 +115,7 @@ void ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) else { // Force us to be in a dirty state so we get processed by the reporting - mFlags.Set(ReadHandlerFlags::ForceDirty); + SetStateFlag(ReadHandlerFlags::ForceDirty); } } @@ -142,7 +142,7 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange { err = SendSubscribeResponse(); - mFlags.Set(ReadHandlerFlags::ActiveSubscription); + SetStateFlag(ReadHandlerFlags::ActiveSubscription); auto * appCallback = mManagementCallback.GetAppCallback(); if (appCallback) @@ -216,7 +216,7 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b { mCurrentReportsBeginGeneration = InteractionModelEngine::GetInstance()->GetReportingEngine().GetDirtySetGeneration(); } - mFlags.Set(ReadHandlerFlags::ChunkedReport, aMoreChunks); + SetStateFlag(ReadHandlerFlags::ChunkedReport, aMoreChunks); bool noResponseExpected = IsType(InteractionType::Read) && !aMoreChunks; if (!noResponseExpected) { @@ -351,7 +351,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa bool isFabricFiltered; ReturnErrorOnFailure(readRequestParser.GetIsFabricFiltered(&isFabricFiltered)); - mFlags.Set(ReadHandlerFlags::FabricFiltered, isFabricFiltered); + SetStateFlag(ReadHandlerFlags::FabricFiltered, isFabricFiltered); ReturnErrorOnFailure(readRequestParser.ExitContainer()); MoveToState(HandlerState::GeneratingReports); @@ -493,17 +493,17 @@ void ReadHandler::MoveToState(const HandlerState aTargetState) InteractionModelEngine::GetInstance()->GetReportingEngine().OnReportConfirm(); } + mState = aTargetState; + ChipLogDetail(DataManagement, "IM RH moving to [%s]", GetStateStr()); + // // If we just unblocked sending reports, let's go ahead and schedule the reporting // engine to run to kick that off. // - if (aTargetState == HandlerState::GeneratingReports) + if (aTargetState == HandlerState::GeneratingReports && IsReportable()) { InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } - - mState = aTargetState; - ChipLogDetail(DataManagement, "IM RH moving to [%s]", GetStateStr()); } bool ReadHandler::CheckEventClean(EventManagement & aEventManager) @@ -546,7 +546,7 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() ReturnErrorOnFailure(RefreshSubscribeSyncTimer()); - mFlags.Set(ReadHandlerFlags::PrimingReports, false); + ClearStateFlag(ReadHandlerFlags::PrimingReports); return mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeResponse, std::move(packet)); } @@ -627,7 +627,7 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP bool isFabricFiltered; ReturnErrorOnFailure(subscribeRequestParser.GetIsFabricFiltered(&isFabricFiltered)); - mFlags.Set(ReadHandlerFlags::FabricFiltered, isFabricFiltered); + SetStateFlag(ReadHandlerFlags::FabricFiltered, isFabricFiltered); ReturnErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast(&mSubscriptionId), sizeof(mSubscriptionId))); ReturnErrorOnFailure(subscribeRequestParser.ExitContainer()); MoveToState(HandlerState::GeneratingReports); @@ -642,11 +642,7 @@ void ReadHandler::OnUnblockHoldReportCallback(System::Layer * apSystemLayer, voi VerifyOrReturn(apAppState != nullptr); ReadHandler * readHandler = static_cast(apAppState); ChipLogDetail(DataManagement, "Unblock report hold after min %d seconds", readHandler->mMinIntervalFloorSeconds); - readHandler->mFlags.Set(ReadHandlerFlags::HoldReport, false); - if (readHandler->IsDirty()) - { - InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); - } + readHandler->ClearStateFlag(ReadHandlerFlags::HoldReport); InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( System::Clock::Seconds16(readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds), OnRefreshSubscribeTimerSyncCallback, readHandler); @@ -656,10 +652,9 @@ void ReadHandler::OnRefreshSubscribeTimerSyncCallback(System::Layer * apSystemLa { VerifyOrReturn(apAppState != nullptr); ReadHandler * readHandler = static_cast(apAppState); - readHandler->mFlags.Set(ReadHandlerFlags::HoldSync, false); + readHandler->ClearStateFlag(ReadHandlerFlags::HoldSync); ChipLogProgress(DataManagement, "Refresh subscribe timer sync after %d seconds", readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds); - InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } CHIP_ERROR ReadHandler::RefreshSubscribeSyncTimer() @@ -673,8 +668,8 @@ CHIP_ERROR ReadHandler::RefreshSubscribeSyncTimer() { ChipLogProgress(DataManagement, "Refresh Subscribe Sync Timer with min %d seconds and max %d seconds", mMinIntervalFloorSeconds, mMaxInterval); - mFlags.Set(ReadHandlerFlags::HoldReport); - mFlags.Set(ReadHandlerFlags::HoldSync); + SetStateFlag(ReadHandlerFlags::HoldReport); + SetStateFlag(ReadHandlerFlags::HoldSync); ReturnErrorOnFailure( InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( System::Clock::Seconds16(mMinIntervalFloorSeconds), OnUnblockHoldReportCallback, this)); @@ -717,6 +712,11 @@ void ReadHandler::SetDirty(const AttributePathParams & aAttributeChanged) mAttributePathExpandIterator.ResetCurrentCluster(); mAttributeEncoderState = AttributeValueEncoder::AttributeEncodeState(); } + + if (IsReportable()) + { + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } } Transport::SecureSession * ReadHandler::GetSession() const @@ -727,5 +727,27 @@ Transport::SecureSession * ReadHandler::GetSession() const } return mSessionHandle->AsSecureSession(); } + +void ReadHandler::UnblockUrgentEventDelivery() +{ + SetStateFlag(ReadHandlerFlags::ForceDirty); +} + +void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue) +{ + bool oldReportable = IsReportable(); + mFlags.Set(aFlag, aValue); + // If we became reportable, schedule a reporting run. + if (!oldReportable && IsReportable()) + { + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } +} + +void ReadHandler::ClearStateFlag(ReadHandlerFlags aFlag) +{ + SetStateFlag(aFlag, false); +} + } // namespace app } // namespace chip diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 7e32fa920a2636..cf0ee7ab50b706 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -263,6 +263,9 @@ class ReadHandler : public Messaging::ExchangeDelegate bool IsIdle() const { return mState == HandlerState::Idle; } bool IsReportable() const { + // Important: Anything that changes the state IsReportable depends on in + // a way that causes IsReportable to become true must call ScheduleRun + // on the reporting engine. return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::HoldReport) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::HoldSync)); } @@ -299,7 +302,7 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() { mFlags.Clear(ReadHandlerFlags::ForceDirty); } + void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -317,7 +320,7 @@ class ReadHandler : public Messaging::ExchangeDelegate auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } - void UnblockUrgentEventDelivery() { mFlags.Set(ReadHandlerFlags::ForceDirty); } + void UnblockUrgentEventDelivery(); const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } @@ -374,6 +377,10 @@ class ReadHandler : public Messaging::ExchangeDelegate const char * GetStateStr() const; + // Helpers for managing our state flags properly. + void SetStateFlag(ReadHandlerFlags aFlag, bool aValue = true); + void ClearStateFlag(ReadHandlerFlags aFlag); + AttributePathExpandIterator mAttributePathExpandIterator = AttributePathExpandIterator(nullptr); // The current generation of the reporting engine dirty set the last time we were notified that a path we're interested in was diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index b1c2f5a3db54ea..e4dcdfc99b012e 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -838,14 +838,6 @@ CHIP_ERROR Engine::SetDirty(AttributePathParams & aAttributePath) } ReturnErrorOnFailure(InsertPathIntoDirtySet(aAttributePath)); - // Schedule work to run asynchronously on the CHIP thread. The scheduled - // work won't execute until the current execution context has - // completed. This ensures that we can 'gather up' multiple attribute - // changes that have occurred in the same execution context without - // requiring any explicit 'start' or 'end' change calls into the engine to - // book-end the change. - ScheduleRun(); - return CHIP_NO_ERROR; } @@ -938,7 +930,7 @@ CHIP_ERROR Engine::ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBy if (isUrgentEvent) { - ChipLogDetail(DataManagement, "urgent event would be sent after min interval"); + ChipLogDetail(DataManagement, "Urgent event will be sent once reporting is not blocked by the min interval"); return CHIP_NO_ERROR; } diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index 45754b490ee7c8..e0f81f4474397c 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -39,6 +39,9 @@ namespace chip { namespace app { + +class TestReadInteraction; + namespace reporting { /* * @class Engine @@ -138,6 +141,7 @@ class Engine void Run(); friend class TestReportingEngine; + friend class ::chip::app::TestReadInteraction; struct AttributePathParamsWithGeneration : public AttributePathParams { diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 44ba49c400a4b9..5ce4a3c33407c5 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -1632,6 +1632,11 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; + // TODO: Fix + // https://github.com/project-chip/connectedhomeip/issues/23260 so this + // test is testing what it thinks it's testing. + // Make sure the reporting engine actually runs. + ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); @@ -1656,10 +1661,12 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); MockInteractionModelApp delegate; + MockInteractionModelApp nonUrgentDelegate; auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); chip::app::EventPathParams eventPathParams[2]; @@ -1673,26 +1680,22 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite readPrepareParams.mEventPathParamsListSize = 2; - chip::app::AttributePathParams attributePathParams[2]; - readPrepareParams.mpAttributePathParamsList = attributePathParams; - readPrepareParams.mpAttributePathParamsList[0].mEndpointId = kTestEndpointId; - readPrepareParams.mpAttributePathParamsList[0].mClusterId = kTestClusterId; - readPrepareParams.mpAttributePathParamsList[0].mAttributeId = 1; - - readPrepareParams.mpAttributePathParamsList[1].mEndpointId = kTestEndpointId; - readPrepareParams.mpAttributePathParamsList[1].mClusterId = kTestClusterId; - readPrepareParams.mpAttributePathParamsList[1].mAttributeId = 2; - - readPrepareParams.mAttributePathParamsListSize = 2; + readPrepareParams.mpAttributePathParamsList = nullptr; + readPrepareParams.mAttributePathParamsListSize = 0; readPrepareParams.mMinIntervalFloorSeconds = 2; - readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mMaxIntervalCeilingSeconds = 3600; printf("\nSend first subscribe request message with wildcard urgent event to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId); - delegate.mNumAttributeResponse = 0; - readPrepareParams.mKeepSubscriptions = false; + readPrepareParams.mKeepSubscriptions = true; { + app::ReadClient nonUrgentReadClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), + nonUrgentDelegate, chip::app::ReadClient::InteractionType::Subscribe); + nonUrgentDelegate.mGotReport = false; + err = nonUrgentReadClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, chip::app::ReadClient::InteractionType::Subscribe); readPrepareParams.mpEventPathParamsList[0].mIsUrgentEvent = true; @@ -1704,47 +1707,122 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 1); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 2); NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); - delegate.mpReadHandler = engine->ActiveHandlerAt(0); + nonUrgentDelegate.mpReadHandler = engine->ActiveHandlerAt(0); + NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(1) != nullptr); + delegate.mpReadHandler = engine->ActiveHandlerAt(1); NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); - NL_TEST_ASSERT(apSuite, delegate.mGotReport); - NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); - NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1); + NL_TEST_ASSERT(apSuite, nonUrgentDelegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 2); GenerateEvents(apSuite, apContext); + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); - NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsDirty() == true); + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsDirty()); delegate.mGotEventResponse = false; delegate.mGotReport = false; - // wait for min interval 2 seconds(in test, we use 1.9second considering the time variation), expect no event is received, - // then wait for 0.5 seconds, then the urgent event would be sent out + NL_TEST_ASSERT(apSuite, nonUrgentDelegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); + nonUrgentDelegate.mGotEventResponse = false; + nonUrgentDelegate.mGotReport = false; + + // wait for min interval 2 seconds (in test, we use 1.6 seconds considering the time variation), expect no event is + // received, then wait for 0.8 seconds, then the urgent event would be sent out // currently DriveIOUntil will call `DriveIO` at least once, which means that if there is any CPU scheduling issues, // there's a chance 1.9s will already have elapsed by the time we get there, which will result in DriveIO being called when // it shouldn't. Better fix could happen inside DriveIOUntil, not sure the sideeffect there. while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(1900)) + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(1600)) { break; } ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed } - NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse != true); + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); startTime = System::SystemClock().GetMonotonicTimestamp(); while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(500)) + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(800)) { break; } ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed } - NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse == true); + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); + + // Since we just sent a report for our urgent subscription, we should have our min interval timer + // running again. + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); + NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsDirty()); + delegate.mGotEventResponse = false; + + // For our non-urgent subscription, we did not send anything, so we + // should not have a min interval timer running there. + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); + + // Wait for the min interval timer to fire. + startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(2100)) + { + break; + } + ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed + } + + // No reporting should have happened. + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); + + // min-interval timer should have fired, and our handler should still + // not be dirty or even reportable. + NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); + NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsDirty()); + NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsReportable()); + + // And the non-urgent one should not have changed state either, since + // it's waiting for the max-interval. + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->mFlags.Has(ReadHandler::ReadHandlerFlags::HoldReport)); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsReportable()); + + // There should be no reporting run scheduled. This is very important; + // otherwise we can get a false-positive pass below because the run was + // already scheduled by here. + NL_TEST_ASSERT(apSuite, !InteractionModelEngine::GetInstance()->GetReportingEngine().mRunScheduled); + + // Generate some events, which should get reported. + GenerateEvents(apSuite, apContext); + + // Urgent read handler should now be dirty, and reportable. + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsDirty()); + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsReportable()); + + // Non-urgent read handler should not be reportable. + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsReportable()); + + // Still no reporting should have happened. + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); + + ctx.DrainAndServiceIO(); + + // Should get those urgent events reported. + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); + + // Should get nothing reported on the non-urgent handler. + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); } // By now we should have closed all exchanges and sent all pending acks, so @@ -2110,6 +2188,8 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); delegate.mpReadHandler = engine->ActiveHandlerAt(0); + // TODO: This bit is not testing anything, because removing that flag + // manually like this will not cause the reporting engine to run! delegate.mpReadHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldReport, false); ctx.DrainAndServiceIO(); @@ -2594,9 +2674,10 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - err = engine->GetReportingEngine().SetDirty(dirtyPath1); delegate.mpReadHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldReport, false); delegate.mpReadHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldSync, false); + err = engine->GetReportingEngine().SetDirty(dirtyPath1); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; @@ -2695,9 +2776,10 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - err = engine->GetReportingEngine().SetDirty(dirtyPath1); delegate.mpReadHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldReport, false); delegate.mpReadHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldSync, false); + err = engine->GetReportingEngine().SetDirty(dirtyPath1); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0;