Skip to content

Commit 4ca9df3

Browse files
nivi-applenivi-apple
committed
Address review comments
1 parent a080ab7 commit 4ca9df3

6 files changed

+293
-778
lines changed

‎src/darwin/Framework/CHIP/MTRDevice.mm

+64-95
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ - (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(N
220220
_attributes[attribute] = value;
221221
}
222222

223-
- (void)_removeValueForAttribute:(NSNumber *)attribute
223+
- (void)removeValueForAttribute:(NSNumber *)attribute
224224
{
225225
[_attributes removeObjectForKey:attribute];
226226
}
@@ -1614,15 +1614,15 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f
16141614
_clusterDataToPersist[clusterPath] = clusterData;
16151615
}
16161616

1617-
- (void)_removeCachedAttributeValue:(MTRClusterPath *)clusterPath forPath:(MTRAttributePath *)attributePath
1617+
- (void)_removeCachedAttribute:(NSNumber *)attributeID fromCluster:(MTRClusterPath *)clusterPath
16181618
{
16191619
os_unfair_lock_assert_owner(&self->_lock);
16201620

16211621
if (_clusterDataToPersist == nil) {
16221622
return;
16231623
}
16241624
auto * clusterData = [_clusterDataToPersist objectForKey:clusterPath];
1625-
[clusterData _removeValueForAttribute:attributePath.attribute];
1625+
[clusterData removeValueForAttribute:attributeID];
16261626
[_clusterDataToPersist setObject:clusterData forKey:clusterPath];
16271627
}
16281628

@@ -2745,42 +2745,55 @@ - (BOOL)_attributeAffectsDeviceConfiguration:(MTRAttributePath *)attributePath
27452745
return NO;
27462746
}
27472747

2748-
- (BOOL)_needsPruningOfEndpointsAndClusters:(MTRAttributePath *)attributePath
2748+
- (void)_removeClusters:(NSSet *)clusterPathsToRemove
27492749
{
2750-
// Check for attributes in the descriptor cluster that could cause removal of endpoints and clusters.
2751-
if (attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID) {
2752-
switch (attributePath.attribute.unsignedLongValue) {
2753-
case MTRAttributeIDTypeClusterDescriptorAttributePartsListID:
2754-
case MTRAttributeIDTypeClusterDescriptorAttributeServerListID:
2755-
return YES;
2756-
}
2750+
os_unfair_lock_assert_owner(&self->_lock);
2751+
2752+
[_persistedClusters minusSet:clusterPathsToRemove];
2753+
2754+
for (MTRClusterPath * path in clusterPathsToRemove)
2755+
{
2756+
[_persistedClusterData removeObjectForKey:path];
2757+
[_persistedClusters removeObject:path];
2758+
[_clusterDataToPersist removeObjectForKey:path];
2759+
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
27572760
}
2761+
}
27582762

2759-
// Check for global attribute - attribute list that could cause removal of attributes.
2760-
switch (attributePath.attribute.unsignedLongValue) {
2761-
case MTRAttributeIDTypeGlobalAttributeAttributeListID:
2762-
return YES;
2763+
- (void)_removeAttributes:(NSSet *)toBeRemovedAttributes fromCluster:(MTRClusterPath *)clusterPathToRemoveAttributesFrom
2764+
{
2765+
if (toBeRemovedAttributes == nil || clusterPathToRemoveAttributesFrom == nil)
2766+
{
2767+
return;
27632768
}
2764-
return NO;
2769+
os_unfair_lock_assert_owner(&self->_lock);
2770+
2771+
for (NSNumber * attribute in toBeRemovedAttributes)
2772+
{
2773+
[self _removeCachedAttribute:attribute fromCluster:clusterPathToRemoveAttributesFrom];
2774+
}
2775+
[_persistedClusterData removeObjectForKey:clusterPathToRemoveAttributesFrom];
2776+
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:clusterPathToRemoveAttributesFrom.endpoint clusterID:clusterPathToRemoveAttributesFrom.cluster];
27652777
}
27662778

27672779
- (void)_pruneOrphanedEndpointsAndClusters:(MTRAttributePath *)attributePath
27682780
previousValue:(NSDictionary *)previousValue
27692781
attributeDataValue:(NSDictionary *)attributeDataValue
27702782
{
27712783
os_unfair_lock_assert_owner(&self->_lock);
2784+
2785+
NSNumber * rootEndpoint = @0;
27722786

27732787
if (_persistedClusters == nil || _persistedClusterData == nil || !previousValue.count) {
27742788
return;
27752789
}
27762790
// Check if parts list changed or server list changed for the descriptor cluster or the attribute list changed for a cluster.
27772791
// If yes, we might need to prune any deleted endpoints, clusters or attributes from the storage and persisted cluster data.
27782792
if (attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID) {
2779-
switch (attributePath.attribute.unsignedLongValue) {
2780-
2781-
// If the parts list changed and one or more endpoints were removed, remove all the clusters in _persistedClusters and _persistedClusterData for all those endpoints.
2782-
// Also remove it from the data store.
2783-
case MTRAttributeIDTypeClusterDescriptorAttributePartsListID: {
2793+
if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeClusterDescriptorAttributePartsListID && [attributePath.endpoint isEqualToNumber:rootEndpoint]) {
2794+
2795+
// If the parts list changed and one or more endpoints were removed, remove all the clusters in _persistedClusters, _persistedClusterData and _clusterDataToPersist for all those endpoints.
2796+
// Also remove it from the data store.
27842797
NSMutableSet * toBeRemovedEndpoints = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousValue]];
27852798
NSSet * endpointsOnDevice = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
27862799
[toBeRemovedEndpoints minusSet:endpointsOnDevice];
@@ -2790,72 +2803,50 @@ - (void)_pruneOrphanedEndpointsAndClusters:(MTRAttributePath *)attributePath
27902803
for (MTRClusterPath * path in _persistedClusters) {
27912804
if ([path.endpoint isEqualToNumber:endpoint]) {
27922805
[clusterPathsToRemove addObject:path];
2793-
[_persistedClusterData removeObjectForKey:path];
2794-
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeIDWithEndpointID:self.nodeID endpointID:endpoint];
27952806
}
27962807
}
2797-
[_persistedClusters minusSet:clusterPathsToRemove];
2808+
[self _removeClusters:[clusterPathsToRemove copy]];
2809+
[self.deviceController.controllerDataStore removeEndpointFromEndpointIndex:endpoint forNodeID:self.nodeID];
27982810
}
2799-
break;
28002811
}
2801-
2802-
// If the server list changed and clusters were removed, remove the clusters from the _persistedClusters and _persistedClusterData for that endpoint
2803-
// Also remove it from the data store.
2804-
case MTRAttributeIDTypeClusterDescriptorAttributeServerListID: {
2805-
NSMutableSet<NSNumber *> * toBeRemovedClusters = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousValue]];
2812+
else if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeClusterDescriptorAttributeServerListID)
2813+
{
2814+
2815+
// If the server list changed and clusters were removed, remove the clusters from the _persistedClusters, _persistedClusterData and _clusterDataToPersist for all those clusters.
2816+
// Also remove it from the data store.
2817+
NSMutableSet<NSNumber *> * toBeRemovedClusters = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:[self _dataValueWithoutDataVersion:previousValue]]];
28062818
NSSet<NSNumber *> * clustersStillOnEndpoint = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
2807-
[toBeRemovedClusters minusSet:clustersOnDevice];
2819+
[toBeRemovedClusters minusSet:clustersStillOnEndpoint];
28082820

