diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 09d4bc01241475..53a0e488cb0e6b 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -46,13 +46,6 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM mReportingEngine.Init(); - for (uint32_t index = 0; index < CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS - 1; index++) - { - mClusterInfoPool[index].mpNext = &mClusterInfoPool[index + 1]; - } - mClusterInfoPool[CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS - 1].mpNext = nullptr; - mpNextAvailableClusterInfo = mClusterInfoPool; - mMagic++; return CHIP_NO_ERROR; @@ -132,12 +125,7 @@ void InteractionModelEngine::Shutdown() mReportingEngine.Shutdown(); - for (uint32_t index = 0; index < CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS; index++) - { - mClusterInfoPool[index].mpNext = nullptr; - } - - mpNextAvailableClusterInfo = nullptr; + mClusterInfoPool.ReleaseAll(); mpExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id); } @@ -515,59 +503,31 @@ bool InteractionModelEngine::InActiveReadClientList(ReadClient * apReadClient) void InteractionModelEngine::ReleaseClusterInfoList(ClusterInfo *& aClusterInfo) { - ClusterInfo * lastClusterInfo = aClusterInfo; - if (lastClusterInfo == nullptr) + ClusterInfo * current = aClusterInfo; + while (current != nullptr) { - return; + ClusterInfo * next = current->mpNext; + mClusterInfoPool.ReleaseObject(current); + current = next; } - while (lastClusterInfo != nullptr && lastClusterInfo->mpNext != nullptr) - { - lastClusterInfo = lastClusterInfo->mpNext; - } - lastClusterInfo->mpNext = mpNextAvailableClusterInfo; - mpNextAvailableClusterInfo = aClusterInfo; - aClusterInfo = nullptr; + aClusterInfo = nullptr; } CHIP_ERROR InteractionModelEngine::PushFront(ClusterInfo *& aClusterInfoList, ClusterInfo & aClusterInfo) { - ClusterInfo * last = aClusterInfoList; - if (mpNextAvailableClusterInfo == nullptr) + ClusterInfo * clusterInfo = mClusterInfoPool.CreateObject(); + if (clusterInfo == nullptr) { ChipLogError(InteractionModel, "ClusterInfo pool full, cannot handle more entries!"); return CHIP_ERROR_NO_MEMORY; } - aClusterInfoList = mpNextAvailableClusterInfo; - mpNextAvailableClusterInfo = mpNextAvailableClusterInfo->mpNext; - *aClusterInfoList = aClusterInfo; - aClusterInfoList->mpNext = last; + *clusterInfo = aClusterInfo; + clusterInfo->mpNext = aClusterInfoList; + aClusterInfoList = clusterInfo; return CHIP_NO_ERROR; } -bool InteractionModelEngine::MergeOverlappedAttributePath(ClusterInfo * apAttributePathList, ClusterInfo & aAttributePath) -{ - ClusterInfo * runner = apAttributePathList; - while (runner != nullptr) - { - // If overlapped, we would skip this target path, - // --If targetPath is part of previous path, return true - // --If previous path is part of target path, update filedid and listindex and mflags with target path, return true - if (runner->IsAttributePathSupersetOf(aAttributePath)) - { - return true; - } - if (aAttributePath.IsAttributePathSupersetOf(*runner)) - { - runner->mListIndex = aAttributePath.mListIndex; - runner->mAttributeId = aAttributePath.mAttributeId; - return true; - } - runner = runner->mpNext; - } - return false; -} - bool InteractionModelEngine::IsOverlappedAttributePath(ClusterInfo & aAttributePath) { for (auto & handler : mReadHandlers) diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 04becfc3bbb200..e52c3c2ad85003 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -144,9 +144,6 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman void ReleaseClusterInfoList(ClusterInfo *& aClusterInfo); CHIP_ERROR PushFront(ClusterInfo *& aClusterInfoLisst, ClusterInfo & aClusterInfo); - // Merges aAttributePath inside apAttributePathList if current path is overlapped with existing path in apAttributePathList - // Overlap means the path is superset or subset of another path - bool MergeOverlappedAttributePath(ClusterInfo * apAttributePathList, ClusterInfo & aAttributePath); bool IsOverlappedAttributePath(ClusterInfo & aAttributePath); CHIP_ERROR RegisterCommandHandler(CommandHandlerInterface * handler); @@ -267,8 +264,7 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman WriteClient mWriteClients[CHIP_IM_MAX_NUM_WRITE_CLIENT]; WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER]; reporting::Engine mReportingEngine; - ClusterInfo mClusterInfoPool[CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS]; - ClusterInfo * mpNextAvailableClusterInfo = nullptr; + BitMapObjectPool mClusterInfoPool; ReadClient * mpActiveReadClientList = nullptr; diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index e432af81147aff..40de12cf41558e 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -44,8 +44,7 @@ void Engine::Shutdown() { mNumReportsInFlight = 0; mCurReadHandlerIdx = 0; - InteractionModelEngine::GetInstance()->ReleaseClusterInfoList(mpGlobalDirtySet); - mpGlobalDirtySet = nullptr; + mGlobalDirtySet.ReleaseAll(); } CHIP_ERROR @@ -96,14 +95,14 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu { bool concretePathDirty = false; // TODO: Optimize this implementation by making the iterator only emit intersected paths. - for (auto dirtyPath = mpGlobalDirtySet; dirtyPath != nullptr; dirtyPath = dirtyPath->mpNext) - { + mGlobalDirtySet.ForEachActiveObject([&](auto * dirtyPath) { if (dirtyPath->IsAttributePathSupersetOf(readPath)) { concretePathDirty = true; - break; + return Loop::Break; } - } + return Loop::Continue; + }); if (!concretePathDirty) { @@ -489,10 +488,27 @@ void Engine::Run() if (allReadClean) { - InteractionModelEngine::GetInstance()->ReleaseClusterInfoList(mpGlobalDirtySet); + mGlobalDirtySet.ReleaseAll(); } } +bool Engine::MergeOverlappedAttributePath(ClusterInfo & aAttributePath) +{ + return Loop::Break == mGlobalDirtySet.ForEachActiveObject([&](auto * path) { + if (path->IsAttributePathSupersetOf(aAttributePath)) + { + return Loop::Break; + } + if (aAttributePath.IsAttributePathSupersetOf(*path)) + { + path->mListIndex = aAttributePath.mListIndex; + path->mAttributeId = aAttributePath.mAttributeId; + return Loop::Break; + } + return Loop::Continue; + }); +} + CHIP_ERROR Engine::SetDirty(ClusterInfo & aClusterInfo) { for (auto & handler : InteractionModelEngine::GetInstance()->mReadHandlers) @@ -513,10 +529,16 @@ CHIP_ERROR Engine::SetDirty(ClusterInfo & aClusterInfo) } } } - if (!InteractionModelEngine::GetInstance()->MergeOverlappedAttributePath(mpGlobalDirtySet, aClusterInfo) && + if (!MergeOverlappedAttributePath(aClusterInfo) && InteractionModelEngine::GetInstance()->IsOverlappedAttributePath(aClusterInfo)) { - ReturnLogErrorOnFailure(InteractionModelEngine::GetInstance()->PushFront(mpGlobalDirtySet, aClusterInfo)); + ClusterInfo * clusterInfo = mGlobalDirtySet.CreateObject(); + if (clusterInfo == nullptr) + { + ChipLogError(DataManagement, "mGlobalDirtySet pool full, cannot handle more entries!"); + return CHIP_ERROR_NO_MEMORY; + } + *clusterInfo = aClusterInfo; } return CHIP_NO_ERROR; } @@ -535,17 +557,22 @@ void Engine::UpdateReadHandlerDirty(ReadHandler & aReadHandler) bool intersected = false; for (auto clusterInfo = aReadHandler.GetAttributeClusterInfolist(); clusterInfo != nullptr; clusterInfo = clusterInfo->mpNext) { - for (auto path = mpGlobalDirtySet; path != nullptr; path = path->mpNext) - { + mGlobalDirtySet.ForEachActiveObject([&](auto * path) { if (path->IsAttributePathSupersetOf(*clusterInfo) || clusterInfo->IsAttributePathSupersetOf(*path)) { intersected = true; - break; + return Loop::Break; } + return Loop::Continue; + }); + if (intersected) + { + break; } } if (!intersected) { + ChipLogDetail(InteractionModel, "clear read handler dirty in UpdateReadHandlerDirty!"); aReadHandler.ClearDirty(); } } diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index bc0bf6621ed173..5f06e11b9e70a3 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -135,6 +135,14 @@ class Engine CHIP_ERROR ScheduleBufferPressureEventDelivery(uint32_t aBytesWritten); void GetMinEventLogPosition(uint32_t & aMinLogPosition); + /** + * If the provided path is a superset of our of our existing paths, update that existing path to match the + * provided path. + * + * Return whether one of our paths is now a superset of the provided path. + */ + bool MergeOverlappedAttributePath(ClusterInfo & aAttributePath); + /** * Boolean to indicate if ScheduleRun is pending. This flag is used to prevent calling ScheduleRun multiple times * within the same execution context to avoid applying too much pressure on platforms that use small, fixed size event queues. @@ -155,13 +163,10 @@ class Engine uint32_t mCurReadHandlerIdx = 0; /** - * mpGlobalDirtySet is used to track the dirty cluster info application modified for attributes during - * post-subscription via SetDirty API, and further form the report. This reporting engine acquires this global dirty - * set from mClusterInfoPool managed by InteractionModelEngine, where all active read handlers also acquire the interested - * cluster Info list from mClusterInfoPool. + * mGlobalDirtySet is used to track the set of attribute/event paths marked dirty for reporting purposes. * */ - ClusterInfo * mpGlobalDirtySet = nullptr; + BitMapObjectPool mGlobalDirtySet; #if CONFIG_IM_BUILD_FOR_UNIT_TEST uint32_t mReservedSize = 0; diff --git a/src/app/tests/TestInteractionModelEngine.cpp b/src/app/tests/TestInteractionModelEngine.cpp index 81ea4662e2216c..75d035e871fed6 100644 --- a/src/app/tests/TestInteractionModelEngine.cpp +++ b/src/app/tests/TestInteractionModelEngine.cpp @@ -44,7 +44,6 @@ class TestInteractionModelEngine { public: static void TestClusterInfoPushRelease(nlTestSuite * apSuite, void * apContext); - static void TestMergeOverlappedAttributePath(nlTestSuite * apSuite, void * apContext); static int GetClusterInfoListLength(ClusterInfo * apClusterInfoList); }; @@ -90,37 +89,6 @@ void TestInteractionModelEngine::TestClusterInfoPushRelease(nlTestSuite * apSuit InteractionModelEngine::GetInstance()->ReleaseClusterInfoList(clusterInfoList); NL_TEST_ASSERT(apSuite, GetClusterInfoListLength(clusterInfoList) == 0); } - -void TestInteractionModelEngine::TestMergeOverlappedAttributePath(nlTestSuite * apSuite, void * apContext) -{ - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - err = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager(), nullptr); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - ClusterInfo clusterInfoList[2]; - - clusterInfoList[0].mAttributeId = 1; - clusterInfoList[1].mAttributeId = 2; - - { - chip::app::ClusterInfo testClusterInfo; - testClusterInfo.mAttributeId = 3; - NL_TEST_ASSERT(apSuite, - !InteractionModelEngine::GetInstance()->MergeOverlappedAttributePath(clusterInfoList, testClusterInfo)); - } - { - chip::app::ClusterInfo testClusterInfo; - NL_TEST_ASSERT(apSuite, - InteractionModelEngine::GetInstance()->MergeOverlappedAttributePath(clusterInfoList, testClusterInfo)); - } - { - chip::app::ClusterInfo testClusterInfo; - testClusterInfo.mAttributeId = 1; - testClusterInfo.mListIndex = 2; - NL_TEST_ASSERT(apSuite, - InteractionModelEngine::GetInstance()->MergeOverlappedAttributePath(clusterInfoList, testClusterInfo)); - } -} } // namespace app } // namespace chip @@ -130,7 +98,6 @@ namespace { const nlTest sTests[] = { NL_TEST_DEF("TestClusterInfoPushRelease", chip::app::TestInteractionModelEngine::TestClusterInfoPushRelease), - NL_TEST_DEF("TestMergeOverlappedAttributePath", chip::app::TestInteractionModelEngine::TestMergeOverlappedAttributePath), NL_TEST_SENTINEL() }; // clang-format on diff --git a/src/app/tests/TestReportingEngine.cpp b/src/app/tests/TestReportingEngine.cpp index 2cc46f2a8baf6c..04c1f3eb9f1839 100644 --- a/src/app/tests/TestReportingEngine.cpp +++ b/src/app/tests/TestReportingEngine.cpp @@ -52,6 +52,7 @@ class TestReportingEngine { public: static void TestBuildAndSendSingleReportData(nlTestSuite * apSuite, void * apContext); + static void TestMergeOverlappedAttributePath(nlTestSuite * apSuite, void * apContext); }; class TestExchangeDelegate : public Messaging::ExchangeDelegate @@ -107,6 +108,33 @@ void TestReportingEngine::TestBuildAndSendSingleReportData(nlTestSuite * apSuite readHandler.Shutdown(app::ReadHandler::ShutdownOptions::AbortCurrentExchange); } +void TestReportingEngine::TestMergeOverlappedAttributePath(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + err = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager(), nullptr); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ClusterInfo * clusterInfo = InteractionModelEngine::GetInstance()->GetReportingEngine().mGlobalDirtySet.CreateObject(); + clusterInfo->mAttributeId = 1; + + { + chip::app::ClusterInfo testClusterInfo; + testClusterInfo.mAttributeId = 3; + NL_TEST_ASSERT(apSuite, + !InteractionModelEngine::GetInstance()->GetReportingEngine().MergeOverlappedAttributePath(testClusterInfo)); + } + { + chip::app::ClusterInfo testClusterInfo; + testClusterInfo.mAttributeId = 1; + testClusterInfo.mListIndex = 2; + NL_TEST_ASSERT(apSuite, + InteractionModelEngine::GetInstance()->GetReportingEngine().MergeOverlappedAttributePath(testClusterInfo)); + } + + InteractionModelEngine::GetInstance()->GetReportingEngine().Shutdown(); +} + } // namespace reporting } // namespace app } // namespace chip @@ -116,6 +144,7 @@ namespace { const nlTest sTests[] = { NL_TEST_DEF("CheckBuildAndSendSingleReportData", chip::app::reporting::TestReportingEngine::TestBuildAndSendSingleReportData), + NL_TEST_DEF("TestMergeOverlappedAttributePath", chip::app::reporting::TestReportingEngine::TestMergeOverlappedAttributePath), NL_TEST_SENTINEL() }; // clang-format on diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index aa6edf837eeb1a..1ab3713963e480 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -2434,6 +2434,7 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; * * #CHIP_IM_MAX_NUM_READ_CLIENT * * #CHIP_IM_MAX_REPORTS_IN_FLIGHT * * #CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS + * * #CHIP_IM_SERVER_MAX_NUM_DIRTY_SET * * #CHIP_IM_MAX_NUM_WRITE_HANDLER * * #CHIP_IM_MAX_NUM_WRITE_CLIENT * * #CHIP_IM_MAX_NUM_TIMED_HANDLER @@ -2486,6 +2487,15 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; #define CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS 8 #endif +/** + * @def CHIP_IM_SERVER_MAX_NUM_DIRTY_SET + * + * @brief Defines the maximum number of dirty set, limits the number of attributes being read or subscribed at the same time. + */ +#ifndef CHIP_IM_SERVER_MAX_NUM_DIRTY_SET +#define CHIP_IM_SERVER_MAX_NUM_DIRTY_SET 8 +#endif + /** * @def CHIP_IM_MAX_NUM_WRITE_HANDLER *