From 5bb92b6ff2dbf719f416eb044cd2816913744156 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Thu, 15 Feb 2024 21:02:17 -0800 Subject: [PATCH] [Darwin] MTRDevice to persist data versions and use for subscription (#32153) * [Darwin] MTRDevice to persist data versions and use for subscription * Fixed XPC connection bits --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 68 +++++-- .../Framework/CHIP/MTRBaseDevice_Internal.h | 12 +- .../CHIP/MTRBaseSubscriptionCallback.mm | 4 +- src/darwin/Framework/CHIP/MTRDevice.h | 5 + src/darwin/Framework/CHIP/MTRDevice.mm | 186 ++++++++++++++---- src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm | 10 + .../CHIPTests/MTRPerControllerStorageTests.m | 38 +++- 7 files changed, 251 insertions(+), 72 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 0d41d925d49507..00f65b25dc000b 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -475,8 +475,21 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue }]; } +static NSDictionary * _MakeDataValueDictionary(NSString * type, id _Nullable value, NSNumber * _Nullable dataVersion) +{ + if (value && dataVersion) { + return @ { MTRTypeKey : type, MTRValueKey : value, MTRDataVersionKey : dataVersion }; + } else if (value) { + return @ { MTRTypeKey : type, MTRValueKey : value }; + } else if (dataVersion) { + return @ { MTRTypeKey : type, MTRDataVersionKey : dataVersion }; + } else { + return @ { MTRTypeKey : type }; + } +} + // Convert TLV data into data-value dictionary as described in MTRDeviceResponseHandler -NSDictionary * _Nullable MTRDecodeDataValueDictionaryFromCHIPTLV(chip::TLV::TLVReader * data) +NSDictionary * _Nullable MTRDecodeDataValueDictionaryFromCHIPTLV(chip::TLV::TLVReader * data, NSNumber * dataVersion) { chip::TLV::TLVType dataTLVType = data->GetType(); switch (dataTLVType) { @@ -487,8 +500,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV signed integer decoding failed", chip::ErrorStr(err)); return nil; } - return [NSDictionary dictionaryWithObjectsAndKeys:MTRSignedIntegerValueType, MTRTypeKey, [NSNumber numberWithLongLong:val], - MTRValueKey, nil]; + return _MakeDataValueDictionary(MTRSignedIntegerValueType, @(val), dataVersion); } case chip::TLV::kTLVType_UnsignedInteger: { uint64_t val; @@ -497,8 +509,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV unsigned integer decoding failed", chip::ErrorStr(err)); return nil; } - return [NSDictionary dictionaryWithObjectsAndKeys:MTRUnsignedIntegerValueType, MTRTypeKey, - [NSNumber numberWithUnsignedLongLong:val], MTRValueKey, nil]; + return _MakeDataValueDictionary(MTRUnsignedIntegerValueType, @(val), dataVersion); } case chip::TLV::kTLVType_Boolean: { bool val; @@ -507,15 +518,14 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV boolean decoding failed", chip::ErrorStr(err)); return nil; } - return [NSDictionary - dictionaryWithObjectsAndKeys:MTRBooleanValueType, MTRTypeKey, [NSNumber numberWithBool:val], MTRValueKey, nil]; + return _MakeDataValueDictionary(MTRBooleanValueType, @(val), dataVersion); } case chip::TLV::kTLVType_FloatingPointNumber: { // Try float first float floatValue; CHIP_ERROR err = data->Get(floatValue); if (err == CHIP_NO_ERROR) { - return @ { MTRTypeKey : MTRFloatValueType, MTRValueKey : [NSNumber numberWithFloat:floatValue] }; + return _MakeDataValueDictionary(MTRFloatValueType, @(floatValue), dataVersion); } double val; err = data->Get(val); @@ -523,8 +533,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV floating point decoding failed", chip::ErrorStr(err)); return nil; } - return [NSDictionary - dictionaryWithObjectsAndKeys:MTRDoubleValueType, MTRTypeKey, [NSNumber numberWithDouble:val], MTRValueKey, nil]; + return _MakeDataValueDictionary(MTRDoubleValueType, @(val), dataVersion); } case chip::TLV::kTLVType_UTF8String: { CharSpan stringValue; @@ -538,7 +547,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV UTF8String value is not actually UTF-8", err.AsString()); return nil; } - return @ { MTRTypeKey : MTRUTF8StringValueType, MTRValueKey : stringObj }; + return _MakeDataValueDictionary(MTRUTF8StringValueType, stringObj, dataVersion); } case chip::TLV::kTLVType_ByteString: { ByteSpan bytesValue; @@ -547,10 +556,10 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV ByteString decoding failed", chip::ErrorStr(err)); return nil; } - return @ { MTRTypeKey : MTROctetStringValueType, MTRValueKey : AsData(bytesValue) }; + return _MakeDataValueDictionary(MTROctetStringValueType, AsData(bytesValue), dataVersion); } case chip::TLV::kTLVType_Null: { - return [NSDictionary dictionaryWithObjectsAndKeys:MTRNullValueType, MTRTypeKey, nil]; + return _MakeDataValueDictionary(MTRNullValueType, nil, dataVersion); } case chip::TLV::kTLVType_Structure: case chip::TLV::kTLVType_Array: { @@ -606,7 +615,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue MTR_LOG_ERROR("Error(%s): TLV container exiting failed", chip::ErrorStr(err)); return nil; } - return [NSDictionary dictionaryWithObjectsAndKeys:typeName, MTRTypeKey, array, MTRValueKey, nil]; + return _MakeDataValueDictionary(typeName, array, dataVersion); } default: MTR_LOG_ERROR("Error: Unsupported TLV type for conversion: %u", (unsigned) data->GetType()); @@ -815,7 +824,7 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const class BufferedReadClientCallback final : public app::ReadClient::Callback { public: using OnSuccessAttributeCallbackType - = std::function; + = std::function; using OnSuccessEventCallbackType = std::function; using OnErrorCallbackType = std::function; @@ -1027,6 +1036,16 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri params:(MTRReadParams * _Nullable)params queue:(dispatch_queue_t)queue completion:(MTRDeviceResponseHandler)completion +{ + [self readAttributePaths:attributePaths eventPaths:eventPaths params:params includeDataVersion:NO queue:queue completion:completion]; +} + +- (void)readAttributePaths:(NSArray * _Nullable)attributePaths + eventPaths:(NSArray * _Nullable)eventPaths + params:(MTRReadParams * _Nullable)params + includeDataVersion:(BOOL)includeDataVersion + queue:(dispatch_queue_t)queue + completion:(MTRDeviceResponseHandler)completion { if ((attributePaths == nil || [attributePaths count] == 0) && (eventPaths == nil || [eventPaths count] == 0)) { // No paths, just return an empty array. @@ -1056,11 +1075,20 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri auto resultArray = [[NSMutableArray alloc] init]; auto onAttributeSuccessCb - = [resultArray](const ConcreteAttributePath & aAttributePath, const MTRDataValueDictionaryDecodableType & aData) { - [resultArray addObject:@ { - MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:aAttributePath], - MTRDataKey : aData.GetDecodedObject() - }]; + = [resultArray, includeDataVersion](const ConcreteDataAttributePath & aAttributePath, const MTRDataValueDictionaryDecodableType & aData) { + // TODO: move this logic into MTRDataValueDictionaryDecodableType + if (includeDataVersion && aAttributePath.mDataVersion.HasValue()) { + NSDictionary * dataValue = aData.GetDecodedObject(); + [resultArray addObject:@{ + MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:aAttributePath], + MTRDataKey : _MakeDataValueDictionary(dataValue[MTRTypeKey], dataValue[MTRValueKey], @(aAttributePath.mDataVersion.Value())) + }]; + } else { + [resultArray addObject:@ { + MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:aAttributePath], + MTRDataKey : aData.GetDecodedObject() + }]; + } }; auto onEventSuccessCb diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h index 71cdee11d3687d..4ebc454925a4a4 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h +++ b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h @@ -185,6 +185,16 @@ static inline MTRTransportType MTRMakeTransportType(chip::Transport::Type type) queue:(dispatch_queue_t)queue completion:(void (^)(id _Nullable value, NSError * _Nullable error))completion; +/** + * Same as the public -readAttributePaths:eventPaths:params:queue:completion: except also include the data version in the data-value dictionary in the response dictionary, if the includeDataVersion argument is set to YES. + */ +- (void)readAttributePaths:(NSArray * _Nullable)attributePaths + eventPaths:(NSArray * _Nullable)eventPaths + params:(MTRReadParams * _Nullable)params + includeDataVersion:(BOOL)includeDataVersion + queue:(dispatch_queue_t)queue + completion:(MTRDeviceResponseHandler)completion; + @end @interface MTRClusterPath () @@ -228,7 +238,7 @@ static inline MTRTransportType MTRMakeTransportType(chip::Transport::Type type) // Exported utility function // Convert TLV data into data-value dictionary as described in MTRDeviceResponseHandler -NSDictionary * _Nullable MTRDecodeDataValueDictionaryFromCHIPTLV(chip::TLV::TLVReader * data); +NSDictionary * _Nullable MTRDecodeDataValueDictionaryFromCHIPTLV(chip::TLV::TLVReader * data, NSNumber * _Nullable dataVersion = nil); // Convert a data-value dictionary as described in MTRDeviceResponseHandler into // TLV Data with an anonymous tag. This method assumes the encoding of the diff --git a/src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm b/src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm index 079cefce5c9fc9..684333bad5f47a 100644 --- a/src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm +++ b/src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm @@ -111,9 +111,9 @@ } VerifyOrDie((aReadPrepareParams.mDataVersionFilterListSize == 0 && aReadPrepareParams.mpDataVersionFilterList == nullptr) - || (aReadPrepareParams.mDataVersionFilterListSize == 1 && aReadPrepareParams.mpDataVersionFilterList != nullptr)); + || (aReadPrepareParams.mDataVersionFilterListSize > 0 && aReadPrepareParams.mpDataVersionFilterList != nullptr)); if (aReadPrepareParams.mpDataVersionFilterList != nullptr) { - delete aReadPrepareParams.mpDataVersionFilterList; + delete[] aReadPrepareParams.mpDataVersionFilterList; } VerifyOrDie((aReadPrepareParams.mEventPathParamsListSize == 0 && aReadPrepareParams.mpEventPathParamsList == nullptr) diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 96526a2ecacf67..d4641d5dd321b3 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.h +++ b/src/darwin/Framework/CHIP/MTRDevice.h @@ -350,6 +350,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) @end MTR_EXTERN NSString * const MTRPreviousDataKey MTR_NEWLY_AVAILABLE; +MTR_EXTERN NSString * const MTRDataVersionKey MTR_NEWLY_AVAILABLE; @protocol MTRDeviceDelegate @required @@ -366,6 +367,10 @@ MTR_EXTERN NSString * const MTRPreviousDataKey MTR_NEWLY_AVAILABLE; * In addition to MTRDataKey, each response-value dictionary in the array may also have this key: * * MTRPreviousDataKey : Same data-value dictionary format as the object for MTRDataKey. This is included when the previous value is known for an attribute. + * + * The data-value dictionary also contains this key: + * + * MTRDataVersionKey : NSNumber-wrapped uin32_t. Monotonically increaseing data version for the cluster. */ - (void)device:(MTRDevice *)device receivedAttributeReport:(NSArray *> *)attributeReport; diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index f2ba2d2173e977..2307a25d22361f 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -46,6 +46,7 @@ typedef void (^MTRDeviceAttributeReportHandler)(NSArray * _Nonnull); NSString * const MTRPreviousDataKey = @"previousData"; +NSString * const MTRDataVersionKey = @"dataVersion"; // Consider moving utility classes to their own file #pragma mark - Utility Classes @@ -213,7 +214,11 @@ - (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device; @end #endif -@implementation MTRDevice +@implementation MTRDevice { +#ifdef DEBUG + NSUInteger _unitTestAttributesReportedSinceLastCheck; +#endif +} - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller { @@ -641,6 +646,55 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor os_unfair_lock_unlock(&self->_lock); } +- (NSDictionary *)_getCachedDataVersions +{ + NSMutableDictionary * dataVersions = [NSMutableDictionary dictionary]; + os_unfair_lock_lock(&self->_lock); + for (MTRAttributePath * path in _readCache) { + NSDictionary * dataValue = _readCache[path]; + NSNumber * dataVersionNumber = dataValue[MTRDataVersionKey]; + if (dataVersionNumber) { + MTRClusterPath * clusterPath = [MTRClusterPath clusterPathWithEndpointID:path.endpoint clusterID:path.cluster]; + NSNumber * currentDataVersion = dataVersions[clusterPath]; + // Use the highest data version + if (currentDataVersion.unsignedLongValue < dataVersionNumber.unsignedLongValue) { + dataVersions[clusterPath] = dataVersionNumber; + } + } + } + os_unfair_lock_unlock(&self->_lock); + + return dataVersions; +} + +- (void)_createDataVersionFilterListFromDictionary:(NSDictionary *)dataVersions dataVersionFilterList:(DataVersionFilter **)dataVersionFilterList count:(size_t *)count sizeReduction:(size_t)sizeReduction +{ + size_t maxDataVersionFilterSize = dataVersions.count; + + // Check if any filter list should be generated + if (!dataVersions.count || (maxDataVersionFilterSize <= sizeReduction)) { + *count = 0; + *dataVersionFilterList = nullptr; + return; + } + maxDataVersionFilterSize -= sizeReduction; + + DataVersionFilter * dataVersionFilterArray = new DataVersionFilter[maxDataVersionFilterSize]; + size_t i = 0; + for (MTRClusterPath * path in dataVersions) { + NSNumber * dataVersionNumber = dataVersions[path]; + if (dataVersionNumber) { + dataVersionFilterArray[i++] = DataVersionFilter(static_cast(path.endpoint.unsignedShortValue), static_cast(path.cluster.unsignedLongValue), static_cast(dataVersionNumber.unsignedLongValue)); + } + if (i == maxDataVersionFilterSize) { + break; + } + } + + *dataVersionFilterList = dataVersionFilterArray; + *count = maxDataVersionFilterSize; +} + // assume lock is held - (void)_setupSubscription { @@ -677,55 +731,21 @@ - (void)_setupSubscription return; } - // Wildcard endpoint, cluster, attribute, event. - auto attributePath = std::make_unique(); - auto eventPath = std::make_unique(); - // We want to get event reports at the minInterval, not the maxInterval. - eventPath->mIsUrgentEvent = true; - ReadPrepareParams readParams(session.Value()); - - readParams.mMinIntervalFloorSeconds = 0; - // Select a max interval based on the device's claimed idle sleep interval. - auto idleSleepInterval = std::chrono::duration_cast( - session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout); - - auto maxIntervalCeilingMin = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN); - if (idleSleepInterval < maxIntervalCeilingMin) { - idleSleepInterval = maxIntervalCeilingMin; - } - - auto maxIntervalCeilingMax = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX); - if (idleSleepInterval > maxIntervalCeilingMax) { - idleSleepInterval = maxIntervalCeilingMax; - } -#ifdef DEBUG - if (maxIntervalOverride.HasValue()) { - idleSleepInterval = maxIntervalOverride.Value(); - } -#endif - readParams.mMaxIntervalCeilingSeconds = static_cast(idleSleepInterval.count()); - - readParams.mpAttributePathParamsList = attributePath.get(); - readParams.mAttributePathParamsListSize = 1; - readParams.mpEventPathParamsList = eventPath.get(); - readParams.mEventPathParamsListSize = 1; - readParams.mKeepSubscriptions = true; - readParams.mIsFabricFiltered = false; - attributePath.release(); - eventPath.release(); - auto callback = std::make_unique( ^(NSArray * value) { MTR_LOG_INFO("%@ got attribute report %@", self, value); dispatch_async(self.queue, ^{ - // OnAttributeData (after OnReportEnd) + // OnAttributeData [self _handleAttributeReport:value]; +#ifdef DEBUG + self->_unitTestAttributesReportedSinceLastCheck += value.count; +#endif }); }, ^(NSArray * value) { MTR_LOG_INFO("%@ got event report %@", self, value); dispatch_async(self.queue, ^{ - // OnEventReport (after OnReportEnd) + // OnEventReport [self _handleEventReport:value]; }); }, @@ -793,19 +813,88 @@ - (void)_setupSubscription auto readClient = std::make_unique(InteractionModelEngine::GetInstance(), exchangeManager, clusterStateCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe); - // SendAutoResubscribeRequest cleans up the params, even on failure. - CHIP_ERROR err = readClient->SendAutoResubscribeRequest(std::move(readParams)); + // Subscribe with data version filter list and retry with smaller list if out of packet space + CHIP_ERROR err; + NSDictionary * dataVersions = [self _getCachedDataVersions]; + size_t dataVersionFilterListSizeReduction = 0; + for (;;) { + // Wildcard endpoint, cluster, attribute, event. + auto attributePath = std::make_unique(); + auto eventPath = std::make_unique(); + // We want to get event reports at the minInterval, not the maxInterval. + eventPath->mIsUrgentEvent = true; + ReadPrepareParams readParams(session.Value()); + + readParams.mMinIntervalFloorSeconds = 0; + // Select a max interval based on the device's claimed idle sleep interval. + auto idleSleepInterval = std::chrono::duration_cast( + session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout); + + auto maxIntervalCeilingMin = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN); + if (idleSleepInterval < maxIntervalCeilingMin) { + idleSleepInterval = maxIntervalCeilingMin; + } + + auto maxIntervalCeilingMax = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX); + if (idleSleepInterval > maxIntervalCeilingMax) { + idleSleepInterval = maxIntervalCeilingMax; + } +#ifdef DEBUG + if (maxIntervalOverride.HasValue()) { + idleSleepInterval = maxIntervalOverride.Value(); + } +#endif + readParams.mMaxIntervalCeilingSeconds = static_cast(idleSleepInterval.count()); + + readParams.mpAttributePathParamsList = attributePath.get(); + readParams.mAttributePathParamsListSize = 1; + readParams.mpEventPathParamsList = eventPath.get(); + readParams.mEventPathParamsListSize = 1; + readParams.mKeepSubscriptions = true; + readParams.mIsFabricFiltered = false; + size_t dataVersionFilterListSize = 0; + DataVersionFilter * dataVersionFilterList; + [self _createDataVersionFilterListFromDictionary:dataVersions dataVersionFilterList:&dataVersionFilterList count:&dataVersionFilterListSize sizeReduction:dataVersionFilterListSizeReduction]; + readParams.mDataVersionFilterListSize = dataVersionFilterListSize; + readParams.mpDataVersionFilterList = dataVersionFilterList; + attributePath.release(); + eventPath.release(); + + // TODO: Change from local filter list generation to rehydrating ClusterStateCache ot take advantage of existing filter list sorting algorithm + + // SendAutoResubscribeRequest cleans up the params, even on failure. + err = readClient->SendAutoResubscribeRequest(std::move(readParams)); + if (err == CHIP_NO_ERROR) { + break; + } + + // If error is not a "no memory" issue, then break and go through regular resubscribe logic + if (err != CHIP_ERROR_NO_MEMORY) { + break; + } + + // If "no memory" error is not caused by data version filter list, break as well + if (!dataVersionFilterListSize) { + break; + } + + // Now "no memory" could mean subscribe request packet space ran out. Reduce size and try again immediately + dataVersionFilterListSizeReduction++; + } if (err != CHIP_NO_ERROR) { NSError * error = [MTRError errorForCHIPErrorCode:err logContext:self]; MTR_LOG_ERROR("%@ SendAutoResubscribeRequest error %@", self, error); dispatch_async(self.queue, ^{ [self _handleSubscriptionError:error]; + [self _handleSubscriptionReset]; }); return; } + MTR_LOG_DEFAULT("%@ Subscribe with data version list size %lu, reduced by %lu", self, dataVersions.count, dataVersionFilterListSizeReduction); + // Callback and ClusterStateCache and ReadClient will be deleted // when OnDone is called. os_unfair_lock_lock(&self->_lock); @@ -818,6 +907,15 @@ - (void)_setupSubscription }]; } +#ifdef DEBUG +- (NSUInteger)unitTestAttributesReportedSinceLastCheck +{ + NSUInteger attributesReportedSinceLastCheck = _unitTestAttributesReportedSinceLastCheck; + _unitTestAttributesReportedSinceLastCheck = 0; + return attributesReportedSinceLastCheck; +} +#endif + #pragma mark Device Interactions // Helper function to determine whether an attribute has "Changes Omitted" quality, which indicates that past the priming report in @@ -1055,6 +1153,7 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath) readAttributePaths:attributePaths eventPaths:nil params:readParams + includeDataVersion:YES queue:self.queue completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { if (values) { @@ -1980,7 +2079,8 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT] }]; } else { - id value = MTRDecodeDataValueDictionaryFromCHIPTLV(apData); + NSNumber * dataVersionNumber = aPath.mDataVersion.HasValue() ? @(aPath.mDataVersion.Value()) : nil; + NSDictionary * value = MTRDecodeDataValueDictionaryFromCHIPTLV(apData, dataVersionNumber); if (value == nil) { MTR_LOG_ERROR("Failed to decode attribute data for path %@", attributePath); [mAttributeReports addObject:@ { diff --git a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm index c37754e15a95e9..36dc5b27410da1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm @@ -138,6 +138,16 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri params:(MTRReadParams * _Nullable)params queue:(dispatch_queue_t)queue completion:(MTRDeviceResponseHandler)completion +{ + [self readAttributePaths:attributePaths eventPaths:eventPaths params:params includeDataVersion:NO queue:queue completion:completion]; +} + +- (void)readAttributePaths:(NSArray * _Nullable)attributePaths + eventPaths:(NSArray * _Nullable)eventPaths + params:(MTRReadParams * _Nullable)params + includeDataVersion:(BOOL)includeDataVersion + queue:(dispatch_queue_t)queue + completion:(MTRDeviceResponseHandler)completion { if (attributePaths == nil || eventPaths != nil) { MTR_LOG_ERROR("MTRBaseDevice doesn't support reading event paths over XPC"); diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 604ed600fb02d6..661dbadb9ac445 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -51,6 +51,7 @@ - (void)removeDevice:(MTRDevice *)device; @interface MTRDevice (Test) - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther; +- (NSUInteger)unitTestAttributesReportedSinceLastCheck; @end #endif // DEBUG @@ -1235,20 +1236,45 @@ - (void)test009_TestDataStoreMTRDevice [controller removeDevice:device]; // Verify the new device is initialized with the same values - __auto_type * deviceNew = [MTRDevice deviceWithNodeID:deviceID controller:controller]; - BOOL storedAttributeDifferFromMTRDevice = NO; + __auto_type * newDevice = [MTRDevice deviceWithNodeID:deviceID controller:controller]; + NSUInteger storedAttributeDifferFromMTRDeviceCount = 0; for (NSDictionary * responseValue in dataStoreValues) { MTRAttributePath * path = responseValue[MTRAttributePathKey]; XCTAssertNotNil(path); NSDictionary * dataValue = responseValue[MTRDataKey]; XCTAssertNotNil(dataValue); - NSDictionary * dataValueFromMTRDevice = [deviceNew readAttributeWithEndpointID:path.endpoint clusterID:path.cluster attributeID:path.attribute params:nil]; - if (![deviceNew _attributeDataValue:dataValue isEqualToDataValue:dataValueFromMTRDevice]) { - storedAttributeDifferFromMTRDevice = YES; + NSDictionary * dataValueFromMTRDevice = [newDevice readAttributeWithEndpointID:path.endpoint clusterID:path.cluster attributeID:path.attribute params:nil]; + if (![newDevice _attributeDataValue:dataValue isEqualToDataValue:dataValueFromMTRDevice]) { + storedAttributeDifferFromMTRDeviceCount++; } } - XCTAssertTrue(storedAttributeDifferFromMTRDevice); + + // Only test that 90% of attributes are the same because there are some changing attributes each time (UTC time, for example) + // * With all-clusters-app as of 2024-02-10, about 1.476% of attributes change. + double storedAttributeDifferFromMTRDevicePercentage = storedAttributeDifferFromMTRDeviceCount * 100.0 / dataStoreValues.count; + XCTAssertTrue(storedAttributeDifferFromMTRDevicePercentage < 10.0); + + // Now + __auto_type * newDelegate = [[MTRDeviceTestDelegate alloc] init]; + + XCTestExpectation * newDeviceSubscriptionExpectation = [self expectationWithDescription:@"Subscription has been set up for new device"]; + + newDelegate.onReportEnd = ^{ + [newDeviceSubscriptionExpectation fulfill]; + }; + + [newDevice setDelegate:newDelegate queue:queue]; + + [self waitForExpectations:@[ newDeviceSubscriptionExpectation ] timeout:60]; + newDelegate.onReportEnd = nil; + + // 1) MTRDevice actually gets some attributes reported more than once + // 2) Some attributes do change on resubscribe + // * With all-clusts-app as of 2024-02-10, out of 1287 persisted attributes, still 450 attributes were reported with filter + // And so conservatively, assert that data version filters save at least 300 entries. + NSUInteger storedAttributeCountDifferenceFromMTRDeviceReport = dataStoreValues.count - [device unitTestAttributesReportedSinceLastCheck]; + XCTAssertTrue(storedAttributeCountDifferenceFromMTRDeviceReport > 300); // Reset our commissionee. __auto_type * baseDevice = [MTRBaseDevice deviceWithNodeID:deviceID controller:controller];