28092821
NSMutableSet<MTRClusterPath *> * clusterPathsToRemove = [[NSMutableSet alloc] init];
28102822
for (NSNumber * cluster in toBeRemovedClusters) {
28112823
for (MTRClusterPath * path in _persistedClusters) {
28122824
if ([path.endpoint isEqualToNumber:attributePath.endpoint] && [path.cluster isEqualToNumber:cluster]) {
28132825
[clusterPathsToRemove addObject:path];
2814-
[_persistedClusterData removeObjectForKey:path];
2815-
2816-
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeIDWithClusterID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
28172826
}
28182827
}
28192828
}
2820-
[_persistedClusters minusSet:clusterPathsToRemove];
2821-
break;
2822-
}
2829+
[self _removeClusters:[clusterPathsToRemove copy]];
28232830
}
28242831
}
28252832

2826-
switch (attributePath.attribute.unsignedLongValue) {
2827-
// If the attribute list changed and attributes were removed, remove the attributes from the _persistedClusterData for that cluster and endpoint.
2828-
// Also remove it from the data store cluster data.
2829-
case MTRAttributeIDTypeGlobalAttributeAttributeListID: {
2833+
if (attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeGlobalAttributeAttributeListID) {
2834+
2835+
// If the attribute list changed and attributes were removed, remove the attributes from the _clusterDataToPersist for that cluster and endpoint.
2836+
// Clear out the _peristedClusterData and data store cluster data. Update the data storage with updated cluster data.
28302837
NSMutableSet<NSNumber *> * toBeRemovedAttributes = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:[self _cachedAttributeValueForPath:attributePath]]];
2831-
NSSet<NSNumber *> * attributesStillIncluster = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
2832-
2833-
[toBeRemovedAttributes minusSet:attributesOnDevice];
2834-
for (NSNumber * attribute in toBeRemovedAttributes) {
2835-
for (MTRClusterPath * path in _persistedClusters) {
2836-
if ([path.endpoint isEqualToNumber:attributePath.endpoint] && [path.cluster isEqualToNumber:attributePath.cluster]) {
2837-
MTRDeviceClusterData * clusterData = [self _clusterDataForPath:path];
2838-
if (clusterData == nil) {
2839-
return;
2840-
}
2841-
[clusterData _removeValueForAttribute:attribute];
2842-
[self->_persistedClusterData setObject:clusterData forKey:path];
2843-
2844-
NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> * dataStoreClusterData = [self.deviceController.controllerDataStore getStoredClusterDataForNodeID:self.nodeID];
2845-
NSMutableDictionary<MTRClusterPath *, MTRDeviceClusterData *> * dataStoreClusterDataCopy = [dataStoreClusterData mutableCopy];
2846-
for (MTRClusterPath * dataStorePath in dataStoreClusterData) {
2847-
if ([dataStorePath.endpoint isEqualToNumber:path.endpoint] && [dataStorePath.cluster isEqualToNumber:path.cluster]) {
2848-
[dataStoreClusterDataCopy removeObjectForKey:path];
2849-
[dataStoreClusterDataCopy setObject:clusterData forKey:path];
2850-
[self.deviceController.controllerDataStore storeClusterData:dataStoreClusterDataCopy forNodeID:self.nodeID];
2851-
}
2852-
}
2853-
[self _removeCachedAttributeValue:path forPath:attributePath];
2854-
}
2838+
NSSet<NSNumber *> * attributesStillInCluster = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
2839+
2840+
[toBeRemovedAttributes minusSet:attributesStillInCluster];
2841+
MTRClusterPath * clusterPathToRemoveAttributesFrom;
2842+
for (MTRClusterPath * path in _persistedClusters) {
2843+
if ([path.endpoint isEqualToNumber:attributePath.endpoint] && [path.cluster isEqualToNumber:attributePath.cluster]) {
2844+
clusterPathToRemoveAttributesFrom = path;
2845+
break;
28552846
}
28562847
}
2857-
break;
2858-
}
2848+
[self _removeAttributes:[toBeRemovedAttributes copy] fromCluster:clusterPathToRemoveAttributesFrom];
2849+
[self.deviceController.controllerDataStore removeAttributes:[toBeRemovedAttributes copy] fromCluster:clusterPathToRemoveAttributesFrom forNodeID:self.nodeID];
28592850
}
28602851
}
28612852

