diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index ffaceaae8bc513..2546c646e38b62 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -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*/); } diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 8dff49866367a3..7f55a8e16149b7 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -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(); @@ -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; @@ -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); @@ -210,6 +227,7 @@ CHIP_ERROR WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayl err = writeRequestParser.GetWriteRequests(&AttributeDataIBsParser); SuccessOrExit(err); + AttributeDataIBsParser.GetReader(&AttributeDataIBsReader); err = ProcessAttributeDataIBs(AttributeDataIBsReader); diff --git a/src/app/tests/suites/TestGroupMessaging.yaml b/src/app/tests/suites/TestGroupMessaging.yaml index ed2a543e593fd5..bb41572895f8c7 100644 --- a/src/app/tests/suites/TestGroupMessaging.yaml +++ b/src/app/tests/suites/TestGroupMessaging.yaml @@ -33,7 +33,6 @@ tests: - label: "Read back Attribute" command: "readAttribute" attribute: "location" - disabled: true response: value: "us" @@ -41,13 +40,11 @@ tests: command: "writeAttribute" attribute: "location" groupId: "1234" - disabled: true arguments: value: "" - label: "Read back Attribute" command: "readAttribute" attribute: "location" - disabled: true response: value: "" diff --git a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m index e23475a6469a96..7cffe7fe426151 100644 --- a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m +++ b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m @@ -21752,6 +21752,76 @@ - (void)testSendClusterTestGroupMessaging_000000_WriteAttribute [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; } +- (void)testSendClusterTestGroupMessaging_000001_ReadAttribute +{ + XCTestExpectation * expectation = [self expectationWithDescription:@"Read back Attribute"]; + + CHIPDevice * device = GetConnectedDevice(); + dispatch_queue_t queue = dispatch_get_main_queue(); + CHIPTestBasic * cluster = [[CHIPTestBasic alloc] initWithDevice:device endpoint:0 queue:queue]; + XCTAssertNotNil(cluster); + + [cluster readAttributeLocationWithResponseHandler:^(NSError * err, NSDictionary * values) { + NSLog(@"Read back Attribute Error: %@", err); + + XCTAssertEqual(err.code, 0); + + { + id actualValue = values[@"value"]; + XCTAssertTrue([actualValue isEqualToString:@"us"]); + } + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} +- (void)testSendClusterTestGroupMessaging_000002_WriteAttribute +{ + XCTestExpectation * expectation = [self expectationWithDescription:@"Restore initial location value"]; + + 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 = @""; + [cluster writeAttributeLocationWithValue:locationArgument + responseHandler:^(NSError * err, NSDictionary * values) { + NSLog(@"Restore initial location value Error: %@", err); + + XCTAssertEqual(err.code, 0); + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} +- (void)testSendClusterTestGroupMessaging_000003_ReadAttribute +{ + XCTestExpectation * expectation = [self expectationWithDescription:@"Read back Attribute"]; + + CHIPDevice * device = GetConnectedDevice(); + dispatch_queue_t queue = dispatch_get_main_queue(); + CHIPTestBasic * cluster = [[CHIPTestBasic alloc] initWithDevice:device endpoint:0 queue:queue]; + XCTAssertNotNil(cluster); + + [cluster readAttributeLocationWithResponseHandler:^(NSError * err, NSDictionary * values) { + NSLog(@"Read back Attribute Error: %@", err); + + XCTAssertEqual(err.code, 0); + + { + id actualValue = values[@"value"]; + XCTAssertTrue([actualValue isEqualToString:@""]); + } + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} - (void)testSendClusterTest_TC_DIAGSW_1_1_000000_ReadAttribute { diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 62282598cb922d..a3da1cdc2b43e4 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -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)); @@ -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()) + { + ReturnErrorOnFailure( + mDispatch->OnMessageReceived(messageCounter, payloadHeader, peerAddress, msgFlags, GetReliableMessageContext())); + } if (IsAckPending() && !mDelegate) { diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index e60e08a9620f1b..5688d6eef84bcb 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -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(); @@ -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; } @@ -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; } @@ -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()) { diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 61cd845e815edc..fdbe6bf3e77cbf 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -38304,6 +38304,18 @@ class TestGroupMessaging : public TestCommand ChipLogProgress(chipTool, " ***** Test Step 0 : Group Write Attribute\n"); err = TestGroupWriteAttribute_0(); break; + case 1: + ChipLogProgress(chipTool, " ***** Test Step 1 : Read back Attribute\n"); + err = TestReadBackAttribute_1(); + break; + case 2: + ChipLogProgress(chipTool, " ***** Test Step 2 : Restore initial location value\n"); + err = TestRestoreInitialLocationValue_2(); + break; + case 3: + ChipLogProgress(chipTool, " ***** Test Step 3 : Read back Attribute\n"); + err = TestReadBackAttribute_3(); + break; } if (CHIP_NO_ERROR != err) @@ -38315,7 +38327,7 @@ class TestGroupMessaging : public TestCommand private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 1; + const uint16_t mTestCount = 4; static void OnFailureCallback_0(void * context, EmberAfStatus status) { @@ -38324,6 +38336,33 @@ class TestGroupMessaging : public TestCommand static void OnSuccessCallback_0(void * context) { (static_cast(context))->OnSuccessResponse_0(); } + static void OnFailureCallback_1(void * context, EmberAfStatus status) + { + (static_cast(context))->OnFailureResponse_1(chip::to_underlying(status)); + } + + static void OnSuccessCallback_1(void * context, chip::CharSpan location) + { + (static_cast(context))->OnSuccessResponse_1(location); + } + + static void OnFailureCallback_2(void * context, EmberAfStatus status) + { + (static_cast(context))->OnFailureResponse_2(chip::to_underlying(status)); + } + + static void OnSuccessCallback_2(void * context) { (static_cast(context))->OnSuccessResponse_2(); } + + static void OnFailureCallback_3(void * context, EmberAfStatus status) + { + (static_cast(context))->OnFailureResponse_3(chip::to_underlying(status)); + } + + static void OnSuccessCallback_3(void * context, chip::CharSpan location) + { + (static_cast(context))->OnSuccessResponse_3(location); + } + // // Tests methods // @@ -38344,6 +38383,59 @@ class TestGroupMessaging : public TestCommand void OnFailureResponse_0(uint8_t status) { ThrowFailureResponse(); } void OnSuccessResponse_0() { NextTest(); } + + CHIP_ERROR TestReadBackAttribute_1() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 0; + chip::Controller::BasicClusterTest cluster; + cluster.Associate(mDevice, endpoint); + + return cluster.ReadAttribute(this, OnSuccessCallback_1, + OnFailureCallback_1); + } + + void OnFailureResponse_1(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_1(chip::CharSpan location) + { + VerifyOrReturn(CheckValueAsString("location", location, chip::CharSpan("us", 2))); + NextTest(); + } + + CHIP_ERROR TestRestoreInitialLocationValue_2() + { + const chip::GroupId groupId = 1234; + chip::Controller::BasicClusterTest cluster; + cluster.AssociateWithGroup(mDevice, groupId); + + chip::CharSpan locationArgument; + locationArgument = chip::Span("garbage: not in length on purpose", 0); + + return cluster.WriteAttribute( + locationArgument, this, OnSuccessCallback_2, OnFailureCallback_2); + } + + void OnFailureResponse_2(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_2() { NextTest(); } + + CHIP_ERROR TestReadBackAttribute_3() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 0; + chip::Controller::BasicClusterTest cluster; + cluster.Associate(mDevice, endpoint); + + return cluster.ReadAttribute(this, OnSuccessCallback_3, + OnFailureCallback_3); + } + + void OnFailureResponse_3(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_3(chip::CharSpan location) + { + VerifyOrReturn(CheckValueAsString("location", location, chip::CharSpan("", 0))); + NextTest(); + } }; class Test_TC_DIAGSW_1_1 : public TestCommand