Darwin: MTRDevice should not persist non-priming attribute values for attributes with Changes Omitted Quality (#33408)
* do not cache attribute values for attributes with Changes Omitted Quality
* Restyled by whitespace
* Restyled by clang-format
* manually address feedback that was previously auto-committed
* Restyled by clang-format
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index 5181b6c..486dcd9 100644
--- a/src/darwin/Framework/CHIP/MTRDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -1163,12 +1163,17 @@
}
}
-- (void)_handleAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport
+- (void)_handleAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport fromSubscription:(BOOL)isFromSubscription
{
std::lock_guard lock(_lock);
// _getAttributesToReportWithReportedValues will log attribute paths reported
- [self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeReport]];
+ [self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeReport fromSubscription:isFromSubscription]];
+}
+
+- (void)_handleAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport
+{
+ [self _handleAttributeReport:attributeReport fromSubscription:NO];
}
#ifdef DEBUG
@@ -1378,11 +1383,11 @@
return clusterData.attributes[path.attribute];
}
-- (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value forPath:(MTRAttributePath *)path
+- (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value forPath:(MTRAttributePath *)path fromSubscription:(BOOL)isFromSubscription
{
os_unfair_lock_assert_owner(&self->_lock);
- // We need an actual MTRClusterPath, not a subsclass, to do _clusterDataForPath.
+ // We need an actual MTRClusterPath, not a subclass, to do _clusterDataForPath.
auto * clusterPath = [MTRClusterPath clusterPathWithEndpointID:path.endpoint clusterID:path.cluster];
MTRDeviceClusterData * clusterData = [self _clusterDataForPath:clusterPath];
@@ -1397,6 +1402,16 @@
[clusterData storeValue:value forAttribute:path.attribute];
+ if (value != nil
+ && isFromSubscription
+ && !_receivingPrimingReport
+ && AttributeHasChangesOmittedQuality(path)) {
+ // Do not persist new values for Changes Omitted Quality attributes unless
+ // they're part of a Priming Report or from a read response.
+ // (removals are OK)
+ return;
+ }
+
if (_clusterDataToPersist == nil) {
_clusterDataToPersist = [NSMutableDictionary dictionary];
}
@@ -1521,7 +1536,7 @@
MTR_LOG_INFO("%@ got attribute report %@", self, value);
dispatch_async(self.queue, ^{
// OnAttributeData
- [self _handleAttributeReport:value];
+ [self _handleAttributeReport:value fromSubscription:YES];
#ifdef DEBUG
self->_unitTestAttributesReportedSinceLastCheck += value.count;
#endif
@@ -2523,7 +2538,7 @@
}
// assume lock is held
-- (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSString *, id> *> *)reportedAttributeValues
+- (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSString *, id> *> *)reportedAttributeValues fromSubscription:(BOOL)isFromSubscription
{
os_unfair_lock_assert_owner(&self->_lock);
@@ -2557,7 +2572,7 @@
_expectedValueCache[attributePath], previousValue);
_expectedValueCache[attributePath] = nil;
// TODO: Is this clearing business really what we want?
- [self _setCachedAttributeValue:nil forPath:attributePath];
+ [self _setCachedAttributeValue:nil forPath:attributePath fromSubscription:isFromSubscription];
} else {
// First separate data version and restore data value to a form without data version
NSNumber * dataVersion = attributeDataValue[MTRDataVersionKey];
@@ -2575,7 +2590,7 @@
[self _noteDataVersion:dataVersion forClusterPath:clusterPath];
}
- [self _setCachedAttributeValue:attributeDataValue forPath:attributePath];
+ [self _setCachedAttributeValue:attributeDataValue forPath:attributePath fromSubscription:isFromSubscription];
if (!_deviceConfigurationChanged) {
_deviceConfigurationChanged = [self _attributeAffectsDeviceConfiguration:attributePath];
if (_deviceConfigurationChanged) {
