Skip to content

Commit

Permalink
[Darwin] MTRDevice delegate should be notified when cache is primed w…
Browse files Browse the repository at this point in the history
…ith basic info (#32251)

* [Darwin] MTRDevice delegate should be notified when cache is primed with basic info

* Changed logic to only report once, and also improve documentation in comments

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <[email protected]>

---------

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
jtung-apple and bzbarsky-apple authored Feb 28, 2024
1 parent db76ad7 commit ed8f37d
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 5 deletions.
9 changes: 9 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,15 @@ MTR_EXTERN NSString * const MTRDataVersionKey MTR_NEWLY_AVAILABLE;
*/
- (void)deviceBecameActive:(MTRDevice *)device MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));

/**
* Notifies delegate when the device attribute cache has been primed with initial configuration data of the device
*
* This is called when the MTRDevice object goes from not knowing the device to having cached the first attribute reports that include basic mandatory information, e.g. Descriptor clusters.
*
* The intention is that after this is called, the client should be able to call read for mandatory attributes and likely expect non-nil values.
*/
- (void)deviceCachePrimed:(MTRDevice *)device MTR_NEWLY_AVAILABLE;

@end

@interface MTRDevice (Deprecated)
Expand Down
84 changes: 81 additions & 3 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ @implementation MTRDevice {
#ifdef DEBUG
NSUInteger _unitTestAttributesReportedSinceLastCheck;
#endif
BOOL _delegateDeviceCachePrimedCalled;
}

- (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
Expand Down Expand Up @@ -502,6 +503,11 @@ - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queu
_weakDelegate = [MTRWeakReference weakReferenceWithObject:delegate];
_delegateQueue = queue;

// If Check if cache is already primed and client hasn't been informed yet, call the -deviceCachePrimed: callback
if (!_delegateDeviceCachePrimedCalled && [self _isCachePrimedWithInitialConfigurationData]) {
[self _callDelegateDeviceCachePrimed];
}

if (setUpSubscription) {
[self _setupSubscription];
}
Expand Down Expand Up @@ -574,6 +580,29 @@ - (BOOL)_subscriptionAbleToReport
return (delegate != nil) && (state == MTRDeviceStateReachable);
}

- (BOOL)_callDelegateWithBlock:(void (^)(id<MTRDeviceDelegate>))block
{
os_unfair_lock_assert_owner(&self->_lock);
id<MTRDeviceDelegate> delegate = _weakDelegate.strongObject;
if (delegate) {
dispatch_async(_delegateQueue, ^{
block(delegate);
});
return YES;
}
return NO;
}

- (void)_callDelegateDeviceCachePrimed
{
os_unfair_lock_assert_owner(&self->_lock);
_delegateDeviceCachePrimedCalled = [self _callDelegateWithBlock:^(id<MTRDeviceDelegate> delegate) {
if ([delegate respondsToSelector:@selector(deviceCachePrimed:)]) {
[delegate deviceCachePrimed:self];
}
}];
}

// assume lock is held
- (void)_changeState:(MTRDeviceState)state
{
Expand Down Expand Up @@ -611,6 +640,11 @@ - (void)_handleSubscriptionEstablished
// reset subscription attempt wait time when subscription succeeds
_lastSubscriptionAttemptWait = 0;

// As subscription is established, check if the delegate needs to be informed
if (!_delegateDeviceCachePrimedCalled) {
[self _callDelegateDeviceCachePrimed];
}

[self _changeState:MTRDeviceStateReachable];

os_unfair_lock_unlock(&self->_lock);
Expand Down Expand Up @@ -741,6 +775,7 @@ - (void)_handleReportEnd
_receivingReport = NO;
_receivingPrimingReport = NO;
_estimatedStartTimeFromGeneralDiagnosticsUpTime = nil;

// For unit testing only
#ifdef DEBUG
id delegate = _weakDelegate.strongObject;
Expand Down Expand Up @@ -1948,17 +1983,24 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt

- (void)setAttributeValues:(NSArray<NSDictionary *> *)attributeValues reportChanges:(BOOL)reportChanges
{
os_unfair_lock_lock(&self->_lock);

if (reportChanges) {
[self _handleAttributeReport:attributeValues];
[self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeValues]];
} else {
os_unfair_lock_lock(&self->_lock);
for (NSDictionary * responseValue in attributeValues) {
MTRAttributePath * path = responseValue[MTRAttributePathKey];
NSDictionary * dataValue = responseValue[MTRDataKey];
_readCache[path] = dataValue;
}
os_unfair_lock_unlock(&self->_lock);
}

// If cache is set from storage and is primed with initial configuration data, then assume the client had beeen informed in the past, and mark that the callback has been called
if ([self _isCachePrimedWithInitialConfigurationData]) {
_delegateDeviceCachePrimedCalled = YES;
}

os_unfair_lock_unlock(&self->_lock);
}

// If value is non-nil, associate with expectedValueID
Expand Down Expand Up @@ -2135,6 +2177,42 @@ - (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath e
}
}

