From 25170586a996ace9fe3ec6c44fd4e9b344ac9a62 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 9 Dec 2022 13:59:50 -0500 Subject: [PATCH] Set up a cluster state cache in MTRDevice. (#23980) We only really need this for data version and event number handling during re-subscribe, to minimize network traffic and load on devices. --- src/darwin/Framework/CHIP/MTRDevice.mm | 24 ++++--- .../Framework/CHIPTests/MTRDeviceTests.m | 72 +++++++++++++++++++ 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 38a52bba989719..fe4ef23bfad23a 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -401,10 +401,7 @@ - (void)_setupSubscription attributePath.release(); eventPath.release(); - std::unique_ptr callback; - std::unique_ptr readClient; - std::unique_ptr clusterStateCache; - callback = std::make_unique( + auto callback = std::make_unique( ^(NSArray * value) { MTR_LOG_INFO("%@ got attribute report %@", self, value); dispatch_async(self.queue, ^{ @@ -447,8 +444,18 @@ - (void)_setupSubscription [self _handleSubscriptionReset]; }); }); - readClient = std::make_unique(InteractionModelEngine::GetInstance(), exchangeManager, - callback->GetBufferedCallback(), ReadClient::InteractionType::Subscribe); + + // Set up a cluster state cache. We really just want this for the + // logic it has for tracking data versions and event numbers so we + // minimize the amount of data we request on resubscribes; we + // don't care about the data it stores. Ideally we could use the + // dataversion-management logic without needing to store the data + // separately from the data store we already have, or we would + // stop storing our data separately. + auto clusterStateCache = std::make_unique(*callback.get()); + auto readClient + = std::make_unique(InteractionModelEngine::GetInstance(), exchangeManager, + clusterStateCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe); // SendAutoResubscribeRequest cleans up the params, even on failure. CHIP_ERROR err = readClient->SendAutoResubscribeRequest(std::move(readParams)); @@ -463,9 +470,10 @@ - (void)_setupSubscription return; } - // Callback and ReadClient will be deleted when OnDone is called or an error is - // encountered. + // Callback and ClusterStateCache and ReadClient will be deleted + // when OnDone is called or an error is encountered. callback->AdoptReadClient(std::move(readClient)); + callback->AdoptClusterStateCache(std::move(clusterStateCache)); callback.release(); }]; } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 1b5e64dd7728dd..f7ab5557af14db 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -116,8 +116,13 @@ - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSEr @end +typedef void (^MTRDeviceTestDelegateDataHandler)(NSArray *> *); + @interface MTRDeviceTestDelegate : NSObject @property (nonatomic) dispatch_block_t onSubscriptionEstablished; +@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onAttributeDataReceived; +@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onEventDataReceived; +@property (nonatomic, nullable) dispatch_block_t onSubscriptionDropped; @end @implementation MTRDeviceTestDelegate @@ -125,15 +130,23 @@ - (void)device:(MTRDevice *)device stateChanged:(MTRDeviceState)state { if (state == MTRDeviceStateReachable) { self.onSubscriptionEstablished(); + } else if (state == MTRDeviceStateUnknown && self.onSubscriptionDropped != nil) { + self.onSubscriptionDropped(); } } - (void)device:(MTRDevice *)device receivedAttributeReport:(NSArray *> *)attributeReport { + if (self.onAttributeDataReceived != nil) { + self.onAttributeDataReceived(attributeReport); + } } - (void)device:(MTRDevice *)device receivedEventReport:(NSArray *> *)eventReport { + if (self.onEventDataReceived != nil) { + self.onEventDataReceived(eventReport); + } } @end @@ -1369,9 +1382,68 @@ - (void)test017_TestMTRDeviceBasics [subscriptionExpectation fulfill]; }; + __block unsigned attributeReportsReceived = 0; + delegate.onAttributeDataReceived = ^(NSArray *> * data) { + attributeReportsReceived += data.count; + }; + + __block unsigned eventReportsReceived = 0; + delegate.onEventDataReceived = ^(NSArray *> * data) { + eventReportsReceived += data.count; + }; + [device setDelegate:delegate queue:queue]; [self waitForExpectations:@[ subscriptionExpectation ] timeout:60]; + + XCTAssertNotEqual(attributeReportsReceived, 0); + + attributeReportsReceived = 0; + eventReportsReceived = 0; + + XCTestExpectation * resubscriptionExpectation = [self expectationWithDescription:@"Resubscription has happened"]; + delegate.onSubscriptionEstablished = ^() { + [resubscriptionExpectation fulfill]; + }; + + XCTestExpectation * subscriptionDroppedExpectation = [self expectationWithDescription:@"Subscription has dropped"]; + delegate.onSubscriptionDropped = ^() { + [subscriptionDroppedExpectation fulfill]; + }; + + // Now trigger another subscription which will cause ours to drop; we should re-subscribe after that. + MTRBaseDevice * baseDevice = GetConnectedDevice(); + __auto_type params = [[MTRSubscribeParams alloc] initWithMinInterval:@(1) maxInterval:@(10)]; + params.resubscribeIfLost = NO; + params.replaceExistingSubscriptions = YES; + // Create second subscription which will cancel the first subscription. We + // can use a non-existent path here to cut down on the work that gets done. + [baseDevice subscribeAttributeWithEndpointId:@10000 + clusterId:@6 + attributeId:@0 + minInterval:@(1) + maxInterval:@(2) + params:params + clientQueue:queue + reportHandler:^(id _Nullable values, NSError * _Nullable error) { + } + subscriptionEstablished:^() { + }]; + + [self waitForExpectations:@[ subscriptionDroppedExpectation, resubscriptionExpectation ] timeout:60 enforceOrder:YES]; + + // Now make sure we ignore later tests. Ideally we would just unsubscribe + // or remove the delegate, but there's no good way to do that. + delegate.onSubscriptionEstablished = ^() { + }; + delegate.onSubscriptionDropped = nil; + delegate.onAttributeDataReceived = nil; + delegate.onEventDataReceived = nil; + + // Make sure we got no updated reports (because we had a cluster state cache + // with data versions) during the resubscribe. + XCTAssertEqual(attributeReportsReceived, 0); + XCTAssertEqual(eventReportsReceived, 0); } - (void)test018_SubscriptionErrorWhenNotResubscribing