Skip to content

Commit

Permalink
[Darwin] MTRDevice attribute reports should include previous value if…
Browse files Browse the repository at this point in the history 
… possible
  • Loading branch information
@jtung-apple
jtung-apple committed Jan 30, 2024
1 parent f4400c5 commit c75f454
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 8 deletions.
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
NSString * const MTREventSystemUpTimeKey = @"eventSystemUpTime";
NSString * const MTREventTimestampDateKey = @"eventTimestampDate";
NSString * const MTREventIsHistoricalKey = @"eventIsHistorical";
NSString * const MTRPreviousDataKey = @"previousData";

class MTRDataValueDictionaryCallbackBridge;

Expand Down
44 changes: 36 additions & 8 deletions src/darwin/Framework/CHIP/MTRDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down Expand Up @@ -1505,6 +1505,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
MTRAttributePath * attributePath = attributeReponseValue[MTRAttributePathKey];
NSDictionary * attributeDataValue = attributeReponseValue[MTRDataKey];
NSError * attributeError = attributeReponseValue[MTRErrorKey];
NSDictionary * previousValue;

// sanity check either data value or error must exist
if (!attributeDataValue && !attributeError) {
Expand All @@ -1526,6 +1527,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
MTR_LOG_INFO("%@ report %@ error %@ purge expected value %@ read cache %@", self, attributePath, attributeError,
_expectedValueCache[attributePath], _readCache[attributePath]);
_expectedValueCache[attributePath] = nil;
previousValue = _readCache[attributePath];
_readCache[attributePath] = nil;
} else {
// if expected values exists, purge and update read cache
Expand All @@ -1534,11 +1536,13 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
if (![self _attributeDataValue:attributeDataValue
isEqualToDataValue:expectedValue[MTRDeviceExpectedValueFieldValueIndex]]) {
shouldReportAttribute = YES;
previousValue = expectedValue[MTRDeviceExpectedValueFieldValueIndex];
}
_expectedValueCache[attributePath] = nil;
_readCache[attributePath] = attributeDataValue;
} else if (![self _attributeDataValue:attributeDataValue isEqualToDataValue:_readCache[attributePath]]) {
// otherwise compare and update read cache
previousValue = _readCache[attributePath];
_readCache[attributePath] = attributeDataValue;
shouldReportAttribute = YES;
}
Expand Down Expand Up @@ -1573,7 +1577,13 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
}

if (shouldReportAttribute) {
[attributesToReport addObject:attributeReponseValue];
if (previousValue) {
NSMutableDictionary * mutableAttributeReponseValue = attributeReponseValue.mutableCopy;
mutableAttributeReponseValue[MTRPreviousDataKey] = previousValue;
[attributesToReport addObject:mutableAttributeReponseValue];
} else {
[attributesToReport addObject:attributeReponseValue];
}
[attributePathsToReport addObject:attributePath];
}
}
Expand All @@ -1585,12 +1595,14 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt

// If value is non-nil, associate with expectedValueID
// If value is nil, remove only if expectedValueID matches
// previousValue is an out parameter
- (void)_setExpectedValue:(NSDictionary<NSString *, id> *)expectedAttributeValue
attributePath:(MTRAttributePath *)attributePath
expirationTime:(NSDate *)expirationTime
shouldReportValue:(BOOL *)shouldReportValue
attributeValueToReport:(NSDictionary<NSString *, id> **)attributeValueToReport
expectedValueID:(uint64_t)expectedValueID
previousValue:(NSDictionary **)previousValue
{
os_unfair_lock_assert_owner(&self->_lock);

Expand All @@ -1604,6 +1616,7 @@ - (void)_setExpectedValue:(NSDictionary<NSString *, id> *)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];
Expand All @@ -1613,6 +1626,7 @@ - (void)_setExpectedValue:(NSDictionary<NSString *, id> *)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;
}
}
Expand All @@ -1623,6 +1637,9 @@ - (void)_setExpectedValue:(NSDictionary<NSString *, id> *)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
Expand All @@ -1649,15 +1666,21 @@ - (NSArray *)_getAttributesToReportWithNewExpectedValues:(NSArray<NSDictionary<N

BOOL shouldReportValue = NO;
NSDictionary<NSString *, id> * attributeValueToReport;
NSDictionary<NSString *, id> * 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];
}
}
Expand Down Expand Up @@ -1721,21 +1744,26 @@ - (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath e

BOOL shouldReportValue;
NSDictionary<NSString *, id> * attributeValueToReport;
NSDictionary<NSString *, id> * 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 ]];
}
}

Expand Down
32 changes: 32 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<NSDictionary<NSString *, id> *> * data) {
for (NSDictionary<NSString *, id> * 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<NSDictionary<NSString *, id> *> * data) {
Expand Down

0 comments on commit c75f454

Please sign in to comment.