Remove duplicate path in IM Read (#9389)

Summary of Changes: --Remove duplicate attribute paths if there are multiple overlapped path in read attribute path for IM Read --Add unit test for multiple overlapped path Restyled by clang-format Restyled by clang-format
project-chip · Sep 30, 2021 · 1741461 · 1741461
1 parent 604864f
commit 1741461
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 8 deletions.
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -284,10 +284,17 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathList::Parser & aAt
         }
         SuccessOrExit(err);
 
-        err = InteractionModelEngine::GetInstance()->PushFront(mpAttributeClusterInfoList, clusterInfo);
-        SuccessOrExit(err);
-        mpAttributeClusterInfoList->SetDirty();
-        SetInitialReport();
+        if (MergeOverlappedAttributePath(clusterInfo))
+        {
+            continue;
+        }
+        else
+        {
+            err = InteractionModelEngine::GetInstance()->PushFront(mpAttributeClusterInfoList, clusterInfo);
+            SuccessOrExit(err);
+            mpAttributeClusterInfoList->SetDirty();
+            SetInitialReport();
+        }
     }
     // if we have exhausted this container
     if (CHIP_END_OF_TLV == err)
@@ -300,6 +307,31 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathList::Parser & aAt
     return err;
 }
 
+bool ReadHandler::MergeOverlappedAttributePath(ClusterInfo & aAttributePath)
+{
+    ClusterInfo * runner = mpAttributeClusterInfoList;
+    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->mFieldId   = aAttributePath.mFieldId;
+            runner->mFlags     = aAttributePath.mFlags;
+            runner->SetDirty();
+            return true;
+        }
+        runner = runner->mpNext;
+    }
+    return false;
+}
+
 CHIP_ERROR ReadHandler::ProcessEventPathList(EventPathList::Parser & aEventPathListParser)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;

diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h
@@ -143,6 +143,9 @@ class ReadHandler : public Messaging::ExchangeDelegate
 
     const char * GetStateStr() const;
 
+    // Merges aAttributePath inside the existing internal mpAttributeClusterInfoList
+    bool MergeOverlappedAttributePath(ClusterInfo & aAttributePath);
+
     Messaging::ExchangeContext * mpExchangeCtx = nullptr;
 
     // Don't need the response for report data if true

diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
@@ -156,7 +156,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
     void OnReportData(const chip::app::ReadClient * apReadClient, const chip::app::ClusterInfo & aPath,
                       chip::TLV::TLVReader * apData, chip::Protocols::InteractionModel::ProtocolCode status) override
     {
-        mGotAttributeResponse = true;
+        mNumAttributeResponse++;
     }
 
     CHIP_ERROR ReportProcessed(const chip::app::ReadClient * apReadClient) override
@@ -175,7 +175,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
     }
 
     bool mGotEventResponse      = false;
-    bool mGotAttributeResponse  = false;
+    int mNumAttributeResponse   = 0;
     bool mGotReport             = false;
     bool mGotReadStatusResponse = false;
 };
@@ -630,18 +630,26 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont
     attributePathParams[0].mListIndex  = 0;
     attributePathParams[0].mFlags.Set(chip::app::AttributePathParams::Flags::kFieldIdValid);
 
+    attributePathParams[1].mNodeId     = chip::kTestDeviceNodeId;
+    attributePathParams[1].mEndpointId = kTestEndpointId;
+    attributePathParams[1].mClusterId  = kTestClusterId;
+    attributePathParams[1].mFieldId    = 1;
+    attributePathParams[1].mListIndex  = 1;
+    attributePathParams[1].mFlags.Set(chip::app::AttributePathParams::Flags::kFieldIdValid);
+    attributePathParams[1].mFlags.Set(chip::app::AttributePathParams::Flags::kListIndexValid);
+
     ReadPrepareParams readPrepareParams;
     readPrepareParams.mSessionHandle               = ctx.GetSessionLocalToPeer();
     readPrepareParams.mpEventPathParamsList        = eventPathParams;
     readPrepareParams.mEventPathParamsListSize     = 2;
     readPrepareParams.mpAttributePathParamsList    = attributePathParams;
-    readPrepareParams.mAttributePathParamsListSize = 1;
+    readPrepareParams.mAttributePathParamsListSize = 2;
     err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
     InteractionModelEngine::GetInstance()->GetReportingEngine().Run();
     NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse);
-    NL_TEST_ASSERT(apSuite, delegate.mGotAttributeResponse);
+    NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1);
     NL_TEST_ASSERT(apSuite, delegate.mGotReport);
 
     // By now we should have closed all exchanges and sent all pending acks, so