Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to intercept attribute report and prune any removed… #33523

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
178cc2d
Add support to intercept attribute report and prune any removed endpo…
nivi-apple May 10, 2024
b32fbee
Restyled by whitespace
restyled-commits May 20, 2024
d08449a
Restyled by clang-format
restyled-commits May 20, 2024
14f5c7d
fixes
nivi-apple May 20, 2024
492e621
Fix the check for the cluster paths
nivi-apple May 20, 2024
30f886c
Apply suggestions from code review
nivi-apple May 22, 2024
b041f16
Address review comments
nivi-apple May 28, 2024
53b38df
Restyled by whitespace
restyled-commits May 28, 2024
adb54cf
Restyled by clang-format
restyled-commits May 28, 2024
85520e6
Added helpers for removing endpoints, clusters and attributes
nivi-apple May 28, 2024
5175ffe
Restyled by clang-format
restyled-commits May 28, 2024
bc38d1d
Clean up the tests
nivi-apple May 28, 2024
1303f37
Remove addition of extra line
nivi-apple May 28, 2024
c6eef37
Restyled by clang-format
restyled-commits May 28, 2024
b37fcfa
Apply suggestions from code review
nivi-apple May 30, 2024
c574483
Addressed review comments
nivi-apple May 30, 2024
a6bf3a0
Restyled by whitespace
restyled-commits May 30, 2024
bbf6bfc
Uncomment subscription pool tests
nivi-apple May 30, 2024
a6f36d3
More clean up
nivi-apple May 30, 2024
700496c
Restyled by clang-format
restyled-commits May 30, 2024
0f76b5f
Add few more strong types for data structures
nivi-apple May 30, 2024
b77cc87
Apply suggestions from code review
nivi-apple May 31, 2024
49b1b11
Restyled by whitespace
restyled-commits May 31, 2024
e568497
Restyled by clang-format
restyled-commits May 31, 2024
678a894
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple Jun 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 148 additions & 3 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,11 @@ - (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(N
_attributes[attribute] = value;
}

- (void)removeValueForAttribute:(NSNumber *)attribute
{
[_attributes removeObjectForKey:attribute];
}

- (NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> *)attributes
{
return _attributes;
Expand Down Expand Up @@ -670,7 +675,7 @@ - (void)_setDSTOffsets:(NSArray<MTRTimeSynchronizationClusterDSTOffsetStruct *>
completion:setDSTOffsetResponseHandler];
}

- (NSMutableArray<NSNumber *> *)arrayOfNumbersFromAttributeValue:(NSDictionary *)dataDictionary
- (NSMutableArray<NSNumber *> *)arrayOfNumbersFromAttributeValue:(MTRDeviceDataValueDictionary)dataDictionary
{
if (![MTRArrayValueType isEqual:dataDictionary[MTRTypeKey]]) {
return nil;
Expand Down Expand Up @@ -1892,6 +1897,17 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f
_clusterDataToPersist[clusterPath] = clusterData;
}

- (void)_removeCachedAttribute:(NSNumber *)attributeID fromCluster:(MTRClusterPath *)clusterPath
{
os_unfair_lock_assert_owner(&self->_lock);

if (_clusterDataToPersist == nil) {
return;
}
auto * clusterData = _clusterDataToPersist[clusterPath];
[clusterData removeValueForAttribute:attributeID];
}

- (void)_createDataVersionFilterListFromDictionary:(NSDictionary<MTRClusterPath *, NSNumber *> *)dataVersions dataVersionFilterList:(DataVersionFilter **)dataVersionFilterList count:(size_t *)count sizeReduction:(size_t)sizeReduction
{
size_t maxDataVersionFilterSize = dataVersions.count;
Expand Down Expand Up @@ -2956,7 +2972,7 @@ - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary
}

// Utility to return data value dictionary without data version
- (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue;
- (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue
{
// Sanity check for nil - return the same input to fail gracefully
if (!attributeValue || !attributeValue[MTRTypeKey]) {
Expand Down Expand Up @@ -3018,6 +3034,116 @@ - (BOOL)_attributeAffectsDeviceConfiguration:(MTRAttributePath *)attributePath
return NO;
}

- (void)_removeClusters:(NSSet<MTRClusterPath *> *)clusterPathsToRemove
doRemoveFromDataStore:(BOOL)doRemoveFromDataStore
{
os_unfair_lock_assert_owner(&self->_lock);

[_persistedClusters minusSet:clusterPathsToRemove];

for (MTRClusterPath * path in clusterPathsToRemove) {
[_persistedClusterData removeObjectForKey:path];
[_clusterDataToPersist removeObjectForKey:path];
if (doRemoveFromDataStore) {
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
}
}
}

- (void)_removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRClusterPath *)clusterPath
{
os_unfair_lock_assert_owner(&self->_lock);

for (NSNumber * attribute in attributes) {
[self _removeCachedAttribute:attribute fromCluster:clusterPath];
}
// Just clear out the NSCache entry for this cluster, so we'll load it from storage as needed.
[_persistedClusterData removeObjectForKey:clusterPath];
[self.deviceController.controllerDataStore removeAttributes:attributes fromCluster:clusterPath forNodeID:self.nodeID];
}

- (void)_pruneEndpointsIn:(MTRDeviceDataValueDictionary)previousPartsListValue
missingFrom:(MTRDeviceDataValueDictionary)newPartsListValue
{
// If the parts list changed and one or more endpoints were removed, remove all the
// clusters for all those endpoints from our data structures.
// Also remove those endpoints from the data store.
NSMutableSet<NSNumber *> * toBeRemovedEndpoints = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousPartsListValue]];
NSSet<NSNumber *> * endpointsOnDevice = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:newPartsListValue]];
[toBeRemovedEndpoints minusSet:endpointsOnDevice];

for (NSNumber * endpoint in toBeRemovedEndpoints) {
NSMutableSet<MTRClusterPath *> * clusterPathsToRemove = [[NSMutableSet alloc] init];
for (MTRClusterPath * path in _persistedClusters) {
if ([path.endpoint isEqualToNumber:endpoint]) {
[clusterPathsToRemove addObject:path];
}
}
[self _removeClusters:clusterPathsToRemove doRemoveFromDataStore:NO];
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:endpoint];
}
}