// This method checks if there is a need to inform delegate that the attribute cache has been "primed"
- (BOOL)_isCachePrimedWithInitialConfigurationData
{
os_unfair_lock_assert_owner(&self->_lock);

// Check if root node descriptor exists
NSDictionary * rootDescriptorPartsListDataValue = _readCache[[MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeDescriptorID) attributeID:@(MTRAttributeIDTypeClusterDescriptorAttributePartsListID)]];
if (!rootDescriptorPartsListDataValue || ![MTRArrayValueType isEqualToString:rootDescriptorPartsListDataValue[MTRTypeKey]]) {
return NO;
}
NSArray * partsList = rootDescriptorPartsListDataValue[MTRValueKey];
if (![partsList isKindOfClass:[NSArray class]] || !partsList.count) {
MTR_LOG_ERROR("%@ unexpected type %@ for parts list %@", self, [partsList class], partsList);
return NO;
}

// Check if we have cached descriptor clusters for each listed endpoint
for (NSDictionary * endpointDataValue in partsList) {
if (![MTRUnsignedIntegerValueType isEqual:endpointDataValue[MTRTypeKey]]) {
MTR_LOG_ERROR("%@ unexpected type for parts list item %@", self, endpointDataValue);
continue;
}
NSNumber * endpoint = endpointDataValue[MTRValueKey];
if (![endpoint isKindOfClass:[NSNumber class]]) {
MTR_LOG_ERROR("%@ unexpected type for parts list item %@", self, endpointDataValue);
continue;
}
NSDictionary * descriptorDeviceTypeListDataValue = _readCache[[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:@(MTRClusterIDTypeDescriptorID) attributeID:@(MTRAttributeIDTypeClusterDescriptorAttributeDeviceTypeListID)]];
if (![MTRArrayValueType isEqualToString:descriptorDeviceTypeListDataValue[MTRTypeKey]] || !descriptorDeviceTypeListDataValue[MTRValueKey]) {
return NO;
}
}

return YES;
}

- (MTRBaseDevice *)newBaseDevice
{
return [MTRBaseDevice deviceWithNodeID:self.nodeID controller:self.deviceController];
Expand Down
15 changes: 13 additions & 2 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,7 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
{
dispatch_queue_t queue = dispatch_get_main_queue();

// First start with clean slate and
// First start with clean slate by removing the MTRDevice and clearing the persisted cache
__auto_type * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:sController];
[sController removeDevice:device];
[sController.controllerDataStore clearAllStoredAttributes];
Expand All @@ -2853,16 +2853,20 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
// Now recreate device and get subscription primed
device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:sController];
XCTestExpectation * gotReportsExpectation = [self expectationWithDescription:@"Attribute and Event reports have been received"];
XCTestExpectation * gotDeviceCachePrimed = [self expectationWithDescription:@"Device cache primed for the first time"];
__auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
__weak __auto_type weakDelegate = delegate;
delegate.onReportEnd = ^{
[gotReportsExpectation fulfill];
__strong __auto_type strongDelegate = weakDelegate;
strongDelegate.onReportEnd = nil;
};
delegate.onDeviceCachePrimed = ^{
[gotDeviceCachePrimed fulfill];
};
[device setDelegate:delegate queue:queue];

[self waitForExpectations:@[ gotReportsExpectation ] timeout:60];
[self waitForExpectations:@[ gotReportsExpectation, gotDeviceCachePrimed ] timeout:60];

NSUInteger attributesReportedWithFirstSubscription = [device unitTestAttributesReportedSinceLastCheck];

Expand All @@ -2879,10 +2883,17 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
__strong __auto_type strongDelegate = weakDelegate;
strongDelegate.onReportEnd = nil;
};
__block BOOL onDeviceCachePrimedCalled = NO;
delegate.onDeviceCachePrimed = ^{
onDeviceCachePrimedCalled = YES;
};
[device setDelegate:delegate queue:queue];

[self waitForExpectations:@[ resubGotReportsExpectation ] timeout:60];

// Make sure that the new callback is only ever called once, the first time subscription was primed
XCTAssertFalse(onDeviceCachePrimedCalled);

NSUInteger attributesReportedWithSecondSubscription = [device unitTestAttributesReportedSinceLastCheck];

XCTAssertTrue(attributesReportedWithSecondSubscription < attributesReportedWithFirstSubscription);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ typedef void (^MTRDeviceTestDelegateDataHandler)(NSArray<NSDictionary<NSString *
@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onAttributeDataReceived;
@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onEventDataReceived;
@property (nonatomic, nullable) dispatch_block_t onReportEnd;
@property (nonatomic, nullable) dispatch_block_t onDeviceCachePrimed;
@end

NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,11 @@ - (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device
return @(2); // seconds
}

- (void)deviceCachePrimed:(MTRDevice *)device
{
if (self.onDeviceCachePrimed != nil) {
self.onDeviceCachePrimed();
}
}

@end

0 comments on commit ed8f37d

Please sign in to comment.