Remove node id from outgoing group session (project-chip#16718)

* Remove node id from outgoing group session

* Return an error when group fabric is not valid

examples/chip-tool/commands/clusters/ClusterCommand.h

@@ -60,9 +60,9 @@ class ClusterCommand : public ModelCommand, public chip::app::CommandSender::Cal
         return ClusterCommand::SendCommand(device, endpointIds.at(0), mClusterId, mCommandId, mPayload);
     }
 
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId) override
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex) override
     {
-        return ClusterCommand::SendGroupCommand(groupId, fabricIndex, senderNodeId, mClusterId, mCommandId, mPayload);
+        return ClusterCommand::SendGroupCommand(groupId, fabricIndex, mClusterId, mCommandId, mPayload);
     }
 
     /////////// CommandSender Callback Interface /////////
@@ -117,8 +117,8 @@ class ClusterCommand : public ModelCommand, public chip::app::CommandSender::Cal
     }
 
     template <class T>
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId,
-                                chip::ClusterId clusterId, chip::CommandId commandId, const T & value)
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::ClusterId clusterId,
+                                chip::CommandId commandId, const T & value)
     {
         chip::app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, clusterId, commandId,
                                                      (chip::app::CommandPathFlags::kGroupIdValid) };
@@ -130,7 +130,7 @@ class ClusterCommand : public ModelCommand, public chip::app::CommandSender::Cal
         VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);
         ReturnErrorOnFailure(commandSender->AddRequestDataNoTimedCheck(commandPath, value, mTimedInteractionTimeoutMs));
 
-        chip::Transport::OutgoingGroupSession session(groupId, fabricIndex, senderNodeId);
+        chip::Transport::OutgoingGroupSession session(groupId, fabricIndex);
         ReturnErrorOnFailure(commandSender->SendGroupCommandRequest(chip::SessionHandle(session)));
         commandSender.release();
 

examples/chip-tool/commands/clusters/ModelCommand.cpp

@@ -32,7 +32,7 @@ CHIP_ERROR ModelCommand::RunCommand()
         ReturnErrorOnFailure(CurrentCommissioner().GetFabricIndex(&fabricIndex));
         ChipLogProgress(chipTool, "Sending command to group 0x%" PRIx16, GroupIdFromNodeId(mNodeId));
 
-        return SendGroupCommand(GroupIdFromNodeId(mNodeId), fabricIndex, CurrentCommissioner().GetNodeId());
+        return SendGroupCommand(GroupIdFromNodeId(mNodeId), fabricIndex);
     }
 
     ChipLogProgress(chipTool, "Sending command to node 0x%" PRIx64, mNodeId);

examples/chip-tool/commands/clusters/ModelCommand.h

@@ -45,10 +45,7 @@ class ModelCommand : public CHIPCommand
 
     virtual CHIP_ERROR SendCommand(ChipDevice * device, std::vector<chip::EndpointId> endPointIds) = 0;
 
-    virtual CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId)
-    {
-        return CHIP_ERROR_BAD_REQUEST;
-    };
+    virtual CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex) { return CHIP_ERROR_BAD_REQUEST; };
 
     void Shutdown() override
     {

examples/chip-tool/commands/clusters/WriteAttributeCommand.h

@@ -63,9 +63,9 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
         return WriteAttribute::SendCommand(device, endpointIds.at(0), mClusterId, mAttributeId, mAttributeValue);
     }
 
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId) override
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex) override
     {
-        return WriteAttribute::SendGroupCommand(groupId, fabricIndex, senderNodeId, mClusterId, mAttributeId, mAttributeValue);
+        return WriteAttribute::SendGroupCommand(groupId, fabricIndex, mClusterId, mAttributeId, mAttributeValue);
     }
 
     /////////// WriteClient Callback Interface /////////
@@ -115,8 +115,8 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
     }
 
     template <class T>
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId,
-                                chip::ClusterId clusterId, chip::AttributeId attributeId, const T & value)
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::ClusterId clusterId,
+                                chip::AttributeId attributeId, const T & value)
     {
 
         chip::app::AttributePathParams attributePathParams;
@@ -132,7 +132,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
         VerifyOrReturnError(writeClient != nullptr, CHIP_ERROR_NO_MEMORY);
         ReturnErrorOnFailure(writeClient->EncodeAttribute(attributePathParams, value, mDataVersion));
 
-        chip::Transport::OutgoingGroupSession session(groupId, fabricIndex, senderNodeId);
+        chip::Transport::OutgoingGroupSession session(groupId, fabricIndex);
         ReturnErrorOnFailure(writeClient->SendWriteRequest(chip::SessionHandle(session)));
         writeClient.release();
 

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

@@ -107,7 +107,7 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
     }
 
     CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, FabricIndex fabric, GroupId groupId,
