From 41598991ecb982650af62880a4a551f07684d163 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 11 Aug 2022 11:01:13 -0400 Subject: [PATCH] Fix sending of timed invoke via MTRBaseDevice invokeCommandWithEndpointId. (#21804) * Fix CommandSender to error out early if it detects that its "am I a timed invoke" state does not match whether a timed invoke timeout was provided. This should prevent invalid message sequences from being sent by accident. * Move the one actual test from MTRClustersTests to MTRDeviceTests. * Enable running MTRDeviceTests in CI. * Add a test to MTRDeviceTests that does a timed invoke. * Fix incorrect initialization of CommandSender in invokeCommandWithEndpointId The tests being enabled fail without that last item. --- src/app/CommandSender.cpp | 9 + src/darwin/Framework/CHIP/MTRBaseDevice.mm | 3 +- .../Framework/CHIPTests/MTRClustersTests.m | 208 ------------------ .../Framework/CHIPTests/MTRDeviceTests.m | 94 ++++++++ .../Matter.xcodeproj/project.pbxproj | 4 - .../xcschemes/Matter Framework Tests.xcscheme | 3 - 6 files changed, 105 insertions(+), 216 deletions(-) delete mode 100644 src/darwin/Framework/CHIPTests/MTRClustersTests.m diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 7e074df054ea6a..0b6bad64213eee 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -76,6 +76,15 @@ CHIP_ERROR CommandSender::SendCommandRequest(const SessionHandle & session, Opti mExchangeCtx->SetResponseTimeout(timeout.ValueOr(session->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime))); + if (mTimedRequest != mTimedInvokeTimeoutMs.HasValue()) + { + ChipLogError( + DataManagement, + "Inconsistent timed request state in CommandSender: mTimedRequest (%d) != mTimedInvokeTimeoutMs.HasValue() (%d)", + mTimedRequest, mTimedInvokeTimeoutMs.HasValue()); + return CHIP_ERROR_INCORRECT_STATE; + } + if (mTimedInvokeTimeoutMs.HasValue()) { ReturnErrorOnFailure(TimedRequest::Send(mExchangeCtx.Get(), mTimedInvokeTimeoutMs.Value())); diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 9b3f10521ae465..c3e060d2751077 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -1146,7 +1146,8 @@ new MTRDataValueDictionaryCallbackBridge(clientQueue, self, completion, decoder->SetOnDoneCallback(onDoneCb); - auto commandSender = chip::Platform::MakeUnique(decoder.get(), &exchangeManager, false); + bool isTimedRequest = (timeoutMs != nil); + auto commandSender = chip::Platform::MakeUnique(decoder.get(), &exchangeManager, isTimedRequest); VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY); ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, MTRDataValueDictionaryDecodableType(commandFields), diff --git a/src/darwin/Framework/CHIPTests/MTRClustersTests.m b/src/darwin/Framework/CHIPTests/MTRClustersTests.m deleted file mode 100644 index bdab9a531ea21e..00000000000000 --- a/src/darwin/Framework/CHIPTests/MTRClustersTests.m +++ /dev/null @@ -1,208 +0,0 @@ -// -// MTRClustersTests.m -// MTRClustersTests -/* - * - * Copyright (c) 2022 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// THIS FILE IS GENERATED BY ZAP - -// module headers -#import - -#import "MTRErrorTestUtils.h" -#import "MTRTestKeys.h" -#import "MTRTestStorage.h" - -// system dependencies -#import - -const uint16_t kPairingTimeoutInSeconds = 10; -const uint16_t kCASESetupTimeoutInSeconds = 30; -const uint16_t kTimeoutInSeconds = 20; -const uint64_t nodeId = 0x12344321; -const uint32_t kSetupPINCode = 20202021; -const uint16_t kRemotePort = 5540; -const uint16_t kLocalPort = 5541; -NSString * kAddress = @"::1"; -static uint16_t kTestVendorId = 0xFFF1u; - -// Singleton controller we use. -static MTRDeviceController * sController = nil; - -@interface MTRToolPairingDelegate : NSObject -@property (nonatomic, strong) XCTestExpectation * expectation; -@end - -@implementation MTRToolPairingDelegate -- (id)initWithExpectation:(XCTestExpectation *)expectation -{ - self = [super init]; - if (self) { - _expectation = expectation; - } - return self; -} - -- (void)onPairingComplete:(NSError *)error -{ - XCTAssertEqual(error.code, 0); - NSError * commissionError = nil; - [sController commissionDevice:nodeId commissioningParams:[[MTRCommissioningParameters alloc] init] error:&commissionError]; - XCTAssertNil(commissionError); - - // Keep waiting for onCommissioningComplete -} - -- (void)onCommissioningComplete:(NSError *)error -{ - XCTAssertEqual(error.code, 0); - [_expectation fulfill]; - _expectation = nil; -} - -- (void)onAddressUpdated:(NSError *)error -{ - XCTAssertEqual(error.code, 0); - [_expectation fulfill]; - _expectation = nil; -} -@end - -@interface MTRClustersTests : XCTestCase -@end - -@implementation MTRClustersTests - -- (void)setUp -{ - [super setUp]; - [self setContinueAfterFailure:NO]; -} - -- (void)testInitStack -{ - XCTestExpectation * expectation = [self expectationWithDescription:@"Pairing Complete"]; - - __auto_type * factory = [MTRControllerFactory sharedInstance]; - XCTAssertNotNil(factory); - - __auto_type * storage = [[MTRTestStorage alloc] init]; - __auto_type * factoryParams = [[MTRControllerFactoryParams alloc] initWithStorage:storage]; - factoryParams.port = @(kLocalPort); - // For our first startup, use a dummy paaCerts array. - factoryParams.paaCerts = [[NSArray alloc] init]; - - BOOL ok = [factory startup:factoryParams]; - XCTAssertTrue(ok); - - // Test that things still work right if we then shut down and - // restart the factory. - [factory shutdown]; - factoryParams.paaCerts = nil; - ok = [factory startup:factoryParams]; - XCTAssertTrue(ok); - - __auto_type * testKeys = [[MTRTestKeys alloc] init]; - XCTAssertNotNil(testKeys); - - __auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:testKeys fabricId:1 ipk:testKeys.ipk]; - params.vendorId = @(kTestVendorId); - - MTRDeviceController * controller = [factory startControllerOnNewFabric:params]; - XCTAssertNotNil(controller); - - sController = controller; - - MTRToolPairingDelegate * pairing = [[MTRToolPairingDelegate alloc] initWithExpectation:expectation]; - dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.pairing", DISPATCH_QUEUE_SERIAL); - - [controller setPairingDelegate:pairing queue:callbackQueue]; - - NSError * error; - [controller pairDevice:nodeId address:kAddress port:kRemotePort setupPINCode:kSetupPINCode error:&error]; - XCTAssertEqual(error.code, 0); - - [self waitForExpectationsWithTimeout:kPairingTimeoutInSeconds handler:nil]; - - __block XCTestExpectation * connectionExpectation = [self expectationWithDescription:@"CASE established"]; - [controller getBaseDevice:nodeId - queue:dispatch_get_main_queue() - completionHandler:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) { - XCTAssertEqual(error.code, 0); - [connectionExpectation fulfill]; - connectionExpectation = nil; - }]; - [self waitForExpectationsWithTimeout:kCASESetupTimeoutInSeconds handler:nil]; -} - -- (void)testShutdownStack -{ - MTRDeviceController * controller = sController; - XCTAssertNotNil(controller); - - [controller shutdown]; - XCTAssertFalse([controller isRunning]); - - [[MTRControllerFactory sharedInstance] shutdown]; - XCTAssertFalse([[MTRControllerFactory sharedInstance] isRunning]); -} - -- (void)testReuseChipClusterObject -{ - MTRDeviceController * controller = sController; - XCTAssertNotNil(controller); - - __block MTRBaseDevice * device; - __block XCTestExpectation * connectionExpectation = [self expectationWithDescription:@"CASE established"]; - [controller getBaseDevice:nodeId - queue:dispatch_get_main_queue() - completionHandler:^(MTRBaseDevice * _Nullable retrievedDevice, NSError * _Nullable error) { - XCTAssertEqual(error.code, 0); - [connectionExpectation fulfill]; - connectionExpectation = nil; - device = retrievedDevice; - }]; - [self waitForExpectationsWithTimeout:kCASESetupTimeoutInSeconds handler:nil]; - - XCTestExpectation * expectation = [self expectationWithDescription:@"ReuseMTRClusterObjectFirstCall"]; - - dispatch_queue_t queue = dispatch_get_main_queue(); - MTRBaseClusterTestCluster * cluster = [[MTRBaseClusterTestCluster alloc] initWithDevice:device endpoint:1 queue:queue]; - XCTAssertNotNil(cluster); - - [cluster testWithCompletionHandler:^(NSError * err) { - NSLog(@"ReuseMTRClusterObject test Error: %@", err); - XCTAssertEqual(err.code, 0); - [expectation fulfill]; - }]; - - [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; - - expectation = [self expectationWithDescription:@"ReuseMTRClusterObjectSecondCall"]; - - // Reuse the MTRCluster Object for multiple times. - - [cluster testWithCompletionHandler:^(NSError * err) { - NSLog(@"ReuseMTRClusterObject test Error: %@", err); - XCTAssertEqual(err.code, 0); - [expectation fulfill]; - }]; - - [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; -} - -@end diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 60e732f125524c..84782d21d90b7b 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -347,6 +347,51 @@ - (void)test003_InvokeCommand [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; } +- (void)test004_InvokeTimedCommand +{ +#if MANUAL_INDIVIDUAL_TEST + [self initStack]; + [self waitForCommissionee]; +#endif + XCTestExpectation * expectation = [self expectationWithDescription:@"invoke Off command"]; + + MTRBaseDevice * device = GetConnectedDevice(); + dispatch_queue_t queue = dispatch_get_main_queue(); + + NSDictionary * fields = @{ + @"type" : @"Structure", + @"value" : @[], + }; + [device invokeCommandWithEndpointId:@1 + clusterId:@6 + commandId:@0 + commandFields:fields + timedInvokeTimeout:@10000 + clientQueue:queue + completion:^(id _Nullable values, NSError * _Nullable error) { + NSLog(@"invoke command: Off values: %@, error: %@", values, error); + + XCTAssertNil(error); + + { + XCTAssertTrue([values isKindOfClass:[NSArray class]]); + NSArray * resultArray = values; + for (NSDictionary * result in resultArray) { + MTRCommandPath * path = result[@"commandPath"]; + XCTAssertEqual([path.endpoint unsignedIntegerValue], 1); + XCTAssertEqual([path.cluster unsignedIntegerValue], 6); + XCTAssertEqual([path.command unsignedIntegerValue], 0); + XCTAssertNil(result[@"error"]); + } + XCTAssertEqual([resultArray count], 1); + } + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} + static void (^globalReportHandler)(id _Nullable values, NSError * _Nullable error) = nil; - (void)test005_Subscribe @@ -1048,6 +1093,55 @@ - (void)test012_SubscriptionError } #endif +- (void)test013_ReuseChipClusterObject +{ +#if MANUAL_INDIVIDUAL_TEST + [self initStack]; + [self waitForCommissionee]; +#endif + + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + + __block MTRBaseDevice * device; + __block XCTestExpectation * connectionExpectation = [self expectationWithDescription:@"CASE established"]; + [controller getBaseDevice:kDeviceId + queue:dispatch_get_main_queue() + completionHandler:^(MTRBaseDevice * _Nullable retrievedDevice, NSError * _Nullable error) { + XCTAssertEqual(error.code, 0); + [connectionExpectation fulfill]; + connectionExpectation = nil; + device = retrievedDevice; + }]; + [self waitForExpectationsWithTimeout:kCASESetupTimeoutInSeconds handler:nil]; + + XCTestExpectation * expectation = [self expectationWithDescription:@"ReuseMTRClusterObjectFirstCall"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + MTRBaseClusterTestCluster * cluster = [[MTRBaseClusterTestCluster alloc] initWithDevice:device endpoint:1 queue:queue]; + XCTAssertNotNil(cluster); + + [cluster testWithCompletionHandler:^(NSError * err) { + NSLog(@"ReuseMTRClusterObject test Error: %@", err); + XCTAssertEqual(err.code, 0); + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; + + expectation = [self expectationWithDescription:@"ReuseMTRClusterObjectSecondCall"]; + + // Reuse the MTRCluster Object for multiple times. + + [cluster testWithCompletionHandler:^(NSError * err) { + NSLog(@"ReuseMTRClusterObject test Error: %@", err); + XCTAssertEqual(err.code, 0); + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil]; +} + - (void)test900_SubscribeAllAttributes { #if MANUAL_INDIVIDUAL_TEST diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj index 8ba1247ae7ce0e..b271e452d160b0 100644 --- a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj @@ -9,7 +9,6 @@ /* Begin PBXBuildFile section */ 1E5801C328941C050033A199 /* MTRTestOTAProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */; }; 1E85730C265519AE0050A4D9 /* callback-stub.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1E857307265519AE0050A4D9 /* callback-stub.cpp */; }; - 1EB41B7B263C4CC60048E4C1 /* MTRClustersTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */; }; 1EC3238D271999E2002A8BF0 /* cluster-objects.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1EC3238C271999E2002A8BF0 /* cluster-objects.cpp */; }; 1EC4CE5D25CC26E900D7304F /* MTRBaseClusters.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1EC4CE5925CC26E900D7304F /* MTRBaseClusters.mm */; }; 1EC4CE6425CC276600D7304F /* MTRBaseClusters.h in Headers */ = {isa = PBXBuildFile; fileRef = 1EC4CE6325CC276600D7304F /* MTRBaseClusters.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -142,7 +141,6 @@ 1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRTestOTAProvider.m; sourceTree = ""; }; 1E748B3928941A45008A1BE8 /* MTRTestOTAProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRTestOTAProvider.h; sourceTree = ""; }; 1E857307265519AE0050A4D9 /* callback-stub.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "callback-stub.cpp"; path = "../../../../zzz_generated/controller-clusters/zap-generated/callback-stub.cpp"; sourceTree = ""; }; - 1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRClustersTests.m; sourceTree = ""; }; 1EC3238C271999E2002A8BF0 /* cluster-objects.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "cluster-objects.cpp"; path = "../../../../zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp"; sourceTree = ""; }; 1EC4CE5925CC26E900D7304F /* MTRBaseClusters.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MTRBaseClusters.mm; path = "zap-generated/MTRBaseClusters.mm"; sourceTree = ""; }; 1EC4CE6325CC276600D7304F /* MTRBaseClusters.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = MTRBaseClusters.h; path = "zap-generated/MTRBaseClusters.h"; sourceTree = ""; }; @@ -447,7 +445,6 @@ 51E24E72274E0DAC007CCF6E /* MTRErrorTestUtils.mm */, 51C8E3F72825CDB600D47D00 /* MTRTestKeys.m */, 51D10D2D2808E2CA00E8CA3D /* MTRTestStorage.m */, - 1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */, 99C65E0F267282F1003402F6 /* MTRControllerTests.m */, 5AE6D4E327A99041001F2493 /* MTRDeviceTests.m */, 5A6FEC9C27B5E48800F25F42 /* MTRXPCProtocolTests.m */, @@ -712,7 +709,6 @@ files = ( 51D10D2E2808E2CA00E8CA3D /* MTRTestStorage.m in Sources */, 7596A8512878709F004DAE0E /* MTRAsyncCallbackQueueTests.m in Sources */, - 1EB41B7B263C4CC60048E4C1 /* MTRClustersTests.m in Sources */, 997DED1A26955D0200975E97 /* MTRThreadOperationalDatasetTests.mm in Sources */, 51C8E3F82825CDB600D47D00 /* MTRTestKeys.m in Sources */, 99C65E10267282F1003402F6 /* MTRControllerTests.m in Sources */, diff --git a/src/darwin/Framework/Matter.xcodeproj/xcshareddata/xcschemes/Matter Framework Tests.xcscheme b/src/darwin/Framework/Matter.xcodeproj/xcshareddata/xcschemes/Matter Framework Tests.xcscheme index a0fb4215189540..d376af22925ab4 100644 --- a/src/darwin/Framework/Matter.xcodeproj/xcshareddata/xcschemes/Matter Framework Tests.xcscheme +++ b/src/darwin/Framework/Matter.xcodeproj/xcshareddata/xcschemes/Matter Framework Tests.xcscheme @@ -38,9 +38,6 @@ ReferencedContainer = "container:Matter.xcodeproj"> - -