Fix behavior of MTRDevice readAttributePaths if it happens mid-primin…

…g. (#35656) * Fix behavior of MTRDevice readAttributePaths if it happens mid-priming. We might not know the values of AttributeList/ServerList/PartsList yet at that point. Just go ahead and figure out which attribute values we have by walking our own data structures, not the metadata list attribute values. * Apply suggestion from code review. Co-authored-by: Nivi Sarkar <[email protected]> --------- Co-authored-by: Nivi Sarkar <[email protected]>
project-chip · Nov 1, 2024 · 7372194 · 7372194
1 parent bb324ca
commit 7372194
Showing 1 changed file with 25 additions and 47 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
@@ -2983,11 +2983,31 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
     // Determine the set of what the spec calls "existent paths" that correspond
     // to the request paths.  Building the whole set in-memory is OK, because
     // we're going to need all those paths for our return value anyway.
+    //
+    // Note that we don't use the structural attributes (PartsList, ServerList,
+    // AttributeList) to determine this set, because we might be in the middle
+    // of priming right now and have not gotten those yet.  Just use the set of
+    // attribute paths we actually have.
     NSMutableSet<MTRAttributePath *> * existentPaths = [[NSMutableSet alloc] init];
     {
         std::lock_guard lock(_lock);
-        for (MTRAttributeRequestPath * path in attributePaths) {
-            [self _addExistentPathsFor:path to:existentPaths];
+        for (MTRAttributeRequestPath * requestPath in attributePaths) {
+            for (MTRClusterPath * clusterPath in [self _knownClusters]) {
+                if (requestPath.endpoint != nil && ![requestPath.endpoint isEqual:clusterPath.endpoint]) {
+                    continue;
+                }
+                if (requestPath.cluster != nil && ![requestPath.cluster isEqual:clusterPath.cluster]) {
+                    continue;
+                }
+                MTRDeviceClusterData * clusterData = [self _clusterDataForPath:clusterPath];
+                if (requestPath.attribute == nil) {
+                    for (NSNumber * attributeID in clusterData.attributes) {
+                        [existentPaths addObject:[MTRAttributePath attributePathWithEndpointID:clusterPath.endpoint clusterID:clusterPath.cluster attributeID:attributeID]];
+                    }
+                } else if ([clusterData.attributes objectForKey:requestPath.attribute] != nil) {
+                    [existentPaths addObject:[MTRAttributePath attributePathWithEndpointID:clusterPath.endpoint clusterID:clusterPath.cluster attributeID:requestPath.attribute]];
+                }
+            }
         }
     }
 
@@ -3006,51 +3026,6 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
     return result;
 }
 
-- (void)_addExistentPathsFor:(MTRAttributeRequestPath *)path to:(NSMutableSet<MTRAttributePath *> *)set
-{
-    os_unfair_lock_assert_owner(&_lock);
-
-    if (path.endpoint != nil) {
-        [self _addExistentPathsForEndpoint:path.endpoint path:path to:set];
-        return;
-    }
-
-    NSArray<NSNumber *> * endpointList = [self _endpointList];
-    for (NSNumber * endpoint in endpointList) {
-        [self _addExistentPathsForEndpoint:endpoint path:path to:set];
-    }
-}
-
-- (void)_addExistentPathsForEndpoint:(NSNumber *)endpoint path:(MTRAttributeRequestPath *)path to:(NSMutableSet<MTRAttributePath *> *)set
-{
-    os_unfair_lock_assert_owner(&_lock);
-
-    if (path.cluster != nil) {
-        [self _addExistentPathsForEndpoint:endpoint cluster:path.cluster attribute:path.attribute to:set];
-        return;
-    }
-
-    auto * clusterList = [self _serverListForEndpointID:endpoint];
-    for (NSNumber * cluster in clusterList) {
-        [self _addExistentPathsForEndpoint:endpoint cluster:cluster attribute:path.attribute to:set];
-    }
-}
-
-- (void)_addExistentPathsForEndpoint:(NSNumber *)endpoint cluster:(NSNumber *)cluster attribute:(NSNumber * _Nullable)attribute to:(NSMutableSet<MTRAttributePath *> *)set
-{
-    os_unfair_lock_assert_owner(&_lock);
-
-    if (attribute != nil) {
-        [set addObject:[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:cluster attributeID:attribute]];
-        return;
-    }
-
-    auto * attributeList = [self _attributeListForEndpointID:endpoint clusterID:cluster];
-    for (NSNumber * existentAttribute in attributeList) {
-        [set addObject:[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:cluster attributeID:existentAttribute]];
-    }
-}
-
 - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID
                            clusterID:(NSNumber *)clusterID
                            commandID:(NSNumber *)commandID
@@ -3652,6 +3627,9 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
     return attributesToReport;
 }
 
+// TODO: Figure out whether we can get rid of this in favor of readAttributePaths.  This differs from
+// readAttributePaths in one respect: that function will do read-through for
+// C-quality attributes, but this one does not.
 - (NSArray<NSDictionary<NSString *, id> *> *)getAllAttributesReport
 {
     std::lock_guard lock(_lock);