Enable Write Group msg test (#11933)

project-chip · Jan 25, 2022 · 1117496 · 1117496
1 parent 21749a2
commit 1117496
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 17 deletions.
diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h
@@ -41,6 +41,10 @@ struct AttributePathParams
         AttributePathParams(aEndpointId, aClusterId, aAttributeId, ClusterInfo::kInvalidListIndex)
     {}
 
+    AttributePathParams(ClusterId aClusterId, AttributeId aAttributeId) :
+        AttributePathParams(kInvalidEndpointId, aClusterId, aAttributeId, ClusterInfo::kInvalidListIndex)
+    {}
+
     AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListIndex aListIndex) :
         mEndpointId(aEndpointId), mClusterId(aClusterId), mAttributeId(aAttributeId), mListIndex(aListIndex)
     {}
@@ -52,6 +56,7 @@ struct AttributePathParams
     bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); }
 
     /**
+     * SPEC 8.9.2.2
      * Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not
      * be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not
      * wildcard.

diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp
@@ -170,9 +170,7 @@ TLV::TLVWriter * WriteClient::GetAttributeDataIBTLVWriter()
 CHIP_ERROR WriteClient::ConstructAttributePath(const AttributePathParams & aAttributePathParams,
                                                AttributeDataIB::Builder aAttributeDataIB)
 {
-    // We do not support wildcard write now, reject them on client side.
-    VerifyOrReturnError(!aAttributePathParams.HasWildcard() && aAttributePathParams.IsValidAttributePath(),
-                        CHIP_ERROR_INVALID_PATH_LIST);
+    VerifyOrReturnError(aAttributePathParams.IsValidAttributePath(), CHIP_ERROR_INVALID_PATH_LIST);
     return aAttributePathParams.BuildAttributePath(aAttributeDataIB.CreatePath());
 }
 
@@ -258,13 +256,15 @@ CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::T
 exit:
     if (err != CHIP_NO_ERROR)
     {
+        ChipLogError(DataManagement, "Write client failed to SendWriteRequest");
         ClearExistingExchangeContext();
     }
 
     if (session.IsGroupSession())
     {
         // Always shutdown on Group communication
-        Shutdown();
+        ChipLogDetail(DataManagement, "Closing on group Communication ");
+        ShutdownInternal();
     }
 
     return err;

diff --git a/src/app/tests/suites/TestGroupMessaging.yaml b/src/app/tests/suites/TestGroupMessaging.yaml
@@ -26,7 +26,6 @@ tests:
     - label: "Group Write Attribute"
       command: "writeAttribute"
       attribute: "location"
-      disabled: true
       groupId: "1234"
       arguments:
           value: "us"

diff --git a/src/controller/WriteInteraction.h b/src/controller/WriteInteraction.h
@@ -99,19 +99,26 @@ CHIP_ERROR WriteAttribute(SessionHandle sessionHandle, chip::EndpointId endpoint
     ReturnErrorOnFailure(app::InteractionModelEngine::GetInstance()->NewWriteClient(handle, callback.get()));
     if (sessionHandle.IsGroupSession())
     {
-        // TODO : Issue #11604
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-        // ReturnErrorOnFailure(
-        // handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(clusterId, attributeId), requestData));
+        ReturnErrorOnFailure(
+            handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(clusterId, attributeId), requestData));
     }
     else
     {
         ReturnErrorOnFailure(
             handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(endpointId, clusterId, attributeId), requestData));
     }
+
     ReturnErrorOnFailure(handle.SendWriteRequest(sessionHandle));
 
     callback.release();
+
+    if (sessionHandle.IsGroupSession())
+    {
+        // Manually call success callback since OnReponse won't be called in WriteClient for group
+        app::ConcreteAttributePath aPath;
+        onSuccessCb(aPath);
+    }
+
     return CHIP_NO_ERROR;
 }
 

diff --git a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m
diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h
@@ -75,6 +75,7 @@ class SessionHandle
 
     NodeId GetPeerNodeId() const { return mPeerNodeId; }
     bool IsGroupSession() const { return mGroupId.HasValue(); }
+    const Optional<GroupId> & GetGroupId() const { return mGroupId; }
     const Optional<uint16_t> & GetPeerSessionId() const { return mPeerSessionId; }
     const Optional<uint16_t> & GetLocalSessionId() const { return mLocalSessionId; }
 

diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp
@@ -117,18 +117,39 @@ CHIP_ERROR SessionManager::PrepareMessage(SessionHandle sessionHandle, PayloadHe
 #endif // CHIP_PROGRESS_LOGGING
     if (sessionHandle.IsSecure())
     {
-        SecureSession * session = GetSecureSession(sessionHandle);
-        if (session == nullptr)
+        if (sessionHandle.IsGroupSession())
         {
-            return CHIP_ERROR_NOT_CONNECTED;
-        }
+            // TODO : #11911
+            // For now, just set the packetHeader with the correct data.
+            packetHeader.SetDestinationGroupId(sessionHandle.GetGroupId());
+            packetHeader.SetFlags(Header::SecFlagValues::kPrivacyFlag);
+            packetHeader.SetSessionType(Header::SessionType::kGroupSession);
+            // TODO : Replace the PeerNodeId with Our nodeId
+            packetHeader.SetSourceNodeId(sessionHandle.GetPeerNodeId());
+
+            if (!packetHeader.IsValidGroupMsg())
+            {
+                return CHIP_ERROR_INTERNAL;
+            }
 
-        MessageCounter & counter = GetSendCounterForPacket(payloadHeader, *session);
-        ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session, payloadHeader, packetHeader, message, counter));
+#if CHIP_PROGRESS_LOGGING
+            destination = sessionHandle.GetPeerNodeId();
+#endif // CHIP_PROGRESS_LOGGING
+        }
+        else
+        {
+            SecureSession * session = GetSecureSession(sessionHandle);
+            if (session == nullptr)
+            {
+                return CHIP_ERROR_NOT_CONNECTED;
+            }
+            MessageCounter & counter = GetSendCounterForPacket(payloadHeader, *session);
+            ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session, payloadHeader, packetHeader, message, counter));
 
 #if CHIP_PROGRESS_LOGGING
-        destination = session->GetPeerNodeId();
+            destination = session->GetPeerNodeId();
 #endif // CHIP_PROGRESS_LOGGING
+        }
     }
     else
     {

diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h