Skip to content

Commit 58b43e0

Browse files
Fix behavior of MTRDevice readAttributePaths if it happens mid-priming. (#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 <55898241+nivi-apple@users.noreply.github.com> --------- Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
1 parent dff8864 commit 58b43e0

File tree

1 file changed

+25
-47
lines changed

1 file changed

+25
-47
lines changed

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

+25-47
Original file line numberDiff line numberDiff line change
@@ -2983,11 +2983,31 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
29832983
// Determine the set of what the spec calls "existent paths" that correspond
29842984
// to the request paths. Building the whole set in-memory is OK, because
29852985
// we're going to need all those paths for our return value anyway.
2986+
//
2987+
// Note that we don't use the structural attributes (PartsList, ServerList,
2988+
// AttributeList) to determine this set, because we might be in the middle
2989+
// of priming right now and have not gotten those yet. Just use the set of
2990+
// attribute paths we actually have.
29862991
NSMutableSet<MTRAttributePath *> * existentPaths = [[NSMutableSet alloc] init];
29872992
{
29882993
std::lock_guard lock(_lock);
2989-
for (MTRAttributeRequestPath * path in attributePaths) {
2990-
[self _addExistentPathsFor:path to:existentPaths];
2994+
for (MTRAttributeRequestPath * requestPath in attributePaths) {
2995+
for (MTRClusterPath * clusterPath in [self _knownClusters]) {
2996+
if (requestPath.endpoint != nil && ![requestPath.endpoint isEqual:clusterPath.endpoint]) {
2997+
continue;
2998+
}
2999+
if (requestPath.cluster != nil && ![requestPath.cluster isEqual:clusterPath.cluster]) {
3000+
continue;
3001+
}
3002+
MTRDeviceClusterData * clusterData = [self _clusterDataForPath:clusterPath];
3003+
if (requestPath.attribute == nil) {
3004+
for (NSNumber * attributeID in clusterData.attributes) {
3005+
[existentPaths addObject:[MTRAttributePath attributePathWithEndpointID:clusterPath.endpoint clusterID:clusterPath.cluster attributeID:attributeID]];
3006+
}
3007+
} else if ([clusterData.attributes objectForKey:requestPath.attribute] != nil) {
3008+
[existentPaths addObject:[MTRAttributePath attributePathWithEndpointID:clusterPath.endpoint clusterID:clusterPath.cluster attributeID:requestPath.attribute]];
3009+
}
3010+
}
29913011
}
29923012
}
29933013

@@ -3006,51 +3026,6 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
30063026
return result;
30073027
}
30083028

3009-
- (void)_addExistentPathsFor:(MTRAttributeRequestPath *)path to:(NSMutableSet<MTRAttributePath *> *)set
3010-
{
3011-
os_unfair_lock_assert_owner(&_lock);
3012-
3013-
if (path.endpoint != nil) {
3014-
[self _addExistentPathsForEndpoint:path.endpoint path:path to:set];
3015-
return;
3016-
}
3017-
3018-
NSArray<NSNumber *> * endpointList = [self _endpointList];
3019-
for (NSNumber * endpoint in endpointList) {
3020-
[self _addExistentPathsForEndpoint:endpoint path:path to:set];
3021-
}
3022-
}
3023-
3024-
- (void)_addExistentPathsForEndpoint:(NSNumber *)endpoint path:(MTRAttributeRequestPath *)path to:(NSMutableSet<MTRAttributePath *> *)set
3025-
{
3026-
os_unfair_lock_assert_owner(&_lock);
3027-
3028-
if (path.cluster != nil) {
3029-
[self _addExistentPathsForEndpoint:endpoint cluster:path.cluster attribute:path.attribute to:set];
3030-
return;
3031-
}
3032-
3033-
auto * clusterList = [self _serverListForEndpointID:endpoint];
3034-
for (NSNumber * cluster in clusterList) {
3035-
[self _addExistentPathsForEndpoint:endpoint cluster:cluster attribute:path.attribute to:set];
3036-
}
3037-
}
3038-
3039-
- (void)_addExistentPathsForEndpoint:(NSNumber *)endpoint cluster:(NSNumber *)cluster attribute:(NSNumber * _Nullable)attribute to:(NSMutableSet<MTRAttributePath *> *)set
3040-
{
3041-
os_unfair_lock_assert_owner(&_lock);
3042-
3043-
if (attribute != nil) {
3044-
[set addObject:[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:cluster attributeID:attribute]];
3045-
return;
3046-
}
3047-
3048-
auto * attributeList = [self _attributeListForEndpointID:endpoint clusterID:cluster];
3049-
for (NSNumber * existentAttribute in attributeList) {
3050-
[set addObject:[MTRAttributePath attributePathWithEndpointID:endpoint clusterID:cluster attributeID:existentAttribute]];
3051-
}
3052-
}
3053-
30543029
- (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID
30553030
clusterID:(NSNumber *)clusterID
30563031
commandID:(NSNumber *)commandID
@@ -3652,6 +3627,9 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
36523627
return attributesToReport;
36533628
}
36543629

3630+
// TODO: Figure out whether we can get rid of this in favor of readAttributePaths. This differs from
3631+
// readAttributePaths in one respect: that function will do read-through for
3632+
// C-quality attributes, but this one does not.
36553633
- (NSArray<NSDictionary<NSString *, id> *> *)getAllAttributesReport
36563634
{
36573635
std::lock_guard lock(_lock);

0 commit comments

Comments
 (0)