Skip to content

Commit ed8f37d

Browse files
[Darwin] MTRDevice delegate should be notified when cache is primed with basic info (project-chip#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 <bzbarsky@apple.com> --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent db76ad7 commit ed8f37d

File tree

5 files changed

+111
-5
lines changed

5 files changed

+111
-5
lines changed

src/darwin/Framework/CHIP/MTRDevice.h

+9
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,15 @@ MTR_EXTERN NSString * const MTRDataVersionKey MTR_NEWLY_AVAILABLE;
381381
*/
382382
- (void)deviceBecameActive:(MTRDevice *)device MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
383383

384+
/**
385+
* Notifies delegate when the device attribute cache has been primed with initial configuration data of the device
386+
*
387+
* 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.
388+
*
389+
* 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.
390+
*/
391+
- (void)deviceCachePrimed:(MTRDevice *)device MTR_NEWLY_AVAILABLE;
392+
384393
@end
385394

386395
@interface MTRDevice (Deprecated)

src/darwin/Framework/CHIP/MTRDevice.mm

+81-3
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ @implementation MTRDevice {
225225
#ifdef DEBUG
226226
NSUInteger _unitTestAttributesReportedSinceLastCheck;
227227
#endif
228+
BOOL _delegateDeviceCachePrimedCalled;
228229
}
229230

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

506+
// If Check if cache is already primed and client hasn't been informed yet, call the -deviceCachePrimed: callback
507+
if (!_delegateDeviceCachePrimedCalled && [self _isCachePrimedWithInitialConfigurationData]) {
508+
[self _callDelegateDeviceCachePrimed];
509+
}
510+
505511
if (setUpSubscription) {
506512
[self _setupSubscription];
507513
}
@@ -574,6 +580,29 @@ - (BOOL)_subscriptionAbleToReport
574580
return (delegate != nil) && (state == MTRDeviceStateReachable);
575581
}
576582

583+
- (BOOL)_callDelegateWithBlock:(void (^)(id<MTRDeviceDelegate>))block
584+
{
585+
os_unfair_lock_assert_owner(&self->_lock);
586+
id<MTRDeviceDelegate> delegate = _weakDelegate.strongObject;
587+
if (delegate) {
588+
dispatch_async(_delegateQueue, ^{
589+
block(delegate);
590+
});
591+
return YES;
592+
}
593+
return NO;
594+
}
595+
596+
- (void)_callDelegateDeviceCachePrimed
597+
{
598+
os_unfair_lock_assert_owner(&self->_lock);
599+
_delegateDeviceCachePrimedCalled = [self _callDelegateWithBlock:^(id<MTRDeviceDelegate> delegate) {
600+
if ([delegate respondsToSelector:@selector(deviceCachePrimed:)]) {
601+
[delegate deviceCachePrimed:self];
602+
}
603+
}];
604+
}
605+
577606
// assume lock is held
578607
- (void)_changeState:(MTRDeviceState)state
579608
{
@@ -611,6 +640,11 @@ - (void)_handleSubscriptionEstablished
611640
// reset subscription attempt wait time when subscription succeeds
612641
_lastSubscriptionAttemptWait = 0;
613642

643+
// As subscription is established, check if the delegate needs to be informed
644+
if (!_delegateDeviceCachePrimedCalled) {
645+
[self _callDelegateDeviceCachePrimed];
646+
}
647+
614648
[self _changeState:MTRDeviceStateReachable];
615649

616650
os_unfair_lock_unlock(&self->_lock);
@@ -741,6 +775,7 @@ - (void)_handleReportEnd
741775
_receivingReport = NO;
742776
_receivingPrimingReport = NO;
743777
_estimatedStartTimeFromGeneralDiagnosticsUpTime = nil;
778+
744779
// For unit testing only
745780
#ifdef DEBUG
746781
id delegate = _weakDelegate.strongObject;
@@ -1948,17 +1983,24 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
19481983

19491984
- (void)setAttributeValues:(NSArray<NSDictionary *> *)attributeValues reportChanges:(BOOL)reportChanges
19501985
{
1986+
os_unfair_lock_lock(&self->_lock);
1987+
19511988
if (reportChanges) {
1952-
[self _handleAttributeReport:attributeValues];
1989+
[self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeValues]];
19531990
} else {
1954-
os_unfair_lock_lock(&self->_lock);
19551991
for (NSDictionary * responseValue in attributeValues) {
19561992
MTRAttributePath * path = responseValue[MTRAttributePathKey];
19571993
NSDictionary * dataValue = responseValue[MTRDataKey];
19581994
_readCache[path] = dataValue;
19591995
}
1960-
os_unfair_lock_unlock(&self->_lock);
19611996
}
1997+
1998+
// 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
1999+
if ([self _isCachePrimedWithInitialConfigurationData]) {
2000+
_delegateDeviceCachePrimedCalled = YES;
2001+
}
2002+
2003+
os_unfair_lock_unlock(&self->_lock);
19622004
}
19632005

19642006
// If value is non-nil, associate with expectedValueID
@@ -2135,6 +2177,42 @@ - (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath e
21352177
}
21362178
}
21372179

