From c75f454258e99c7e826a1757252c404a3209c9c6 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Mon, 29 Jan 2024 21:15:15 -0800 Subject: [PATCH] [Darwin] MTRDevice attribute reports should include previous value if possible --- src/darwin/Framework/CHIP/MTRBaseDevice.h | 3 ++ src/darwin/Framework/CHIP/MTRBaseDevice.mm | 1 + src/darwin/Framework/CHIP/MTRDevice.mm | 44 +++++++++++++++---- .../Framework/CHIPTests/MTRDeviceTests.m | 32 ++++++++++++++ 4 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.h b/src/darwin/Framework/CHIP/MTRBaseDevice.h index 64c8fd99981595..6dd14a83fd2e1a 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.h +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.h @@ -55,6 +55,8 @@ NS_ASSUME_NONNULL_BEGIN * MTREventIsHistoricalKey : NSNumber-wrapped BOOL value. * Value is YES if the event is in the far past or not realtime. * Only present when MTREventPathKey is present. + * MTRPreviousDataKey: Data-value NSDictionary object. + * Included when there is MTRDataKey and when a previous value is known. * * Only one of MTREventTimestampDateKey and MTREventSystemUpTimeKey will be present, depending on the value for * MTREventTimeTypeKey. @@ -145,6 +147,7 @@ MTR_EXTERN NSString * const MTREventTimeTypeKey MTR_AVAILABLE(ios(16.5), macos(1 MTR_EXTERN NSString * const MTREventSystemUpTimeKey MTR_AVAILABLE(ios(16.5), macos(13.4), watchos(9.5), tvos(16.5)); MTR_EXTERN NSString * const MTREventTimestampDateKey MTR_AVAILABLE(ios(16.5), macos(13.4), watchos(9.5), tvos(16.5)); MTR_EXTERN NSString * const MTREventIsHistoricalKey MTR_AVAILABLE(ios(17.3), macos(14.3), watchos(10.3), tvos(17.3)); +MTR_EXTERN NSString * const MTRPreviousDataKey MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4)); @class MTRClusterStateCacheContainer; @class MTRAttributeCacheContainer; diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index a14986d2e24f5a..3b95c937f4283d 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -87,6 +87,7 @@ NSString * const MTREventSystemUpTimeKey = @"eventSystemUpTime"; NSString * const MTREventTimestampDateKey = @"eventTimestampDate"; NSString * const MTREventIsHistoricalKey = @"eventIsHistorical"; +NSString * const MTRPreviousDataKey = @"previousData"; class MTRDataValueDictionaryCallbackBridge; diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index f25ac5b9564187..d97ce3aad8847a 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1413,7 +1413,7 @@ - (void)_checkExpiredExpectedValues NSDictionary * cachedAttributeDataValue = _readCache[attributePath]; if (cachedAttributeDataValue && ![self _attributeDataValue:attributeDataValue isEqualToDataValue:cachedAttributeDataValue]) { - [attributesToReport addObject:@{ MTRAttributePathKey : attributePath, MTRDataKey : cachedAttributeDataValue }]; + [attributesToReport addObject:@{ MTRAttributePathKey : attributePath, MTRDataKey : cachedAttributeDataValue, MTRPreviousDataKey : attributeDataValue }]; [attributePathsToReport addObject:attributePath]; } @@ -1505,6 +1505,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray *)expectedAttributeValue attributePath:(MTRAttributePath *)attributePath expirationTime:(NSDate *)expirationTime shouldReportValue:(BOOL *)shouldReportValue attributeValueToReport:(NSDictionary **)attributeValueToReport expectedValueID:(uint64_t)expectedValueID + previousValue:(NSDictionary **)previousValue { os_unfair_lock_assert_owner(&self->_lock); @@ -1604,6 +1616,7 @@ - (void)_setExpectedValue:(NSDictionary *)expectedAttributeValue // Case where new expected value overrides previous expected value - report new expected value *shouldReportValue = YES; *attributeValueToReport = expectedAttributeValue; + *previousValue = previousExpectedValue[MTRDeviceExpectedValueFieldValueIndex]; } else if (!expectedAttributeValue) { // Remove previous expected value only if it's from the same setExpectedValues operation NSNumber * previousExpectedValueID = previousExpectedValue[MTRDeviceExpectedValueFieldIDIndex]; @@ -1613,6 +1626,7 @@ - (void)_setExpectedValue:(NSDictionary *)expectedAttributeValue // Case of removing expected value that is different than read cache - report read cache value *shouldReportValue = YES; *attributeValueToReport = _readCache[attributePath]; + *previousValue = previousExpectedValue[MTRDeviceExpectedValueFieldValueIndex]; _expectedValueCache[attributePath] = nil; } } @@ -1623,6 +1637,9 @@ - (void)_setExpectedValue:(NSDictionary *)expectedAttributeValue // Case where new expected value is different than read cache - report new expected value *shouldReportValue = YES; *attributeValueToReport = expectedAttributeValue; + *previousValue = _readCache[attributePath]; + } else { + *previousValue = nil; } // No need to report if new and previous expected value are both nil @@ -1649,15 +1666,21 @@ - (NSArray *)_getAttributesToReportWithNewExpectedValues:(NSArray * attributeValueToReport; + NSDictionary * previousValue; [self _setExpectedValue:attributeDataValue attributePath:attributePath expirationTime:expirationTime shouldReportValue:&shouldReportValue attributeValueToReport:&attributeValueToReport - expectedValueID:expectedValueIDToReturn]; + expectedValueID:expectedValueIDToReturn + previousValue:&previousValue]; if (shouldReportValue) { - [attributesToReport addObject:@{ MTRAttributePathKey : attributePath, MTRDataKey : attributeValueToReport }]; + if (previousValue) { + [attributesToReport addObject:@{ MTRAttributePathKey : attributePath, MTRDataKey : attributeValueToReport, MTRPreviousDataKey : previousValue }]; + } else { + [attributesToReport addObject:@{ MTRAttributePathKey : attributePath, MTRDataKey : attributeValueToReport }]; + } [attributePathsToReport addObject:attributePath]; } } @@ -1721,21 +1744,26 @@ - (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath e BOOL shouldReportValue; NSDictionary * attributeValueToReport; + NSDictionary * previousValue; [self _setExpectedValue:nil attributePath:attributePath expirationTime:nil shouldReportValue:&shouldReportValue attributeValueToReport:&attributeValueToReport - expectedValueID:expectedValueID]; + expectedValueID:expectedValueID + previousValue:&previousValue]; MTR_LOG_INFO("%@ remove expected value for path %@ should report %@", self, attributePath, shouldReportValue ? @"YES" : @"NO"); if (shouldReportValue) { + NSMutableDictionary * attribute = [NSMutableDictionary dictionaryWithObject:attributePath forKey:MTRAttributePathKey]; if (attributeValueToReport) { - [self _reportAttributes:@[ @{ MTRAttributePathKey : attributePath, MTRDataKey : attributeValueToReport } ]]; - } else { - [self _reportAttributes:@[ @{ MTRAttributePathKey : attributePath } ]]; + attribute[MTRDataKey] = attributeValueToReport; + } + if (previousValue) { + attribute[MTRPreviousDataKey] = previousValue; } + [self _reportAttributes:@[ attribute ]]; } } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 645fdc0acf7415..ffe7fa989cd50e 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -1575,6 +1575,38 @@ - (void)test017_TestMTRDeviceBasics // nonexistent attribute [self waitForExpectations:@[ expectedValueReportedExpectation, expectedValueRemovedExpectation ] timeout:5 enforceOrder:YES]; + // Test if previous value is reported on a write + uint16_t testOnTimeValue = 10; + XCTestExpectation * onTimeWriteSuccess = [self expectationWithDescription:@"OnTime write success"]; + XCTestExpectation * onTimePreviousValue = [self expectationWithDescription:@"OnTime previous value"]; + delegate.onAttributeDataReceived = ^(NSArray *> * data) { + for (NSDictionary * attributeReponseValue in data) { + MTRAttributePath * path = attributeReponseValue[MTRAttributePathKey]; + if (path.cluster.unsignedIntValue == MTRClusterIDTypeOnOffID && path.attribute.unsignedLongValue == MTRAttributeIDTypeClusterOnOffAttributeOnTimeID) { + NSDictionary * dataValue = attributeReponseValue[MTRDataKey]; + NSNumber * onTimeValue = dataValue[MTRValueKey]; + if (onTimeValue && (onTimeValue.unsignedIntValue == testOnTimeValue)) { + [onTimeWriteSuccess fulfill]; + } + + NSDictionary * previousDataValue = attributeReponseValue[MTRPreviousDataKey]; + NSNumber * previousOnTimeValue = previousDataValue[MTRValueKey]; + if (previousOnTimeValue) { + [onTimePreviousValue fulfill]; + } + } + } + }; + NSDictionary * writeOnTimeValue = @{ MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(testOnTimeValue) }; + [device writeAttributeWithEndpointID:@(1) + clusterID:@(MTRClusterIDTypeOnOffID) + attributeID:@(MTRAttributeIDTypeClusterOnOffAttributeOnTimeID) + value:writeOnTimeValue + expectedValueInterval:@(10000) + timedWriteTimeout:nil]; + + [self waitForExpectations:@[ onTimeWriteSuccess, onTimePreviousValue ] timeout:10]; + // Test if errors are properly received XCTestExpectation * attributeReportErrorExpectation = [self expectationWithDescription:@"Attribute read error"]; delegate.onAttributeDataReceived = ^(NSArray *> * data) {