diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index ffb0a5643d463e..4e916d14b101c8 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -376,7 +376,11 @@ @implementation MTRDevice { #ifdef DEBUG NSUInteger _unitTestAttributesReportedSinceLastCheck; #endif - BOOL _delegateDeviceCachePrimedCalled; + + // _deviceCachePrimed is true if we have the data that comes from an initial + // subscription priming report (whether it came from storage or from our + // subscription). + BOOL _deviceCachePrimed; // _persistedClusterData stores data that we have already persisted (when we have // cluster data persistence enabled). Nil when we have no persistence enabled. @@ -736,11 +740,6 @@ - (void)setDelegate:(id)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) { _initialSubscribeStart = [NSDate now]; if ([self _deviceUsesThread]) { @@ -915,7 +914,7 @@ - (BOOL)_callDelegateWithBlock:(void (^)(id))block - (void)_callDelegateDeviceCachePrimed { os_unfair_lock_assert_owner(&self->_lock); - _delegateDeviceCachePrimedCalled = [self _callDelegateWithBlock:^(id delegate) { + [self _callDelegateWithBlock:^(id delegate) { if ([delegate respondsToSelector:@selector(deviceCachePrimed:)]) { [delegate deviceCachePrimed:self]; } @@ -994,11 +993,6 @@ - (void)_handleSubscriptionEstablished [self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished]; } - // As subscription is established, check if the delegate needs to be informed - if (!_delegateDeviceCachePrimedCalled) { - [self _callDelegateDeviceCachePrimed]; - } - [self _changeState:MTRDeviceStateReachable]; // No need to monitor connectivity after subscription establishment @@ -1480,6 +1474,13 @@ - (void)_scheduleClusterDataPersistence return; } + // If we have nothing stored at all yet, store directly, so we move into a + // primed state. + if (!_deviceCachePrimed) { + [self _persistClusterData]; + return; + } + // Ensure there is an array to keep the most recent report times if (!_mostRecentReportTimes) { _mostRecentReportTimes = [NSMutableArray array]; @@ -1525,7 +1526,7 @@ - (void)_scheduleClusterDataPersistence // Set current multiplier to [1, MaxMultiplier] _reportToPersistenceDelayCurrentMultiplier = 1 + (proportionTowardMinThreshold * (_storageBehaviorConfiguration.reportToPersistenceDelayMaxMultiplier - 1)); - MTR_LOG("%@ storage behavior: device reporting frequently - setting delay multiplied to %lf", self, _reportToPersistenceDelayCurrentMultiplier); + MTR_LOG("%@ storage behavior: device reporting frequently - setting delay multiplier to %lf", self, _reportToPersistenceDelayCurrentMultiplier); } else { _reportToPersistenceDelayCurrentMultiplier = 1; } @@ -1601,6 +1602,19 @@ - (void)_handleReportEnd _deviceConfigurationChanged = NO; } + // Do this after the _deviceConfigurationChanged check, so that we don't + // call deviceConfigurationChanged: immediately after telling our delegate + // we are now primed. + // + // TODO: Maybe we shouldn't dispatch deviceConfigurationChanged: for the + // initial priming bits? + if (!_deviceCachePrimed) { + // This is the end of the priming sequence of data reports, so we have + // all the data for the device now. + _deviceCachePrimed = YES; + [self _callDelegateDeviceCachePrimed]; + } + // For unit testing only #ifdef DEBUG id delegate = _weakDelegate.strongObject; @@ -3165,10 +3179,9 @@ - (void)setPersistedClusterData:(NSDictionary_lock); - - // Check if root node descriptor exists - MTRDeviceDataValueDictionary rootDescriptorPartsListDataValue = [self _cachedAttributeValueForPath:[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 * endpointDictionary in partsList) { - NSDictionary * endpointDataValue = endpointDictionary[MTRDataKey]; - if (![endpointDataValue isKindOfClass:[NSDictionary class]]) { - MTR_LOG_ERROR("%@ unexpected parts list dictionary %@ data value class %@", self, endpointDictionary, [endpointDataValue class]); - continue; - } - if (![MTRUnsignedIntegerValueType isEqual:endpointDataValue[MTRTypeKey]]) { - MTR_LOG_ERROR("%@ unexpected parts list data value %@ item type %@", self, endpointDataValue, endpointDataValue[MTRTypeKey]); - continue; - } - NSNumber * endpoint = endpointDataValue[MTRValueKey]; - if (![endpoint isKindOfClass:[NSNumber class]]) { - MTR_LOG_ERROR("%@ unexpected parts list item value class %@", self, [endpoint class]); - continue; - } - MTRDeviceDataValueDictionary descriptorDeviceTypeListDataValue = [self _cachedAttributeValueForPath:[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]; diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 7a26b31aa62b21..f0d6aee1a8556b 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -3669,7 +3669,7 @@ - (void)test035_TestMTRDeviceSubscriptionNotEstablishedOverXPC XCTAssertEqual([device _getInternalState], MTRInternalDeviceStateUnsubscribed); } -- (NSArray *> *)testAttributeReportWithValue:(unsigned int)testValue +- (NSArray *> *)_testAttributeReportWithValue:(unsigned int)testValue { return @[ @{ MTRAttributePathKey : [MTRAttributePath attributePathWithEndpointID:@(0) clusterID:@(MTRClusterIDTypeLevelControlID) attributeID:@(MTRAttributeIDTypeClusterLevelControlAttributeCurrentLevelID)], @@ -3696,7 +3696,7 @@ - (void)test036_TestStorageBehaviorConfiguration __block NSDate * reportEndTime = nil; __block NSDate * dataPersistedTime = nil; - XCTestExpectation * dataPersisted1 = [self expectationWithDescription:@"data persisted 1"]; + XCTestExpectation * dataPersistedInitial = [self expectationWithDescription:@"data persisted initial"]; delegate.onReportEnd = ^() { os_unfair_lock_lock(&lock); if (!reportEndTime) { @@ -3711,7 +3711,7 @@ - (void)test036_TestStorageBehaviorConfiguration dataPersistedTime = [NSDate now]; } os_unfair_lock_unlock(&lock); - [dataPersisted1 fulfill]; + [dataPersistedInitial fulfill]; }; // Do not subscribe - only inject sequence of reports to control the timing @@ -3732,11 +3732,29 @@ - (void)test036_TestStorageBehaviorConfiguration [device setDelegate:delegate queue:queue]; - // Use a mutable dictionary so the data value can be easily changed between reports + // Use a counter that will be incremented for each report as the value. unsigned int currentTestValue = 1; + // Initial setup: Inject report and see that the attribute persisted. No delay is + // expected for the first (priming) report. + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + + [self waitForExpectations:@[ dataPersistedInitial ] timeout:60]; + + XCTestExpectation * dataPersisted1 = [self expectationWithDescription:@"data persisted 1"]; + delegate.onClusterDataPersisted = ^{ + os_unfair_lock_lock(&lock); + if (!dataPersistedTime) { + dataPersistedTime = [NSDate now]; + } + os_unfair_lock_unlock(&lock); + [dataPersisted1 fulfill]; + }; + // Test 1: Inject report and see that the attribute persisted, with a delay - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + reportEndTime = nil; + dataPersistedTime = nil; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; [self waitForExpectations:@[ dataPersisted1 ] timeout:60]; @@ -3762,20 +3780,20 @@ - (void)test036_TestStorageBehaviorConfiguration // Test 2: Inject multiple reports with delay and see that the attribute persisted eventually reportEndTime = nil; dataPersistedTime = nil; - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; double frequentReportMultiplier = 0.5; usleep((useconds_t) (baseTestDelayTime * frequentReportMultiplier * USEC_PER_SEC)); - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; usleep((useconds_t) (baseTestDelayTime * frequentReportMultiplier * USEC_PER_SEC)); - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; usleep((useconds_t) (baseTestDelayTime * frequentReportMultiplier * USEC_PER_SEC)); - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; usleep((useconds_t) (baseTestDelayTime * frequentReportMultiplier * USEC_PER_SEC)); - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; // At this point, the threshold for reportToPersistenceDelayTimeMax should have hit, and persistence // should have happened with timer running down to persist again with the 5th report above. Need to @@ -3817,7 +3835,7 @@ - (void)test036_TestStorageBehaviorConfiguration ]]]; // Inject final report that makes MTRDevice recalculate delay with multiplier - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; [self waitForExpectations:@[ dataPersisted3 ] timeout:60]; @@ -3856,13 +3874,13 @@ - (void)test036_TestStorageBehaviorConfiguration ]]]; // Inject report that makes MTRDevice detect the device is reporting excessively - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; // Now keep reporting excessively for base delay time max times max multiplier, plus a bit more NSDate * excessiveStartTime = [NSDate now]; for (;;) { usleep((useconds_t) (baseTestDelayTime * 0.1 * USEC_PER_SEC)); - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; NSTimeInterval elapsed = -[excessiveStartTime timeIntervalSinceNow]; if (elapsed > (baseTestDelayTime * 2 * 5 * 1.2)) { break; @@ -3879,7 +3897,7 @@ - (void)test036_TestStorageBehaviorConfiguration // And inject a report to trigger MTRDevice to recalculate that this device is no longer // reporting excessively - [device unitTestInjectAttributeReport:[self testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; [self waitForExpectations:@[ dataPersisted4 ] timeout:60]; diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 601cbcc9125536..52b1ae36673b28 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -1437,29 +1437,29 @@ - (void)doDataStoreMTRDeviceTestWithStorageDelegate:(id * dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:deviceID]; @@ -1498,6 +1498,9 @@ - (void)doDataStoreMTRDeviceTestWithStorageDelegate:(id