From b0f527575d4b0e808f0cb06fd192707d38e6de7a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 24 Sep 2024 18:14:25 -0400 Subject: [PATCH] MTRDevice should remember new DataVersion values even if the attribute value did not change. (#35756) We used to ignore "same value but new DataVersion" because some devices would spam reports that looked like that and we were constantly hitting storage for the DataVersion updates. But now that we have backoffs on the storage of our cluster data, we can go ahead and just note the new DataVersion and rely on that backoff to prevent hitting storage too much. The test update verifies that: 1) Reports with same value but new DataVersion do get persisted but are subject to the persistence backoff settings. 2) Reports with the same value and same DataVersion do not lead to any storage traffic. --- .../Framework/CHIP/MTRDevice_Concrete.mm | 6 +- .../Framework/CHIPTests/MTRDeviceTests.m | 123 +++++++++++------- 2 files changed, 81 insertions(+), 48 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 4617a78b161a2a..41962667a8a2fb 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3533,6 +3533,8 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray *> *)_testAttributeReportWithValue:(unsigned int)testValue +{ + return [self _testAttributeReportWithValue:testValue dataVersion:testValue]; +} + +- (NSArray *> *)_testAttributeReportWithValue:(unsigned int)testValue dataVersion:(unsigned int)dataVersion { return @[ @{ MTRAttributePathKey : [MTRAttributePath attributePathWithEndpointID:@(0) clusterID:@(MTRClusterIDTypeLevelControlID) attributeID:@(MTRAttributeIDTypeClusterLevelControlAttributeCurrentLevelID)], MTRDataKey : @ { - MTRDataVersionKey : @(testValue), + MTRDataVersionKey : @(dataVersion), MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(testValue), } @@ -3809,7 +3814,7 @@ - (void)test036_TestStorageBehaviorConfiguration [device setDelegate:delegate queue:queue]; // Use a counter that will be incremented for each report as the value. - unsigned int currentTestValue = 1; + __block unsigned int currentTestValue = 1; // Initial setup: Inject report and see that the attribute persisted. No delay is // expected for the first (priming) report. @@ -3928,54 +3933,84 @@ - (void)test036_TestStorageBehaviorConfiguration XCTAssertLessThan(reportToPersistenceDelay, baseTestDelayTime * 2 * 5 * 1.3); // Test 4: test reporting excessively, and see that persistence does not happen until - // reporting frequency goes back above the threshold - reportEndTime = nil; - dataPersistedTime = nil; - XCTestExpectation * dataPersisted4 = [self expectationWithDescription:@"data persisted 4"]; - delegate.onClusterDataPersisted = ^{ - os_unfair_lock_lock(&lock); - if (!dataPersistedTime) { - dataPersistedTime = [NSDate now]; - } - os_unfair_lock_unlock(&lock); - [dataPersisted4 fulfill]; - }; - - // Set report times with short delay and check that the multiplier is engaged - [device unitTestSetMostRecentReportTimes:[NSMutableArray arrayWithArray:@[ - [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 4)], - [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 3)], - [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 2)], - [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1)], - ]]]; - - // Inject report that makes MTRDevice detect the device is reporting excessively - [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + // reporting frequency goes back below the threshold + __auto_type excessiveReportTest = ^(unsigned int testId, NSArray *> * (^reportGenerator)(void), bool expectPersistence) { + reportEndTime = nil; + dataPersistedTime = nil; + XCTestExpectation * dataPersisted = [self expectationWithDescription:[NSString stringWithFormat:@"data persisted %u", testId]]; + dataPersisted.inverted = !expectPersistence; + delegate.onClusterDataPersisted = ^{ + os_unfair_lock_lock(&lock); + if (!dataPersistedTime) { + dataPersistedTime = [NSDate now]; + } + os_unfair_lock_unlock(&lock); + [dataPersisted fulfill]; + }; - // 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]; - NSTimeInterval elapsed = -[excessiveStartTime timeIntervalSinceNow]; - if (elapsed > (baseTestDelayTime * 2 * 5 * 1.2)) { - break; + // Set report times with short delay and check that the multiplier is engaged + [device unitTestSetMostRecentReportTimes:[NSMutableArray arrayWithArray:@[ + [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 4)], + [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 3)], + [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1 * 2)], + [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 0.1)], + ]]]; + + // Inject report that makes MTRDevice detect the device is reporting excessively + [device unitTestInjectAttributeReport:reportGenerator() 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:reportGenerator() fromSubscription:YES]; + NSTimeInterval elapsed = -[excessiveStartTime timeIntervalSinceNow]; + if (elapsed > (baseTestDelayTime * 2 * 5 * 1.2)) { + break; + } } - } - // Check that persistence has not happened because it's now turned off - XCTAssertNil(dataPersistedTime); + // Check that persistence has not happened because it's now turned off + XCTAssertNil(dataPersistedTime); - // Now force report times to large number, to simulate time passage - [device unitTestSetMostRecentReportTimes:[NSMutableArray arrayWithArray:@[ - [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 10)], - ]]]; + // Now force report times to large number, to simulate time passage + [device unitTestSetMostRecentReportTimes:[NSMutableArray arrayWithArray:@[ + [NSDate dateWithTimeIntervalSinceNow:-(baseTestDelayTime * 10)], + ]]]; - // And inject a report to trigger MTRDevice to recalculate that this device is no longer - // reporting excessively - [device unitTestInjectAttributeReport:[self _testAttributeReportWithValue:currentTestValue++] fromSubscription:YES]; + // And inject a report to trigger MTRDevice to recalculate that this device is no longer + // reporting excessively + [device unitTestInjectAttributeReport:reportGenerator() fromSubscription:YES]; + + [self waitForExpectations:@[ dataPersisted ] timeout:60]; + }; - [self waitForExpectations:@[ dataPersisted4 ] timeout:60]; + excessiveReportTest( + 4, ^{ + return [self _testAttributeReportWithValue:currentTestValue++]; + }, true); + + // Test 5: test reporting excessively with the same value and different data + // versions, and see that persistence does not happen until reporting + // frequency goes back below the threshold. + __block __auto_type dataVersion = currentTestValue; + // We incremented currentTestValue after injecting the last report. Make sure all the new + // reports use that last-reported value. + __auto_type lastReportedValue = currentTestValue - 1; + excessiveReportTest( + 5, ^{ + return [self _testAttributeReportWithValue:lastReportedValue dataVersion:dataVersion++]; + }, true); + + // Test 6: test reporting excessively with the same value and same data + // version, and see that persistence does not happen at all. + // We incremented dataVersion after injecting the last report. Make sure all the new + // reports use that last-reported value. + __block __auto_type lastReportedDataVersion = dataVersion - 1; + excessiveReportTest( + 6, ^{ + return [self _testAttributeReportWithValue:lastReportedValue dataVersion:lastReportedDataVersion]; + }, false); delegate.onReportEnd = nil; delegate.onClusterDataPersisted = nil;