Skip to content

Commit

Permalink
Fix Group Write Attribute sending and receiving
Browse files Browse the repository at this point in the history
  • Loading branch information
jepenven-silabs committed Nov 19, 2021
1 parent a54795d commit 062a125
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 13 deletions.
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
6 changes: 6 additions & 0 deletions src/app/WriteClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::T
mpExchangeCtx = mpExchangeMgr->NewContext(session, this);
VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);

if ((mpExchangeCtx->IsGroupExchangeContext() && !mpExchangeCtx->IsInitiator()))
{
err = CHIP_ERROR_INTERNAL;
SuccessOrExit(err);
}

mpExchangeCtx->SetResponseTimeout(timeout);

// kExpectResponse is ignored by ExchangeContext in case of groupcast
Expand Down
25 changes: 22 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,20 @@ 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;
SuccessOrExit(err);
}
else
{
err = attributePath.GetEndpoint(&(clusterInfo.mEndpointId));
SuccessOrExit(err);
}

err = attributePath.GetCluster(&(clusterInfo.mClusterId));
SuccessOrExit(err);
Expand Down Expand Up @@ -210,6 +228,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.

13 changes: 11 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,11 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
MessageHandled();
});

ReturnErrorOnFailure(
mDispatch->OnMessageReceived(messageCounter, payloadHeader, peerAddress, msgFlags, GetReliableMessageContext()));
if (!IsGroupExchangeContext())
{
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
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.

0 comments on commit 062a125

Please sign in to comment.