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.

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

[Group] Fix Group Write Attribute sending and receiving path #12052

Merged
merged 2 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
}

exit:
if (status != Protocols::InteractionModel::Status::Success)
if (status != Protocols::InteractionModel::Status::Success && !apExchangeContext->IsGroupExchangeContext())
{
err = StatusResponse::SendStatusResponse(status, apExchangeContext, false /*aExpectResponse*/);
}
Expand Down
24 changes: 21 additions & 3 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,12 @@ CHIP_ERROR WriteHandler::OnWriteRequest(Messaging::ExchangeContext * apExchangeC

err = ProcessWriteRequest(std::move(aPayload));
SuccessOrExit(err);
err = SendWriteResponse();

// Do not send response on Group Write
if (!apExchangeContext->IsGroupExchangeContext())
{
err = SendWriteResponse();
}

exit:
Shutdown();
Expand Down Expand Up @@ -110,6 +115,8 @@ CHIP_ERROR WriteHandler::SendWriteResponse()
CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeDataIBsReader)
{
CHIP_ERROR err = CHIP_NO_ERROR;
VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_INTERNAL);

while (CHIP_NO_ERROR == (err = aAttributeDataIBsReader.Next()))
{
chip::TLV::TLVReader dataReader;
Expand All @@ -131,9 +138,19 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
{
err = CHIP_NO_ERROR;
}
if (mpExchangeCtx->IsGroupExchangeContext())
{
// TODO retrieve Endpoint ID with GroupDataProvider using GroupId and FabricId
// Issue 11075

err = attributePath.GetEndpoint(&(clusterInfo.mEndpointId));
SuccessOrExit(err);
// Using endpoint 0 for test purposes
clusterInfo.mEndpointId = 0;
}
else
{
err = attributePath.GetEndpoint(&(clusterInfo.mEndpointId));
SuccessOrExit(err);
}

err = attributePath.GetCluster(&(clusterInfo.mClusterId));
SuccessOrExit(err);
Expand Down Expand Up @@ -210,6 +227,7 @@ CHIP_ERROR WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayl

err = writeRequestParser.GetWriteRequests(&AttributeDataIBsParser);
SuccessOrExit(err);

AttributeDataIBsParser.GetReader(&AttributeDataIBsReader);
err = ProcessAttributeDataIBs(AttributeDataIBsReader);

Expand Down
3 changes: 0 additions & 3 deletions src/app/tests/suites/TestGroupMessaging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,18 @@ tests:
- label: "Read back Attribute"
command: "readAttribute"
attribute: "location"
disabled: true
response:
value: "us"

- label: "Restore initial location value"
command: "writeAttribute"
attribute: "location"
groupId: "1234"
disabled: true
arguments:
value: ""

- label: "Read back Attribute"
command: "readAttribute"
attribute: "location"
disabled: true
response:
value: ""
70 changes: 70 additions & 0 deletions src/darwin/Framework/CHIPTests/CHIPClustersTests.m

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ CHIP_ERROR ExchangeContext::SendMessage(Protocols::Id protocolId, uint8_t msgTyp
}

{
// ExchangeContext for group are supposed to always be Initiator
if (IsGroupExchangeContext() && !IsInitiator())
{
return CHIP_ERROR_INTERNAL;
}

// Create a new scope for `err`, to avoid shadowing warning previous `err`.
CHIP_ERROR err = mDispatch->SendMessage(mSession.Value(), mExchangeId, IsInitiator(), GetReliableMessageContext(),
reliableTransmissionRequested, protocolId, msgType, std::move(msgBuf));
Expand Down Expand Up @@ -445,8 +451,12 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
MessageHandled();
});

ReturnErrorOnFailure(
mDispatch->OnMessageReceived(messageCounter, payloadHeader, peerAddress, msgFlags, GetReliableMessageContext()));
// TODO : Remove this bypass for group as to perform the MessagePermitted function Issue # 12101
if (!IsGroupExchangeContext())
jepenven-silabs marked this conversation as resolved.
Show resolved Hide resolved
{
ReturnErrorOnFailure(
mDispatch->OnMessageReceived(messageCounter, payloadHeader, peerAddress, msgFlags, GetReliableMessageContext()));
}

if (IsAckPending() && !mDelegate)
{
Expand Down
10 changes: 7 additions & 3 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ CHIP_ERROR SessionManager::PrepareMessage(SessionHandle sessionHandle, PayloadHe
{
return CHIP_ERROR_INTERNAL;
}
// TODO #11911 Update SecureMessageCodec::Encrypt for Group
ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(message));

#if CHIP_PROGRESS_LOGGING
destination = sessionHandle.GetPeerNodeId();
Expand Down Expand Up @@ -414,7 +416,7 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea

if (msg.IsNull())
{
ChipLogError(Inet, "Secure transport received NULL packet, discarding");
ChipLogError(Inet, "Secure transport received Unicast NULL packet, discarding");
return;
}

Expand Down Expand Up @@ -485,9 +487,9 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
FabricIndex fabricIndex = 0; // TODO : remove initialization once GroupDataProvider->Decrypt is implemented
// Credentials::GroupDataProvider * groups = Credentials::GetGroupDataProvider();

if (!msg.IsNull())
if (msg.IsNull())
{
ChipLogError(Inet, "Secure transport received NULL packet, discarding");
ChipLogError(Inet, "Secure transport received Groupcast NULL packet, discarding");
return;
}

Expand All @@ -503,6 +505,8 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
// VerifyOrExit(CHIP_NO_ERROR == groups->DecryptMessage(packetHeader, payloadHeader, msg),
// ChipLogError(Inet, "Secure transport received group message, but failed to decode it, discarding"));

ReturnOnFailure(payloadHeader.DecodeAndConsume(msg));

// MCSP check
if (packetHeader.IsValidMCSPMsg())
{
Expand Down
94 changes: 93 additions & 1 deletion zzz_generated/chip-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.