-                                  NodeId sourceNodeId, const RequestType & aRequestData)
+                                  const RequestType & aRequestData)
     {
         app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, RequestType::GetClusterId(), RequestType::GetCommandId(),
                                                (app::CommandPathFlags::kGroupIdValid) };
@@ -117,7 +117,7 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
 
         ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));
 
-        Transport::OutgoingGroupSession session(groupId, fabric, sourceNodeId);
+        Transport::OutgoingGroupSession session(groupId, fabric);
 
         // this (invoker) and commandSender will be deleted by the onDone call before the return of SendGroupCommandRequest
         // this (invoker) should not be used after the SendGroupCommandRequest call
@@ -230,8 +230,7 @@ CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext,
     //
     //  We assume the aDevice already has a Case session which is way we can use he established Secure Session
     ReturnErrorOnFailure(invoker->InvokeGroupCommand(aDevice->GetExchangeManager(),
-                                                     aDevice->GetSecureSession().Value()->GetFabricIndex(), groupId,
-                                                     aDevice->GetDeviceId(), aRequestData));
+                                                     aDevice->GetSecureSession().Value()->GetFabricIndex(), groupId, aRequestData));
 
     //  invoker is already deleted and is not to be used
     invoker.release();

examples/chip-tool/templates/commands.zapt

@@ -45,11 +45,11 @@ public:
         return ClusterCommand::SendCommand(device, endpointIds.at(0), {{asHex parent.code 8}}, {{asHex code 8}}, mRequest);
     }
 
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId) override
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex) override
     {
         ChipLogProgress(chipTool, "Sending cluster ({{asHex parent.code 8}}) command ({{asHex code 8}}) on Group %" PRIu16, groupId);
 
-        return ClusterCommand::SendGroupCommand(groupId, fabricIndex, senderNodeId, {{asHex parent.code 8}}, {{asHex code 8}}, mRequest);
+        return ClusterCommand::SendGroupCommand(groupId, fabricIndex, {{asHex parent.code 8}}, {{asHex code 8}}, mRequest);
     }
 
 private:
@@ -88,9 +88,9 @@ public:
         return WriteAttribute::SendCommand(device, endpointIds.at(0), {{asHex parent.code 8}}, {{asHex code 8}}, mValue);
     }
 
-    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex, chip::NodeId senderNodeId) override
+    CHIP_ERROR SendGroupCommand(chip::GroupId groupId, chip::FabricIndex fabricIndex) override
     {
-        return WriteAttribute::SendGroupCommand(groupId, fabricIndex, senderNodeId, {{asHex parent.code 8}}, {{asHex code 8}}, mValue);
+        return WriteAttribute::SendGroupCommand(groupId, fabricIndex, {{asHex parent.code 8}}, {{asHex code 8}}, mValue);
     }
 
 private:

examples/chip-tool/templates/tests/partials/test_cluster.zapt

