diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h index 855587f3661acd..a583d6c04051a5 100644 --- a/src/app/AttributePathParams.h +++ b/src/app/AttributePathParams.h @@ -41,6 +41,10 @@ struct AttributePathParams AttributePathParams(aEndpointId, aClusterId, aAttributeId, ClusterInfo::kInvalidListIndex) {} + AttributePathParams(ClusterId aClusterId, AttributeId aAttributeId) : + AttributePathParams(kInvalidEndpointId, aClusterId, aAttributeId, ClusterInfo::kInvalidListIndex) + {} + AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListIndex aListIndex) : mEndpointId(aEndpointId), mClusterId(aClusterId), mAttributeId(aAttributeId), mListIndex(aListIndex) {} @@ -52,6 +56,7 @@ struct AttributePathParams bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); } /** + * SPEC 8.9.2.2 * Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not * be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not * wildcard. diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index f540ac75116a8b..13d46d2665990c 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -170,9 +170,7 @@ TLV::TLVWriter * WriteClient::GetAttributeDataIBTLVWriter() CHIP_ERROR WriteClient::ConstructAttributePath(const AttributePathParams & aAttributePathParams, AttributeDataIB::Builder aAttributeDataIB) { - // We do not support wildcard write now, reject them on client side. - VerifyOrReturnError(!aAttributePathParams.HasWildcard() && aAttributePathParams.IsValidAttributePath(), - CHIP_ERROR_INVALID_PATH_LIST); + VerifyOrReturnError(aAttributePathParams.IsValidAttributePath(), CHIP_ERROR_INVALID_PATH_LIST); return aAttributePathParams.BuildAttributePath(aAttributeDataIB.CreatePath()); } @@ -258,13 +256,15 @@ CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::T exit: if (err != CHIP_NO_ERROR) { + ChipLogError(DataManagement, "Write client failed to SendWriteRequest"); ClearExistingExchangeContext(); } if (session.IsGroupSession()) { // Always shutdown on Group communication - Shutdown(); + ChipLogDetail(DataManagement, "Closing on group Communication "); + ShutdownInternal(); } return err; diff --git a/src/app/tests/suites/TestGroupMessaging.yaml b/src/app/tests/suites/TestGroupMessaging.yaml index cb913c391ece7f..ed2a543e593fd5 100644 --- a/src/app/tests/suites/TestGroupMessaging.yaml +++ b/src/app/tests/suites/TestGroupMessaging.yaml @@ -26,7 +26,6 @@ tests: - label: "Group Write Attribute" command: "writeAttribute" attribute: "location" - disabled: true groupId: "1234" arguments: value: "us" diff --git a/src/controller/WriteInteraction.h b/src/controller/WriteInteraction.h index cd36b3e2f90444..ac1bf55b978cd7 100644 --- a/src/controller/WriteInteraction.h +++ b/src/controller/WriteInteraction.h @@ -99,19 +99,26 @@ CHIP_ERROR WriteAttribute(SessionHandle sessionHandle, chip::EndpointId endpoint ReturnErrorOnFailure(app::InteractionModelEngine::GetInstance()->NewWriteClient(handle, callback.get())); if (sessionHandle.IsGroupSession()) { - // TODO : Issue #11604 - return CHIP_ERROR_NOT_IMPLEMENTED; - // ReturnErrorOnFailure( - // handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(clusterId, attributeId), requestData)); + ReturnErrorOnFailure( + handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(clusterId, attributeId), requestData)); } else { ReturnErrorOnFailure( handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(endpointId, clusterId, attributeId), requestData)); } + ReturnErrorOnFailure(handle.SendWriteRequest(sessionHandle)); callback.release(); + + if (sessionHandle.IsGroupSession()) + { + // Manually call success callback since OnReponse won't be called in WriteClient for group + app::ConcreteAttributePath aPath; + onSuccessCb(aPath); + } + return CHIP_NO_ERROR; } diff --git a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m index d10d1bee5c7575..7a2e82990686ff 100644 --- a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m +++ b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m @@ -20004,6 +20004,29 @@ - (void)testSendClusterTestModeSelectCluster_000007_ChangeToMode [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; } +- (void)testSendClusterTestGroupMessaging_000000_WriteAttribute +{ + XCTestExpectation * expectation = [self expectationWithDescription:@"Group Write Attribute"]; + + CHIPDevice * device = GetConnectedDevice(); + dispatch_queue_t queue = dispatch_get_main_queue(); + CHIPTestBasic * cluster = [[CHIPTestBasic alloc] initWithDevice:device endpoint:0 queue:queue]; + XCTAssertNotNil(cluster); + + id locationArgument; + locationArgument = @"us"; + [cluster writeAttributeLocationWithValue:locationArgument + responseHandler:^(NSError * err, NSDictionary * values) { + NSLog(@"Group Write Attribute Error: %@", err); + + XCTAssertEqual(err.code, 0); + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} + - (void)testSendClusterTest_TC_DIAGSW_1_1_000000_ReadAttribute { XCTestExpectation * expectation = diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h index 2c85ca3ddd0e95..f65a3b2a88908e 100644 --- a/src/transport/SessionHandle.h +++ b/src/transport/SessionHandle.h @@ -75,6 +75,7 @@ class SessionHandle NodeId GetPeerNodeId() const { return mPeerNodeId; } bool IsGroupSession() const { return mGroupId.HasValue(); } + const Optional & GetGroupId() const { return mGroupId; } const Optional & GetPeerSessionId() const { return mPeerSessionId; } const Optional & GetLocalSessionId() const { return mLocalSessionId; } diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index c3c16b0d505cf6..e60e08a9620f1b 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -117,18 +117,39 @@ CHIP_ERROR SessionManager::PrepareMessage(SessionHandle sessionHandle, PayloadHe #endif // CHIP_PROGRESS_LOGGING if (sessionHandle.IsSecure()) { - SecureSession * session = GetSecureSession(sessionHandle); - if (session == nullptr) + if (sessionHandle.IsGroupSession()) { - return CHIP_ERROR_NOT_CONNECTED; - } + // TODO : #11911 + // For now, just set the packetHeader with the correct data. + packetHeader.SetDestinationGroupId(sessionHandle.GetGroupId()); + packetHeader.SetFlags(Header::SecFlagValues::kPrivacyFlag); + packetHeader.SetSessionType(Header::SessionType::kGroupSession); + // TODO : Replace the PeerNodeId with Our nodeId + packetHeader.SetSourceNodeId(sessionHandle.GetPeerNodeId()); + + if (!packetHeader.IsValidGroupMsg()) + { + return CHIP_ERROR_INTERNAL; + } - MessageCounter & counter = GetSendCounterForPacket(payloadHeader, *session); - ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session, payloadHeader, packetHeader, message, counter)); +#if CHIP_PROGRESS_LOGGING + destination = sessionHandle.GetPeerNodeId(); +#endif // CHIP_PROGRESS_LOGGING + } + else + { + SecureSession * session = GetSecureSession(sessionHandle); + if (session == nullptr) + { + return CHIP_ERROR_NOT_CONNECTED; + } + MessageCounter & counter = GetSendCounterForPacket(payloadHeader, *session); + ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session, payloadHeader, packetHeader, message, counter)); #if CHIP_PROGRESS_LOGGING - destination = session->GetPeerNodeId(); + destination = session->GetPeerNodeId(); #endif // CHIP_PROGRESS_LOGGING + } } else { diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 06e8e8be883d97..727f7c2f6d3f52 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -36126,6 +36126,10 @@ class TestGroupMessaging : public TestCommand // incorrect mTestIndex value observed when we get the response. switch (mTestIndex++) { + case 0: + ChipLogProgress(chipTool, " ***** Test Step 0 : Group Write Attribute\n"); + err = TestGroupWriteAttribute_0(); + break; } if (CHIP_NO_ERROR != err) @@ -36137,11 +36141,35 @@ class TestGroupMessaging : public TestCommand private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 0; + const uint16_t mTestCount = 1; + + static void OnFailureCallback_0(void * context, EmberAfStatus status) + { + (static_cast(context))->OnFailureResponse_0(chip::to_underlying(status)); + } + + static void OnSuccessCallback_0(void * context) { (static_cast(context))->OnSuccessResponse_0(); } // // Tests methods // + + CHIP_ERROR TestGroupWriteAttribute_0() + { + const chip::GroupId groupId = 1234; + chip::Controller::BasicClusterTest cluster; + cluster.AssociateWithGroup(mDevice, groupId); + + chip::CharSpan locationArgument; + locationArgument = chip::Span("usgarbage: not in length on purpose", 2); + + return cluster.WriteAttribute( + locationArgument, this, OnSuccessCallback_0, OnFailureCallback_0); + } + + void OnFailureResponse_0(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_0() { NextTest(); } }; class Test_TC_DIAGSW_1_1 : public TestCommand