- (void)_pruneClustersIn:(MTRDeviceDataValueDictionary)previousServerListValue
missingFrom:(MTRDeviceDataValueDictionary)newServerListValue
forEndpoint:(NSNumber *)endpointID
{
// If the server list changed and clusters were removed, remove those clusters from our data structures.
// Also remove them from the data store.
NSMutableSet<NSNumber *> * toBeRemovedClusters = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousServerListValue]];
NSSet<NSNumber *> * clustersStillOnEndpoint = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:newServerListValue]];
[toBeRemovedClusters minusSet:clustersStillOnEndpoint];

NSMutableSet<MTRClusterPath *> * clusterPathsToRemove = [[NSMutableSet alloc] init];
for (MTRClusterPath * path in _persistedClusters) {
if ([path.endpoint isEqualToNumber:endpointID] && [toBeRemovedClusters containsObject:path.cluster])
[clusterPathsToRemove addObject:path];
}
[self _removeClusters:clusterPathsToRemove doRemoveFromDataStore:YES];
}

- (void)_pruneAttributesIn:(MTRDeviceDataValueDictionary)previousAttributeListValue
missingFrom:(MTRDeviceDataValueDictionary)newAttributeListValue
forCluster:(MTRClusterPath *)clusterPath
{
// If the attribute list changed and attributes were removed, remove the attributes from our
// data structures.
NSMutableSet<NSNumber *> * toBeRemovedAttributes = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousAttributeListValue]];
NSSet<NSNumber *> * attributesStillInCluster = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:newAttributeListValue]];

[toBeRemovedAttributes minusSet:attributesStillInCluster];
[self _removeAttributes:toBeRemovedAttributes fromCluster:clusterPath];
}