@@ -288,7 +288,7 @@ class {{filename}}Suite: public TestCommand
 
         {{#if isWriteAttribute}}
         {{#if isGroupCommand}}
-          ReturnErrorOnFailure(cluster.WriteAttribute<chip::app::Clusters::{{asUpperCamelCase cluster}}::Attributes::{{asUpperCamelCase attribute}}::TypeInfo>(groupId, {{>device}}->GetSecureSession().Value()->GetFabricIndex(), {{>device}}->GetDeviceId(),{{#chip_tests_item_parameters}}{{asLowerCamelCase name}}Argument, {{/chip_tests_item_parameters}}this, {{>staticSuccessResponse}}, {{>staticFailureResponse}}
+          ReturnErrorOnFailure(cluster.WriteAttribute<chip::app::Clusters::{{asUpperCamelCase cluster}}::Attributes::{{asUpperCamelCase attribute}}::TypeInfo>(groupId, {{>device}}->GetSecureSession().Value()->GetFabricIndex(), {{#chip_tests_item_parameters}}{{asLowerCamelCase name}}Argument, {{/chip_tests_item_parameters}}this, {{>staticSuccessResponse}}, {{>staticFailureResponse}}
             {{~> maybeTimedInteractionTimeout ~}}
             , {{>staticDoneResponse}}
             ));

examples/light-switch-app/efr32/src/binding-handler.cpp

@@ -85,25 +85,24 @@ void ProcessOnOffUnicastBindingCommand(CommandId commandId, const EmberBindingTa
 
 void ProcessOnOffGroupBindingCommand(CommandId commandId, const EmberBindingTableEntry & binding)
 {
-    NodeId sourceNodeId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(binding.fabricIndex)->GetNodeId();
     Messaging::ExchangeManager & exchangeMgr = Server::GetInstance().GetExchangeManager();
 
     switch (commandId)
     {
     case Clusters::OnOff::Commands::Toggle::Id:
         Clusters::OnOff::Commands::Toggle::Type toggleCommand;
-        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, sourceNodeId, toggleCommand);
+        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, toggleCommand);
         break;
 
     case Clusters::OnOff::Commands::On::Id:
         Clusters::OnOff::Commands::On::Type onCommand;
-        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, sourceNodeId, onCommand);
+        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, onCommand);
 
         break;
 
     case Clusters::OnOff::Commands::Off::Id:
         Clusters::OnOff::Commands::Off::Type offCommand;
-        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, sourceNodeId, offCommand);
+        Controller::InvokeGroupCommandRequest(&exchangeMgr, binding.fabricIndex, binding.groupId, offCommand);
         break;
     }
 }

examples/light-switch-app/nrfconnect/main/BindingHandler.cpp

@@ -60,10 +60,8 @@ void BindingHandler::OnOffProcessCommand(CommandId aCommandId, const EmberBindin
         else
         {
 
-            NodeId sourceNodeId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(aBinding.fabricIndex)->GetNodeId();
             Messaging::ExchangeManager & exchangeMgr = Server::GetInstance().GetExchangeManager();
-            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, sourceNodeId,
-                                                        toggleCommand);
+            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, toggleCommand);
         }
         break;
 
@@ -76,10 +74,8 @@ void BindingHandler::OnOffProcessCommand(CommandId aCommandId, const EmberBindin
         }
         else
         {
-            NodeId sourceNodeId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(aBinding.fabricIndex)->GetNodeId();
             Messaging::ExchangeManager & exchangeMgr = Server::GetInstance().GetExchangeManager();
-            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, sourceNodeId,
-                                                        onCommand);
+            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, onCommand);
         }
         break;
 
@@ -92,10 +88,8 @@ void BindingHandler::OnOffProcessCommand(CommandId aCommandId, const EmberBindin
         }
         else
         {
-            NodeId sourceNodeId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(aBinding.fabricIndex)->GetNodeId();
             Messaging::ExchangeManager & exchangeMgr = Server::GetInstance().GetExchangeManager();
-            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, sourceNodeId,
-                                                        onCommand);
+            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, onCommand);
         }
         break;
     default:
@@ -134,10 +128,8 @@ void BindingHandler::LevelControlProcessCommand(CommandId aCommandId, const Embe
         }
         else
         {
-            NodeId sourceNodeId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(aBinding.fabricIndex)->GetNodeId();
             Messaging::ExchangeManager & exchangeMgr = Server::GetInstance().GetExchangeManager();
-            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, sourceNodeId,
-                                                        moveToLevelCommand);
+            ret = Controller::InvokeGroupCommandRequest(&exchangeMgr, aBinding.fabricIndex, aBinding.groupId, moveToLevelCommand);
         }
     }
     break;

src/controller/CHIPCluster.h

@@ -146,8 +146,8 @@ class DLL_EXPORT ClusterBase
     }
 
     template <typename AttrType>
-    CHIP_ERROR WriteAttribute(GroupId groupId, FabricIndex fabricIndex, NodeId sourceNodeId, const AttrType & requestData,
-                              void * context, ClusterId clusterId, AttributeId attributeId, WriteResponseSuccessCallback successCb,
+    CHIP_ERROR WriteAttribute(GroupId groupId, FabricIndex fabricIndex, const AttrType & requestData, void * context,
+                              ClusterId clusterId, AttributeId attributeId, WriteResponseSuccessCallback successCb,
                               WriteResponseFailureCallback failureCb, const Optional<uint16_t> & aTimedWriteTimeoutMs,
                               WriteResponseDoneCallback doneCb = nullptr, const Optional<DataVersion> & aDataVersion = NullOptional)
     {
@@ -173,20 +173,19 @@ class DLL_EXPORT ClusterBase
             }
         };
 
