Address review comments

jtung-apple · Feb 6, 2024 · 701752f · 701752f
1 parent 90e3324
commit 701752f
Show file tree

Hide file tree

Showing 6 changed files with 221 additions and 175 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -1508,15 +1508,15 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
     if (dataStoreExists) {
         attributesToPersist = [NSMutableArray array];
     }
-    for (NSDictionary<NSString *, id> * attributeReponseValue in reportedAttributeValues) {
-        MTRAttributePath * attributePath = attributeReponseValue[MTRAttributePathKey];
-        NSDictionary * attributeDataValue = attributeReponseValue[MTRDataKey];
-        NSError * attributeError = attributeReponseValue[MTRErrorKey];
+    for (NSDictionary<NSString *, id> * attributeResponseValue in reportedAttributeValues) {
+        MTRAttributePath * attributePath = attributeResponseValue[MTRAttributePathKey];
+        NSDictionary * attributeDataValue = attributeResponseValue[MTRDataKey];
+        NSError * attributeError = attributeResponseValue[MTRErrorKey];
         NSDictionary * previousValue;
 
         // sanity check either data value or error must exist
         if (!attributeDataValue && !attributeError) {
-            MTR_LOG_INFO("%@ report %@ no data value or error: %@", self, attributePath, attributeReponseValue);
+            MTR_LOG_INFO("%@ report %@ no data value or error: %@", self, attributePath, attributeResponseValue);
             continue;
         }
 
@@ -1540,7 +1540,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
             BOOL readCacheValueChanged = ![self _attributeDataValue:attributeDataValue isEqualToDataValue:_readCache[attributePath]];
             // Check if attribute needs to be persisted - compare only to read cache and disregard expected values
             if (dataStoreExists && readCacheValueChanged) {
-                [attributesToPersist addObject:attributeReponseValue];
+                [attributesToPersist addObject:attributeResponseValue];
             }
 
             // if expected values exists, purge and update read cache
@@ -1591,11 +1591,11 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
 
         if (shouldReportAttribute) {
             if (previousValue) {
-                NSMutableDictionary * mutableAttributeReponseValue = attributeReponseValue.mutableCopy;
-                mutableAttributeReponseValue[MTRPreviousDataKey] = previousValue;
-                [attributesToReport addObject:mutableAttributeReponseValue];
+                NSMutableDictionary * mutableAttributeResponseValue = attributeResponseValue.mutableCopy;
+                mutableAttributeResponseValue[MTRPreviousDataKey] = previousValue;
+                [attributesToReport addObject:mutableAttributeResponseValue];
             } else {
-                [attributesToReport addObject:attributeReponseValue];
+                [attributesToReport addObject:attributeResponseValue];
             }
             [attributePathsToReport addObject:attributePath];
         }
@@ -1604,7 +1604,7 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
     MTR_LOG_INFO("%@ report from reported values %@", self, attributePathsToReport);
 
     if (dataStoreExists && attributesToPersist.count) {
-        [_deviceController.controllerDataStore storeInCacheAttributes:attributesToPersist forNodeID:_nodeID];
+        [_deviceController.controllerDataStore storeAttributeValues:attributesToPersist forNodeID:_nodeID];
     }
 
     return attributesToReport;
@@ -1692,9 +1692,9 @@ - (NSArray *)_getAttributesToReportWithNewExpectedValues:(NSArray<NSDictionary<N
 
     NSMutableArray * attributesToReport = [NSMutableArray array];
     NSMutableArray * attributePathsToReport = [NSMutableArray array];
-    for (NSDictionary<NSString *, id> * attributeReponseValue in expectedAttributeValues) {
-        MTRAttributePath * attributePath = attributeReponseValue[MTRAttributePathKey];
-        NSDictionary * attributeDataValue = attributeReponseValue[MTRDataKey];
+    for (NSDictionary<NSString *, id> * attributeResponseValue in expectedAttributeValues) {
+        MTRAttributePath * attributePath = attributeResponseValue[MTRAttributePathKey];
+        NSDictionary * attributeDataValue = attributeResponseValue[MTRDataKey];
 
         BOOL shouldReportValue = NO;
         NSDictionary<NSString *, id> * attributeValueToReport;

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -853,7 +853,7 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
         }
 
         // Load persisted attributes if they exist.
-        NSArray * attributesFromCache = [_controllerDataStore getAttributeCacheForNodeID:nodeID];
+        NSArray * attributesFromCache = [_controllerDataStore getStoredAttributesForNodeID:nodeID];
         if (attributesFromCache) {
             [deviceToReturn setAttributeValues:attributesFromCache reportChanges:NO];
         }
@@ -1238,21 +1238,21 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
 
 #pragma - Unit Test accessors for data store
 #ifdef DEBUG
-- (nullable NSArray<NSDictionary *> *)unitTest_dataStore_getAttributeCacheForNodeID:(NSNumber *)nodeID
+- (nullable NSArray<NSDictionary *> *)unitTest_dataStore_getStoredAttributesForNodeID:(NSNumber *)nodeID
 {
-    return [_controllerDataStore getAttributeCacheForNodeID:nodeID];
+    return [_controllerDataStore getStoredAttributesForNodeID:nodeID];
 }
-- (void)unitTest_dataStore_storeInCacheAttributes:(NSArray<NSDictionary *> *)dataValues forNodeID:(NSNumber *)nodeID
+- (void)unitTest_dataStore_storeAttributeValues:(NSArray<NSDictionary *> *)dataValues forNodeID:(NSNumber *)nodeID
 {
-    [_controllerDataStore storeInCacheAttributes:dataValues forNodeID:nodeID];
+    [_controllerDataStore storeAttributeValues:dataValues forNodeID:nodeID];
 }
-- (void)unitTest_dataStore_clearAttributeCacheForNodeID:(NSNumber *)nodeID
+- (void)unitTest_dataStore_clearStoredAttributesForNodeID:(NSNumber *)nodeID
 {
-    [_controllerDataStore clearAttributeCacheForNodeID:nodeID];
+    [_controllerDataStore clearStoredAttributesForNodeID:nodeID];
 }
-- (void)unitTest_dataStore_clearAllAttributeCache
+- (void)unitTest_dataStore_clearAllStoredAttributes
 {
-    [_controllerDataStore clearAllAttributeCache];
+    [_controllerDataStore clearAllStoredAttributes];
 }
 #endif
 

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
@@ -63,49 +63,13 @@ NS_ASSUME_NONNULL_BEGIN
 - (CHIP_ERROR)storeLastLocallyUsedNOC:(MTRCertificateTLVBytes)noc;
 - (MTRCertificateTLVBytes _Nullable)fetchLastLocallyUsedNOC;
 
-/** MTRDevice cache storage
- *
- *  Per controller:
- *    NodeID index
- *        key: "nodeID index"
- *        value: list of nodeIDs
- *    EndpointID index
- *        key: "<nodeID> endpoints"
- *        value: list of endpoint IDs
- *    ClusterID index
- *        key: "<nodeID+endpointID> clusters"
- *        value: list of cluster IDs
- *    AttributeID index
- *        key: "<nodeID+endpointID+clusterID> attributes"
- *        value: list of attribute IDs
- *    Attribute data:
- *        key: "<nodeID+endpointID+clusterID+attributeID> attribute data"
- *        value: serialized dictionary of attribute data
- *
- *    Attribute data dictionary
- *        Additional value "serial number"
- */
-- (nullable NSArray<NSDictionary *> *)getAttributeCacheForNodeID:(NSNumber *)nodeID;
-- (void)storeInCacheAttributes:(NSArray<NSDictionary *> *)dataValues forNodeID:(NSNumber *)nodeID;
-- (void)clearAttributeCacheForNodeID:(NSNumber *)nodeID;
-- (void)clearAllAttributeCache;
-
 /**
- * MTRDevice cache
- *     getAttributesToReportWithReportedValues
- *         Take updaes => send non-error paths and call store
- *     init
- *         Call getAttributeCacheForNodeID
- *
- * MTRDevieControllerDataStore
- *     init
- *         Read serial number and advance-jump / store
- *
- * MTRDevieControllerDataStore
- *     Need lock for store / get transactions to work without stomping each other
- *
- * TODO: Need to think about clean up for stale nodes
+ * Storage for MTRDevice attribute read cache. This is local-only storage as an optimization. New controller devices using MTRDevice API can prime their own local cache from devices directly.
  */
+- (nullable NSArray<NSDictionary *> *)getStoredAttributesForNodeID:(NSNumber *)nodeID;
+- (void)storeAttributeValues:(NSArray<NSDictionary *> *)dataValues forNodeID:(NSNumber *)nodeID;
+- (void)clearStoredAttributesForNodeID:(NSNumber *)nodeID;
+- (void)clearAllStoredAttributes;
 
 @end