Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove node id from outgoing group session #16718

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Remove node id from outgoing group session #16718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69f3b9a
Remove node id from outgoing group session
kghost Mar 28, 2022
468adc4
Return an error when group fabric is not valid
kghost Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions examples/chip-tool/commands/clusters/ClusterCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 /////////
Expand Down Expand Up @@ -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) };
Expand All @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/ModelCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions examples/chip-tool/commands/clusters/ModelCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
10 changes: 5 additions & 5 deletions examples/chip-tool/commands/clusters/WriteAttributeCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 /////////
Expand Down Expand Up @@ -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;
Expand All @@ -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();

Expand Down
7 changes: 3 additions & 4 deletions examples/chip-tool/commands/common/CommandInvoker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions examples/chip-tool/templates/commands.zapt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/templates/tests/partials/test_cluster.zapt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
));
Expand Down
7 changes: 3 additions & 4 deletions examples/light-switch-app/efr32/src/binding-handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
16 changes: 4 additions & 12 deletions examples/light-switch-app/nrfconnect/main/BindingHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 6 additions & 7 deletions src/controller/CHIPCluster.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/controller/InvokeInteraction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/messaging/tests/MessagingContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
8 changes: 1 addition & 7 deletions src/transport/GroupSession.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -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
Expand Down
Loading