-        Transport::OutgoingGroupSession groupSession(groupId, fabricIndex, sourceNodeId);
+        Transport::OutgoingGroupSession groupSession(groupId, fabricIndex);
         return chip::Controller::WriteAttribute<AttrType>(SessionHandle(groupSession), 0 /*Unused for Group*/, clusterId,
                                                           attributeId, requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs,
                                                           onDoneCb, aDataVersion);
     }
 
     template <typename AttributeInfo>
-    CHIP_ERROR WriteAttribute(GroupId groupId, FabricIndex fabricIndex, NodeId sourceNodeId,
-                              const typename AttributeInfo::Type & requestData, void * context,
-                              WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
+    CHIP_ERROR WriteAttribute(GroupId groupId, FabricIndex fabricIndex, const typename AttributeInfo::Type & requestData,
+                              void * context, WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
                               WriteResponseDoneCallback doneCb = nullptr, const Optional<DataVersion> & aDataVersion = NullOptional,
                               const Optional<uint16_t> & aTimedWriteTimeoutMs = NullOptional)
     {
-        return WriteAttribute(groupId, fabricIndex, sourceNodeId, requestData, context, AttributeInfo::GetClusterId(),
+        return WriteAttribute(groupId, fabricIndex, requestData, context, AttributeInfo::GetClusterId(),
                               AttributeInfo::GetAttributeId(), successCb, failureCb, aTimedWriteTimeoutMs, doneCb, aDataVersion);
     }
 

src/controller/InvokeInteraction.h

@@ -103,12 +103,12 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, const SessionHan
  */
 template <typename RequestObjectT>
 CHIP_ERROR InvokeGroupCommandRequest(Messaging::ExchangeManager * exchangeMgr, chip::FabricIndex fabric, chip::GroupId groupId,
-                                     chip::NodeId sourceNodeId, const RequestObjectT & requestCommandData)
+                                     const RequestObjectT & requestCommandData)
 {
     CHIP_ERROR error                   = CHIP_NO_ERROR;
     app::CommandPathParams commandPath = { groupId, RequestObjectT::GetClusterId(), RequestObjectT::GetCommandId(),
                                            app::CommandPathFlags::kGroupIdValid };
-    Transport::OutgoingGroupSession session(groupId, fabric, sourceNodeId);
+    Transport::OutgoingGroupSession session(groupId, fabric);
 
     auto commandSender = chip::Platform::MakeUnique<app::CommandSender>(nullptr, exchangeMgr);
     VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);

src/messaging/tests/MessagingContext.cpp

@@ -109,7 +109,7 @@ CHIP_ERROR MessagingContext::CreateSessionAliceToBob()
 
 CHIP_ERROR MessagingContext::CreateSessionBobToFriends()
 {
-    mSessionBobToFriends.Emplace(GetFriendsGroupId(), mBobFabricIndex, GetBobFabric()->GetNodeId());
+    mSessionBobToFriends.Emplace(GetFriendsGroupId(), mBobFabricIndex);
     return CHIP_NO_ERROR;
 }
 

src/transport/GroupSession.h

@@ -76,10 +76,7 @@ class IncomingGroupSession : public Session
 class OutgoingGroupSession : public Session
 {
 public:
-    OutgoingGroupSession(GroupId group, FabricIndex fabricIndex, NodeId sourceNodeId) : mGroupId(group), mSourceNodeId(sourceNodeId)
-    {
-        SetFabricIndex(fabricIndex);
-    }
+    OutgoingGroupSession(GroupId group, FabricIndex fabricIndex) : mGroupId(group) { SetFabricIndex(fabricIndex); }
     ~OutgoingGroupSession() override { NotifySessionReleased(); }
 
     Session::SessionType GetSessionType() const override { return Session::SessionType::kGroupOutgoing; }
@@ -111,11 +108,8 @@ class OutgoingGroupSession : public Session
 
     GroupId GetGroupId() const { return mGroupId; }
 
-    NodeId GetSourceNodeId() const { return mSourceNodeId; }
-
 private:
     const GroupId mGroupId;
-    const NodeId mSourceNodeId;
 };
 
 } // namespace Transport