Skip to content

Commit

Permalink
Set up a cluster state cache in MTRDevice. (#23980)
Browse files Browse the repository at this point in the history
We only really need this for data version and event number handling
during re-subscribe, to minimize network traffic and load on devices.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 5, 2023
1 parent bba0323 commit 2517058
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 8 deletions.
24 changes: 16 additions & 8 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,7 @@ - (void)_setupSubscription
attributePath.release();
eventPath.release();

std::unique_ptr<SubscriptionCallback> callback;
std::unique_ptr<ReadClient> readClient;
std::unique_ptr<ClusterStateCache> clusterStateCache;
callback = std::make_unique<SubscriptionCallback>(
auto callback = std::make_unique<SubscriptionCallback>(
^(NSArray * value) {
MTR_LOG_INFO("%@ got attribute report %@", self, value);
dispatch_async(self.queue, ^{
Expand Down Expand Up @@ -447,8 +444,18 @@ - (void)_setupSubscription
[self _handleSubscriptionReset];
});
});
readClient = std::make_unique<ReadClient>(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<ClusterStateCache>(*callback.get());
auto readClient
= std::make_unique<ReadClient>(InteractionModelEngine::GetInstance(), exchangeManager,
clusterStateCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe);

// SendAutoResubscribeRequest cleans up the params, even on failure.
CHIP_ERROR err = readClient->SendAutoResubscribeRequest(std::move(readParams));
Expand All @@ -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();
}];
}
Expand Down
72 changes: 72 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,37 @@ - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSEr

@end

typedef void (^MTRDeviceTestDelegateDataHandler)(NSArray<NSDictionary<NSString *, id> *> *);

@interface MTRDeviceTestDelegate : NSObject <MTRDeviceDelegate>
@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
- (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<NSDictionary<NSString *, id> *> *)attributeReport
{
if (self.onAttributeDataReceived != nil) {
self.onAttributeDataReceived(attributeReport);
}
}

- (void)device:(MTRDevice *)device receivedEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport
{
if (self.onEventDataReceived != nil) {
self.onEventDataReceived(eventReport);
}
}

@end
Expand Down Expand Up @@ -1369,9 +1382,68 @@ - (void)test017_TestMTRDeviceBasics
[subscriptionExpectation fulfill];
};

__block unsigned attributeReportsReceived = 0;
delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * data) {
attributeReportsReceived += data.count;
};

__block unsigned eventReportsReceived = 0;
delegate.onEventDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * 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
Expand Down

0 comments on commit 2517058

Please sign in to comment.