[Group] Fix group messaging after test were disabled (#13505)

* Fix Group Messaging

* generated files

* zap generation error

* comment clean up

examples/chip-tool/commands/common/CommandInvoker.h

@@ -115,9 +115,11 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
         {
             return CHIP_ERROR_NO_MEMORY;
         }
-        ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
+        ReturnErrorOnFailure(commandSender->SendCommandRequest(session.Value()));
 
         commandSender.release();
+        exchangeManager->GetSessionManager()->RemoveGroupSession(session.Value()->AsGroupSession());
+
         return CHIP_NO_ERROR;
     }
 };

src/app/CommandHandler.cpp

@@ -436,7 +436,17 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()
 
 FabricIndex CommandHandler::GetAccessingFabricIndex() const
 {
-    return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
+    FabricIndex fabric = kUndefinedFabricIndex;
+    if (mpExchangeCtx->GetSessionHandle()->IsGroupSession())
+    {
+        fabric = mpExchangeCtx->GetSessionHandle()->AsGroupSession()->GetFabricIndex();
+    }
+    else
+    {
+        fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
+    }
+
+    return fabric;
 }
 
 CommandHandler * CommandHandler::Handle::Get()

src/app/WriteHandler.cpp

@@ -279,7 +279,17 @@ CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathPar
 
 FabricIndex WriteHandler::GetAccessingFabricIndex() const
 {
-    return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
+    FabricIndex fabric = kUndefinedFabricIndex;
+    if (mpExchangeCtx->GetSessionHandle()->IsGroupSession())
+    {
+        fabric = mpExchangeCtx->GetSessionHandle()->AsGroupSession()->GetFabricIndex();
+    }
+    else
+    {
+        fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
+    }
+
+    return fabric;
 }
 
 const char * WriteHandler::GetStateStr() const

src/app/server/Server.cpp

@@ -159,7 +159,7 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
 // for (iterate through all GroupDataProvider multicast Address)
 // {
 #ifdef CHIP_ENABLE_GROUP_MESSAGING_TESTS
-    err = mTransports.MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(1, 1234), true);
+    err = mTransports.MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(0, 1234), true);
     SuccessOrExit(err);
 #endif
     //}

src/app/tests/suites/TestGroupMessaging.yaml

@@ -12,6 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# !!!!!!!!!! TEST INFORMATION !!!!!!!!!!!!!!!!!!
+# This test file tests Group Multicast Messaging.
+# For this test to work, A Group Write/Command and a unicast read need to occur to be able to confirm that the group Communication works. Every test comes in a pair
+# Only Works on Linux machines
+# When building chip-tool, chip_enable_group_messaging_tests needs to be set to true in the build command for the test to pass
+# ./scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false is_tsan=true chip_enable_group_messaging_tests=true
 name: Basic Group Messaging Tests
 
 config:
@@ -26,41 +32,44 @@ tests:
     #TODO : Add Group membership command when implemented Issue #11077
     # - label: "Add device to Group"
     #   command: "TODO"
-
+    # Test Pair 1 : Sends a Group Write Attribute
     - label: "Group Write Attribute"
       command: "writeAttribute"
       attribute: "location"
       groupId: "1234"
       arguments:
           value: "us"
-    #Disabled due to issue-12983
+
+      # Test Pair 1 : Validates previous group write attribute with a unicast to read
     - label: "Read back Attribute"
-      disabled: true
       command: "readAttribute"
       attribute: "location"
       response:
           value: "us"
 
+    # Test Pair 2 : Sends a Group Write Attribute
     - label: "Restore initial location value"
       command: "writeAttribute"
       attribute: "location"
       groupId: "1234"
       arguments:
           value: ""
-    #Disabled due to issue-12983
+
+    # Test Pair 2 : Validates previous group write attribute with a unicast to read
     - label: "Read back Attribute"
-      disabled: true
       command: "readAttribute"
       attribute: "location"
       response:
           value: ""
 
+    # Test Pair 3 : Sends a Group command
     - label: "Turn On the light to see attribute change"
       cluster: "On/Off"
       command: "On"
       groupId: "1234"
       disabled: true
 
+      # Test Pair 3 : Validates previous group command with a unicast to read
     - label: "Check on/off attribute value is true after on command"
       cluster: "On/Off"
       command: "readAttribute"

src/controller/CHIPCluster.h

@@ -115,6 +115,7 @@ class DLL_EXPORT ClusterBase
                               WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
                               const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr)
     {
+        CHIP_ERROR err = CHIP_NO_ERROR;
         VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
         auto onSuccessCb = [context, successCb](const app::ConcreteAttributePath & commandPath) {
@@ -141,15 +142,17 @@ class DLL_EXPORT ClusterBase
 
         if (mGroupSession)
         {
-            return chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
-                                                              onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
+            err = chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
+                                                             onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
+            mDevice->GetExchangeManager()->GetSessionManager()->RemoveGroupSession(mGroupSession->AsGroupSession());
         }
         else
         {
-            return chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId,
-                                                              attributeId, requestData, onSuccessCb, onFailureCb,
-                                                              aTimedWriteTimeoutMs, onDoneCb);
+            err = chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId, attributeId,
+                                                             requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
         }
+
+        return err;
     }
 
     template <typename AttributeInfo>

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread_LwIP.cpp

@@ -261,6 +261,8 @@ void GenericThreadStackManagerImpl_OpenThread_LwIP<ImplClass>::UpdateThreadInter
                         it->Release();
                     }
                 }
+
+                err = transportMgr->MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(0, 1234), true);
             }
 #endif // CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_UDP
         }

src/transport/GroupSession.h

@@ -119,6 +119,13 @@ class GroupSessionTable
         }
     }
 
+    /**
+     * @brief Deletes an entry from the object pool
+     *
+     * @param entry
+     */
+    void DeleteEntry(GroupSession * entry) { mEntries.ReleaseObject(entry); }
+
 private:
     BitMapObjectPool<GroupSession, kMaxSessionCount> mEntries;
 };

src/transport/SessionManager.cpp

@@ -540,12 +540,6 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
         return; // malformed packet
     }
 
-    Optional<SessionHandle> session = FindGroupSession(packetHeader.GetDestinationGroupId().Value());
-    if (!session.HasValue())
-    {
-        return;
-    }
-
     if (msg.IsNull())
     {
         ChipLogError(Inet, "Secure transport received Groupcast NULL packet, discarding");
@@ -603,7 +597,12 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
 
     if (mCB != nullptr)
     {
+        Optional<SessionHandle> session = CreateGroupSession(packetHeader.GetDestinationGroupId().Value());
+        VerifyOrReturn(session.HasValue(), ChipLogError(Inet, "Error when creating group session handle."));
+
         mCB->OnMessageReceived(packetHeader, payloadHeader, session.Value(), peerAddress, isDuplicate, std::move(msg));
+
+        RemoveGroupSession(session.Value()->AsGroupSession());
     }
 }
 

src/transport/SessionManager.h

@@ -212,6 +212,7 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
     // TODO: implements group sessions
     Optional<SessionHandle> CreateGroupSession(GroupId group) { return mGroupSessions.AllocEntry(group, kUndefinedFabricIndex); }
     Optional<SessionHandle> FindGroupSession(GroupId group) { return mGroupSessions.FindEntry(group, kUndefinedFabricIndex); }
+    void RemoveGroupSession(Transport::GroupSession * session) { mGroupSessions.DeleteEntry(session); }
 
     // TODO: this is a temporary solution for legacy tests which use nodeId to send packets
     // and tv-casting-app that uses the TV's node ID to find the associated secure session