Skip to content

Commit

Permalink
Fix sending of timed invoke via MTRBaseDevice invokeCommandWithEndpoi…
Browse files Browse the repository at this point in the history 
…ntId. (#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.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 5, 2023
1 parent 4f934e2 commit 1704416
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 216 deletions.
9 changes: 9 additions & 0 deletions src/app/CommandSender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
3 changes: 2 additions & 1 deletion src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,8 @@ new MTRDataValueDictionaryCallbackBridge(clientQueue, self, completion,

decoder->SetOnDoneCallback(onDoneCb);

auto commandSender = chip::Platform::MakeUnique<app::CommandSender>(decoder.get(), &exchangeManager, false);
bool isTimedRequest = (timeoutMs != nil);
auto commandSender = chip::Platform::MakeUnique<app::CommandSender>(decoder.get(), &exchangeManager, isTimedRequest);
VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);

ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, MTRDataValueDictionaryDecodableType(commandFields),
Expand Down
208 changes: 0 additions & 208 deletions src/darwin/Framework/CHIPTests/MTRClustersTests.m
View file

This file was deleted.

94 changes: 94 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions src/darwin/Framework/Matter.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, ); }; };
Expand Down Expand Up @@ -142,7 +141,6 @@
1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRTestOTAProvider.m; sourceTree = "<group>"; };
1E748B3928941A45008A1BE8 /* MTRTestOTAProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRTestOTAProvider.h; sourceTree = "<group>"; };
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 = "<group>"; };
1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRClustersTests.m; sourceTree = "<group>"; };
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 = "<group>"; };
1EC4CE5925CC26E900D7304F /* MTRBaseClusters.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MTRBaseClusters.mm; path = "zap-generated/MTRBaseClusters.mm"; sourceTree = "<group>"; };
1EC4CE6325CC276600D7304F /* MTRBaseClusters.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = MTRBaseClusters.h; path = "zap-generated/MTRBaseClusters.h"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
3 changes: 0 additions & 3 deletions src/darwin/Framework/Matter.xcodeproj/xcshareddata/xcschemes/Matter Framework Tests.xcscheme
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@
ReferencedContainer = "container:Matter.xcodeproj">
</BuildableReference>
<SkippedTests>
<Test
Identifier = "MTRDeviceTests">
</Test>
<Test
Identifier = "MTRRemoteDeviceSampleTests">
</Test>
Expand Down

0 comments on commit 1704416

Please sign in to comment.