- (void)_pruneStoredDataForPath:(MTRAttributePath *)attributePath
missingFrom:(MTRDeviceDataValueDictionary)newAttributeDataValue
{
os_unfair_lock_assert_owner(&self->_lock);

if (![self _dataStoreExists] && !_clusterDataToPersist.count) {
MTR_LOG_DEBUG("%@ No data store to prune from", self);
return;
}

// Check if parts list changed or server list changed for the descriptor cluster or the attribute list changed for a cluster.
// If yes, we might need to prune any deleted endpoints, clusters or attributes from the storage and persisted cluster data.
if (attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID) {
if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeClusterDescriptorAttributePartsListID && [attributePath.endpoint isEqualToNumber:@(kRootEndpointId)]) {
[self _pruneEndpointsIn:[self _cachedAttributeValueForPath:attributePath] missingFrom:newAttributeDataValue];
return;
}

if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeClusterDescriptorAttributeServerListID) {
[self _pruneClustersIn:[self _cachedAttributeValueForPath:attributePath] missingFrom:newAttributeDataValue forEndpoint:attributePath.endpoint];
return;
}
}

if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeGlobalAttributeAttributeListID) {
[self _pruneAttributesIn:[self _cachedAttributeValueForPath:attributePath] missingFrom:newAttributeDataValue forCluster:[MTRClusterPath clusterPathWithEndpointID:attributePath.endpoint clusterID:attributePath.cluster]];
}
}

// assume lock is held
- (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSString *, id> *> *)reportedAttributeValues fromSubscription:(BOOL)isFromSubscription
{
Expand Down Expand Up @@ -3071,13 +3197,16 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
[self _noteDataVersion:dataVersion forClusterPath:clusterPath];
}

[self _setCachedAttributeValue:attributeDataValue forPath:attributePath fromSubscription:isFromSubscription];
[self _pruneStoredDataForPath:attributePath missingFrom:attributeDataValue];

if (!_deviceConfigurationChanged) {
_deviceConfigurationChanged = [self _attributeAffectsDeviceConfiguration:attributePath];
if (_deviceConfigurationChanged) {
MTR_LOG("Device configuration changed due to changes in attribute %@", attributePath);
}
}

[self _setCachedAttributeValue:attributeDataValue forPath:attributePath fromSubscription:isFromSubscription];
}

#ifdef DEBUG
Expand Down Expand Up @@ -3230,6 +3359,22 @@ - (void)_storePersistedDeviceData
[datastore storeDeviceData:[data copy] forNodeID:self.nodeID];
}

#ifdef DEBUG
- (MTRDeviceClusterData *)_getClusterDataForPath:(MTRClusterPath *)path
{
std::lock_guard lock(_lock);

return [[self _clusterDataForPath:path] copy];
}

- (BOOL)_clusterHasBeenPersisted:(MTRClusterPath *)path
{
std::lock_guard lock(_lock);

return [_persistedClusters containsObject:path];
}
#endif