2180+
// This method checks if there is a need to inform delegate that the attribute cache has been "primed"
2181+
- (BOOL)_isCachePrimedWithInitialConfigurationData
2182+
{
2183+
os_unfair_lock_assert_owner(&self->_lock);
2184+
2185+
// Check if root node descriptor exists
2186+
NSDictionary * rootDescriptorPartsListDataValue = _readCache[[MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeDescriptorID) attributeID:@(MTRAttributeIDTypeClusterDescriptorAttributePartsListID)]];
2187+
if (!rootDescriptorPartsListDataValue || ![MTRArrayValueType isEqualToString:rootDescriptorPartsListDataValue[MTRTypeKey]]) {
2188+
return NO;
2189+
}
2190+
NSArray * partsList = rootDescriptorPartsListDataValue[MTRValueKey];
2191+
if (![partsList isKindOfClass:[NSArray class]] || !partsList.count) {
2192+
MTR_LOG_ERROR("%@ unexpected type %@ for parts list %@", self, [partsList class], partsList);
2193+
return NO;
2194+
}
2195+
2196+
// Check if we have cached descriptor clusters for each listed endpoint
2197+
for (NSDictionary * endpointDataValue in partsList) {
2198+
if (![MTRUnsignedIntegerValueType isEqual:endpointDataValue[MTRTypeKey]]) {
2199+
MTR_LOG_ERROR("%@ unexpected type for parts list item %@", self, endpointDataValue);
2200+
continue;
2201+
}
2202+
NSNumber * endpoint = endpointDataValue[MTRValueKey];
2203+
if (![endpoint isKindOfClass:[NSNumber class]]) {
2204+
MTR_LOG_ERROR("%@ unexpected type for parts list item %@", self, endpointDataValue);
2205+
continue;
2206+
}
2207+
NSDictionary * descriptorDeviceTypeListDataValue = _readCache[[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:@(MTRClusterIDTypeDescriptorID) attributeID:@(MTRAttributeIDTypeClusterDescriptorAttributeDeviceTypeListID)]];
2208+
if (![MTRArrayValueType isEqualToString:descriptorDeviceTypeListDataValue[MTRTypeKey]] || !descriptorDeviceTypeListDataValue[MTRValueKey]) {
2209+
return NO;
2210+
}
2211+
}
2212+
2213+
return YES;
2214+
}
2215+
21382216
- (MTRBaseDevice *)newBaseDevice
21392217
{
21402218
return [MTRBaseDevice deviceWithNodeID:self.nodeID controller:self.deviceController];

src/darwin/Framework/CHIPTests/MTRDeviceTests.m

+13-2
Original file line numberDiff line numberDiff line change
@@ -2843,7 +2843,7 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
28432843
{
28442844
dispatch_queue_t queue = dispatch_get_main_queue();
28452845

2846-
// First start with clean slate and
2846+
// First start with clean slate by removing the MTRDevice and clearing the persisted cache
28472847
__auto_type * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:sController];
28482848
[sController removeDevice:device];
28492849
[sController.controllerDataStore clearAllStoredAttributes];
@@ -2853,16 +2853,20 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
28532853
// Now recreate device and get subscription primed
28542854
device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:sController];
28552855
XCTestExpectation * gotReportsExpectation = [self expectationWithDescription:@"Attribute and Event reports have been received"];
2856+
XCTestExpectation * gotDeviceCachePrimed = [self expectationWithDescription:@"Device cache primed for the first time"];
28562857
__auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
28572858
__weak __auto_type weakDelegate = delegate;
28582859
delegate.onReportEnd = ^{
28592860
[gotReportsExpectation fulfill];
28602861
__strong __auto_type strongDelegate = weakDelegate;
28612862
strongDelegate.onReportEnd = nil;
28622863
};
2864+
delegate.onDeviceCachePrimed = ^{
2865+
[gotDeviceCachePrimed fulfill];
2866+
};
28632867
[device setDelegate:delegate queue:queue];
28642868

2865-
[self waitForExpectations:@[ gotReportsExpectation ] timeout:60];
2869+
[self waitForExpectations:@[ gotReportsExpectation, gotDeviceCachePrimed ] timeout:60];
28662870

28672871
NSUInteger attributesReportedWithFirstSubscription = [device unitTestAttributesReportedSinceLastCheck];
28682872

@@ -2879,10 +2883,17 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
28792883
__strong __auto_type strongDelegate = weakDelegate;
28802884
strongDelegate.onReportEnd = nil;
28812885
};
2886+
__block BOOL onDeviceCachePrimedCalled = NO;
2887+
delegate.onDeviceCachePrimed = ^{
2888+
onDeviceCachePrimedCalled = YES;
2889+
};
28822890
[device setDelegate:delegate queue:queue];
28832891

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

2894+
// Make sure that the new callback is only ever called once, the first time subscription was primed
2895+
XCTAssertFalse(onDeviceCachePrimedCalled);
2896+
28862897
NSUInteger attributesReportedWithSecondSubscription = [device unitTestAttributesReportedSinceLastCheck];
28872898

28882899
XCTAssertTrue(attributesReportedWithSecondSubscription < attributesReportedWithFirstSubscription);

src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.h

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ typedef void (^MTRDeviceTestDelegateDataHandler)(NSArray<NSDictionary<NSString *
2727
@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onAttributeDataReceived;
2828
@property (nonatomic, nullable) MTRDeviceTestDelegateDataHandler onEventDataReceived;
2929
@property (nonatomic, nullable) dispatch_block_t onReportEnd;
30+
@property (nonatomic, nullable) dispatch_block_t onDeviceCachePrimed;
3031
@end
3132

3233
NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m

+7
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,11 @@ - (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device
5353
return @(2); // seconds
5454
}
5555

56+
- (void)deviceCachePrimed:(MTRDevice *)device
57+
{
58+
if (self.onDeviceCachePrimed != nil) {
59+
self.onDeviceCachePrimed();
60+
}
61+
}
62+
5663
@end

0 commit comments

Comments
 (0)