@@ -2912,10 +2903,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
29122903
[self _noteDataVersion:dataVersion forClusterPath:clusterPath];
29132904
}
29142905

2915-
if ([self _needsPruningOfEndpointsAndClusters:attributePath]) {
2916-
previousValue = [self _dataValueWithoutDataVersion:previousValue];
2917-
[self _pruneOrphanedEndpointsAndClusters:attributePath previousValue:previousValue attributeDataValue:attributeDataValue];
2918-
}
2906+
[self _pruneOrphanedEndpointsAndClusters:attributePath previousValue:previousValue attributeDataValue:attributeDataValue];
29192907

29202908
if (!_deviceConfigurationChanged) {
29212909
_deviceConfigurationChanged = [self _attributeAffectsDeviceConfiguration:attributePath];
@@ -3078,35 +3066,15 @@ - (void)_storePersistedDeviceData
30783066
[datastore storeDeviceData:[data copy] forNodeID:self.nodeID];
30793067
}
30803068

3081-
- (void)_removePersistedClusterDataForPath:(MTRClusterPath *)path
3082-
{
3083-
os_unfair_lock_assert_owner(&self->_lock);
3084-
if (_persistedClusters == nil || _persistedClusterData == nil) {
3085-
return;
3086-
}
3087-
3088-
[_persistedClusterData removeObjectForKey:path];
3089-
[_persistedClusters removeObject:path];
3090-
}
3091-
3092-
- (NSMutableSet<MTRClusterPath *> *)_getPersistedClusters
3093-
{
3094-
std::lock_guard lock(_lock);
3095-
3096-
return _persistedClusters;
3097-
}
3098-
3099-
- (MTRDeviceClusterData *)_getPersistedClusterDataForPath:(MTRClusterPath *)path
3069+
#ifdef DEBUG
3070+
- (MTRDeviceClusterData *)_getClusterDataForPath:(MTRClusterPath *)path
31003071
{
31013072
std::lock_guard lock(_lock);
31023073

3103-
if ([_persistedClusters containsObject:path]) {
3104-
return [_persistedClusterData objectForKey:path];
3105-
}
3106-
return nil;
3074+
return [[self _clusterDataForPath:path] copy];
31073075
}
31083076

3109-
- (BOOL)_persistedClusterContains:(MTRClusterPath *)path
3077+
- (BOOL)_clusterHasBeenPersisted:(MTRClusterPath *)path
31103078
{
31113079
std::lock_guard lock(_lock);
31123080

@@ -3115,6 +3083,7 @@ - (BOOL)_persistedClusterContains:(MTRClusterPath *)path
31153083
}
31163084
return NO;
31173085
}
3086+
#endif
31183087

31193088
- (BOOL)deviceCachePrimed
31203089
{

‎src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,9 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
7676
- (nullable MTRDeviceClusterData *)getStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
7777
- (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID;
7878
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID;
79-
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID;
8079
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
80+
- (void)removeEndpointFromEndpointIndex:(NSNumber *)endpointID forNodeID:(NSNumber *)nodeID;
81+
- (void)removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRClusterPath *)path forNodeID:(NSNumber *)nodeID;
8182
- (void)clearAllStoredClusterData;
8283

8384
/**

0 commit comments

Comments
 (0)