- (BOOL)deviceCachePrimed
{
std::lock_guard lock(_lock);
Expand Down
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
- (nullable MTRDeviceClusterData *)getStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
- (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID;
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID;
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID;
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
- (void)removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRClusterPath *)path forNodeID:(NSNumber *)nodeID;
- (void)clearAllStoredClusterData;

/**
Expand Down
87 changes: 87 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,23 @@ - (BOOL)_storeEndpointIndex:(NSArray<NSNumber *> *)endpointIndex forNodeID:(NSNu
return [self _storeAttributeCacheValue:endpointIndex forKey:[self _endpointIndexKeyForNodeID:nodeID]];
}

- (BOOL)_removeEndpointFromEndpointIndex:(NSNumber *)endpointID forNodeID:(NSNumber *)nodeID
{
dispatch_assert_queue(_storageDelegateQueue);
if (!endpointID || !nodeID) {
MTR_LOG_ERROR("%s: unexpected nil input", __func__);
return NO;
}

NSMutableArray<NSNumber *> * endpointIndex = [[self _fetchEndpointIndexForNodeID:nodeID] mutableCopy];
if (endpointIndex == nil) {
return NO;
}

[endpointIndex removeObject:endpointID];
return [self _storeEndpointIndex:endpointIndex forNodeID:nodeID];
}

- (BOOL)_deleteEndpointIndexForNodeID:(NSNumber *)nodeID
{
dispatch_assert_queue(_storageDelegateQueue);
Expand Down Expand Up @@ -699,6 +716,76 @@ - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID
});
}

- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID
{
dispatch_async(_storageDelegateQueue, ^{
NSArray<NSNumber *> * clusterIndex = [self _fetchClusterIndexForNodeID:nodeID endpointID:endpointID];
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
NSMutableArray<NSNumber *> * clusterIndexCopy = [clusterIndex mutableCopy];
[clusterIndexCopy removeObject:clusterID];

BOOL success;
if (clusterIndexCopy.count != clusterIndex.count) {
success = [self _storeClusterIndex:clusterIndexCopy forNodeID:nodeID endpointID:endpointID];
if (!success) {
MTR_LOG_ERROR("clearStoredClusterDataForNodeID: _storeClusterIndex failed for node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
}
}

success = [self _deleteClusterDataForNodeID:nodeID endpointID:endpointID clusterID:clusterID];
if (!success) {
MTR_LOG_ERROR("clearStoredClusterDataForNodeID: _deleteClusterDataForNodeID failed for node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue, clusterID.unsignedLongValue);
return;
}

MTR_LOG("clearStoredClusterDataForNodeID: Deleted endpoint %u cluster 0x%08lX for node 0x%016llX successfully", endpointID.unsignedShortValue, clusterID.unsignedLongValue, nodeID.unsignedLongLongValue);
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
});
}

- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID
{
dispatch_async(_storageDelegateQueue, ^{
BOOL success = [self _removeEndpointFromEndpointIndex:endpointID forNodeID:nodeID];
if (!success) {
MTR_LOG_ERROR("removeEndpointFromEndpointIndex for endpointID %u failed for node 0x%016llX", endpointID.unsignedShortValue, nodeID.unsignedLongLongValue);
}

NSArray<NSNumber *> * clusterIndex = [self _fetchClusterIndexForNodeID:nodeID endpointID:endpointID];

for (NSNumber * cluster in clusterIndex) {
success = [self _deleteClusterDataForNodeID:nodeID endpointID:endpointID clusterID:cluster];
if (!success) {
MTR_LOG_ERROR("Delete failed for clusterData for node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue, cluster.unsignedLongValue);
}
}

success = [self _deleteClusterIndexForNodeID:nodeID endpointID:endpointID];
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
if (!success) {
MTR_LOG_ERROR("Delete failed for clusterIndex for node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
}

MTR_LOG("clearStoredClusterDataForNodeID: Deleted endpoint %u for node 0x%016llX successfully", endpointID.unsignedShortValue, nodeID.unsignedLongLongValue);
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
});
}

- (void)removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRClusterPath *)path forNodeID:(NSNumber *)nodeID
{
MTRDeviceClusterData * clusterData = [self getStoredClusterDataForNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
if (clusterData == nil) {
return;
}
for (NSNumber * attribute in attributes) {
[clusterData removeValueForAttribute:attribute];
}

dispatch_async(_storageDelegateQueue, ^{
BOOL success = [self _storeClusterData:clusterData forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
if (!success) {
MTR_LOG_ERROR("removeAttributes: _storeClusterData failed for node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue);
}
MTR_LOG("removeAttributes: Deleted attributes %@ from endpoint %u cluster 0x%08lX for node 0x%016llX successfully", attributes, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue, nodeID.unsignedLongLongValue);
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
});
}

- (void)clearAllStoredClusterData
{
dispatch_async(_storageDelegateQueue, ^{
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ MTR_TESTABLE
@property (nonatomic, readonly) NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * attributes; // attributeID => data-value dictionary

- (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute;
- (void)removeValueForAttribute:(NSNumber *)attribute;

- (nullable instancetype)initWithDataVersion:(NSNumber * _Nullable)dataVersion attributes:(NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _Nullable)attributes;
@end
Expand Down
Loading
Loading