From 8a327fcd042c4ff855903df0a45281f7504ff743 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Wed, 22 May 2024 16:13:24 -0700 Subject: [PATCH 1/9] add metric keys for additional metrics --- src/darwin/Framework/CHIP/MTRMetricKeys.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/darwin/Framework/CHIP/MTRMetricKeys.h b/src/darwin/Framework/CHIP/MTRMetricKeys.h index 5ea03da70d9752..536da36b582053 100644 --- a/src/darwin/Framework/CHIP/MTRMetricKeys.h +++ b/src/darwin/Framework/CHIP/MTRMetricKeys.h @@ -69,6 +69,9 @@ constexpr Tracing::MetricKey kMetricDeviceVendorID = "dwnfw_device_vendor_id"; // Device Product ID constexpr Tracing::MetricKey kMetricDeviceProductID = "dwnfw_device_product_id"; +// Device Uses Thread +constexpr Tracing::MetricKey kMetricDeviceUsesThread = "dwnfw_device_uses_thread_bool"; + // Counter of number of devices discovered on the network during setup constexpr Tracing::MetricKey kMetricOnNetworkDevicesAdded = "dwnfw_onnet_devices_added"; @@ -81,6 +84,9 @@ constexpr Tracing::MetricKey kMetricBLEDevicesAdded = "dwnfw_ble_devices_added"; // Counter of number of BLE devices removed during setup constexpr Tracing::MetricKey kMetricBLEDevicesRemoved = "dwnfw_ble_devices_removed"; +// Unexpected C quality attribute update outside of priming +constexpr Tracing::MetricKey kMetricUnexpectedCQualityUpdate = "dwnpm_bad_c_attr_update"; + } // namespace DarwinFramework } // namespace Tracing } // namespace chip From a8c9851928630c95e5fb687034150f8c729b5185 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Wed, 22 May 2024 16:14:26 -0700 Subject: [PATCH 2/9] add metric collection for unexpected C Quality attribute update --- src/darwin/Framework/CHIP/MTRDevice.mm | 70 +++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 4014bd5f6924bf..d32ad5c7baa1ba 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -35,6 +35,8 @@ #import "MTRError_Internal.h" #import "MTREventTLVValueDecoder_Internal.h" #import "MTRLogging_Internal.h" +#import "MTRMetricKeys.h" +#import "MTRMetricsCollector.h" #import "MTRTimeUtils.h" #import "MTRUnfairLock.h" #import "zap-generated/MTRCommandPayloads_Internal.h" @@ -91,6 +93,39 @@ - (id)strongObject } @end +// Stores essential-for-logging attributes immutably for use in logs +@interface MTRDeviceEssentialAttributes : NSObject +@property (readonly) UInt16 vendorID; +@property (readonly) UInt16 productID; +@property (readonly) BOOL usesThread; + +- (void)addEssentialAttributesToCurrentMetricScope; + +@end + +@implementation MTRDeviceEssentialAttributes + +- (instancetype)initWithVendorID:(UInt16)vendorID productID:(UInt16)productID usesThread:(BOOL)usesThread { + self = [super init]; + + if (self) { + _vendorID = vendorID; + _productID = productID; + _usesThread = usesThread; + } + + return self; +} + +- (void)addEssentialAttributesToCurrentMetricScope { + using namespace chip::Tracing::DarwinFramework; + MATTER_LOG_METRIC(kMetricDeviceVendorID, _vendorID); + MATTER_LOG_METRIC(kMetricDeviceProductID, _productID); + MATTER_LOG_METRIC(kMetricDeviceUsesThread, _usesThread); +} + +@end + NSNumber * MTRClampedNumber(NSNumber * aNumber, NSNumber * min, NSNumber * max) { if ([aNumber compare:min] == NSOrderedAscending) { @@ -324,6 +359,10 @@ @interface MTRDevice () @property (nonatomic) MTRInternalDeviceState internalDeviceState; +// TODO: cache this once I understand the point in the MTRDevice lifecycle that the relevant attributes will be present. +// kmo 22 may 2024 14h55 +// @property (nonatomic) MTRDeviceEssentialAttributes * essentialAttributes; + #define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MIN_WAIT_SECONDS (1) #define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MAX_WAIT_SECONDS (3600) @property (nonatomic) uint32_t lastSubscriptionAttemptWait; @@ -1715,6 +1754,8 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor BOOL isStartUpEvent = (eventPath.cluster.unsignedLongValue == MTRClusterIDTypeBasicInformationID) && (eventPath.event.unsignedLongValue == MTREventIDTypeClusterBasicInformationEventStartUpID); if (isStartUpEvent) { + // REVIEWERS: this seems like a good place to set up / cache + // the essential device attributes - is it? if (_estimatedStartTimeFromGeneralDiagnosticsUpTime) { // If UpTime was received, make use of it as mark of system start time MTR_LOG("%@ StartUp event: set estimated start time forward to %@", self, @@ -1886,9 +1927,18 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f && isFromSubscription && !_receivingPrimingReport && AttributeHasChangesOmittedQuality(path)) { - // Do not persist new values for Changes Omitted Quality attributes unless - // they're part of a Priming Report or from a read response. + // Do not persist new values for Changes Omitted Quality (aka C Quality) + // attributes unless they're part of a Priming Report or from a read response. // (removals are OK) + + // log when a device violates expectations for Changes Omitted Quality attributes. + MTRDeviceEssentialAttributes * attributes = [self _essentialAttributesForCurrentState]; + + using namespace chip::Tracing::DarwinFramework; + MATTER_LOG_METRIC_BEGIN(kMetricUnexpectedCQualityUpdate); + [attributes addEssentialAttributesToCurrentMetricScope]; + MATTER_LOG_METRIC_END(kMetricUnexpectedCQualityUpdate); + return; } @@ -3642,6 +3692,22 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID [self.temporaryMetaDataCache removeObjectForKey:[NSString stringWithFormat:@"%@:%@", key, endpointID]]; } +#pragma mark Log Help + +- (MTRDeviceEssentialAttributes *)_essentialAttributesForCurrentState { + MTRClusterPath * basicInfoClusterPath = [MTRClusterPath clusterPathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID)]; + MTRDeviceClusterData * basicInfoClusterData = [self _clusterDataForPath:basicInfoClusterPath]; + + NSNumber * vidObj = basicInfoClusterData.attributes[@(MTRAttributeIDTypeClusterBasicInformationAttributeVendorIDID)][MTRValueKey]; + UInt16 vendorID = vidObj.unsignedShortValue; + NSNumber * pidObj = basicInfoClusterData.attributes[@(MTRAttributeIDTypeClusterBasicInformationAttributeProductIDID)][MTRValueKey]; + UInt16 productID = pidObj.unsignedShortValue; + + BOOL usesThread = [self _deviceUsesThread]; + + return [[MTRDeviceEssentialAttributes alloc] initWithVendorID:vendorID productID:productID usesThread:usesThread]; +} + @end /* BEGIN DRAGONS: Note methods here cannot be renamed, and are used by private callers, do not rename, remove or modify behavior here */ From 3c53619ec52c23918580385caf68eaeb82e9ae2e Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Fri, 24 May 2024 12:20:21 -0700 Subject: [PATCH 3/9] unwind premature optimization placeholders/comments --- src/darwin/Framework/CHIP/MTRDevice.mm | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index d32ad5c7baa1ba..0d15599a6d78d6 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -359,10 +359,6 @@ @interface MTRDevice () @property (nonatomic) MTRInternalDeviceState internalDeviceState; -// TODO: cache this once I understand the point in the MTRDevice lifecycle that the relevant attributes will be present. -// kmo 22 may 2024 14h55 -// @property (nonatomic) MTRDeviceEssentialAttributes * essentialAttributes; - #define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MIN_WAIT_SECONDS (1) #define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MAX_WAIT_SECONDS (3600) @property (nonatomic) uint32_t lastSubscriptionAttemptWait; @@ -1754,8 +1750,6 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor BOOL isStartUpEvent = (eventPath.cluster.unsignedLongValue == MTRClusterIDTypeBasicInformationID) && (eventPath.event.unsignedLongValue == MTREventIDTypeClusterBasicInformationEventStartUpID); if (isStartUpEvent) { - // REVIEWERS: this seems like a good place to set up / cache - // the essential device attributes - is it? if (_estimatedStartTimeFromGeneralDiagnosticsUpTime) { // If UpTime was received, make use of it as mark of system start time MTR_LOG("%@ StartUp event: set estimated start time forward to %@", self, From ada74dbd470ffc0bfa2201830021fc4062ec73c6 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Fri, 24 May 2024 12:26:26 -0700 Subject: [PATCH 4/9] rename to indicate the attributes are for logging / informational use --- src/darwin/Framework/CHIP/MTRDevice.mm | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 0d15599a6d78d6..684df930ba02cd 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -93,17 +93,17 @@ - (id)strongObject } @end -// Stores essential-for-logging attributes immutably for use in logs -@interface MTRDeviceEssentialAttributes : NSObject +// convenience object for commonly-logged device attributes +@interface MTRDeviceInformationalAttributes : NSObject @property (readonly) UInt16 vendorID; @property (readonly) UInt16 productID; @property (readonly) BOOL usesThread; -- (void)addEssentialAttributesToCurrentMetricScope; +- (void)addInformationalAttributesToCurrentMetricScope; @end -@implementation MTRDeviceEssentialAttributes +@implementation MTRDeviceInformationalAttributes - (instancetype)initWithVendorID:(UInt16)vendorID productID:(UInt16)productID usesThread:(BOOL)usesThread { self = [super init]; @@ -117,7 +117,7 @@ - (instancetype)initWithVendorID:(UInt16)vendorID productID:(UInt16)productID us return self; } -- (void)addEssentialAttributesToCurrentMetricScope { +- (void)addInformationalAttributesToCurrentMetricScope { using namespace chip::Tracing::DarwinFramework; MATTER_LOG_METRIC(kMetricDeviceVendorID, _vendorID); MATTER_LOG_METRIC(kMetricDeviceProductID, _productID); @@ -1926,11 +1926,11 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f // (removals are OK) // log when a device violates expectations for Changes Omitted Quality attributes. - MTRDeviceEssentialAttributes * attributes = [self _essentialAttributesForCurrentState]; + MTRDeviceInformationalAttributes * attributes = [self _informationalAttributesForCurrentState]; using namespace chip::Tracing::DarwinFramework; MATTER_LOG_METRIC_BEGIN(kMetricUnexpectedCQualityUpdate); - [attributes addEssentialAttributesToCurrentMetricScope]; + [attributes addInformationalAttributesToCurrentMetricScope]; MATTER_LOG_METRIC_END(kMetricUnexpectedCQualityUpdate); return; @@ -3688,7 +3688,7 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID #pragma mark Log Help -- (MTRDeviceEssentialAttributes *)_essentialAttributesForCurrentState { +- (MTRDeviceInformationalAttributes *)_informationalAttributesForCurrentState { MTRClusterPath * basicInfoClusterPath = [MTRClusterPath clusterPathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID)]; MTRDeviceClusterData * basicInfoClusterData = [self _clusterDataForPath:basicInfoClusterPath]; @@ -3699,7 +3699,7 @@ - (MTRDeviceEssentialAttributes *)_essentialAttributesForCurrentState { BOOL usesThread = [self _deviceUsesThread]; - return [[MTRDeviceEssentialAttributes alloc] initWithVendorID:vendorID productID:productID usesThread:usesThread]; + return [[MTRDeviceInformationalAttributes alloc] initWithVendorID:vendorID productID:productID usesThread:usesThread]; } @end From acb88a2ce1ca00107bcb5ae8565edf4c04b8328c Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Fri, 24 May 2024 18:32:18 -0700 Subject: [PATCH 5/9] simplify - pull attributes from cache without intermediate object, should be fast enough --- src/darwin/Framework/CHIP/MTRDevice.mm | 79 +++++++++++--------------- 1 file changed, 33 insertions(+), 46 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 684df930ba02cd..3f28d404c1cfc0 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -93,39 +93,6 @@ - (id)strongObject } @end -// convenience object for commonly-logged device attributes -@interface MTRDeviceInformationalAttributes : NSObject -@property (readonly) UInt16 vendorID; -@property (readonly) UInt16 productID; -@property (readonly) BOOL usesThread; - -- (void)addInformationalAttributesToCurrentMetricScope; - -@end - -@implementation MTRDeviceInformationalAttributes - -- (instancetype)initWithVendorID:(UInt16)vendorID productID:(UInt16)productID usesThread:(BOOL)usesThread { - self = [super init]; - - if (self) { - _vendorID = vendorID; - _productID = productID; - _usesThread = usesThread; - } - - return self; -} - -- (void)addInformationalAttributesToCurrentMetricScope { - using namespace chip::Tracing::DarwinFramework; - MATTER_LOG_METRIC(kMetricDeviceVendorID, _vendorID); - MATTER_LOG_METRIC(kMetricDeviceProductID, _productID); - MATTER_LOG_METRIC(kMetricDeviceUsesThread, _usesThread); -} - -@end - NSNumber * MTRClampedNumber(NSNumber * aNumber, NSNumber * min, NSNumber * max) { if ([aNumber compare:min] == NSOrderedAscending) { @@ -1916,7 +1883,7 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f } [clusterData storeValue:value forAttribute:path.attribute]; - + if (value != nil && isFromSubscription && !_receivingPrimingReport @@ -1926,11 +1893,9 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f // (removals are OK) // log when a device violates expectations for Changes Omitted Quality attributes. - MTRDeviceInformationalAttributes * attributes = [self _informationalAttributesForCurrentState]; - using namespace chip::Tracing::DarwinFramework; MATTER_LOG_METRIC_BEGIN(kMetricUnexpectedCQualityUpdate); - [attributes addInformationalAttributesToCurrentMetricScope]; + [self _addInformationalAttributesToCurrentMetricScope]; MATTER_LOG_METRIC_END(kMetricUnexpectedCQualityUpdate); return; @@ -3688,18 +3653,40 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID #pragma mark Log Help -- (MTRDeviceInformationalAttributes *)_informationalAttributesForCurrentState { - MTRClusterPath * basicInfoClusterPath = [MTRClusterPath clusterPathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID)]; - MTRDeviceClusterData * basicInfoClusterData = [self _clusterDataForPath:basicInfoClusterPath]; +- (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath { + auto * cachedData = [self _cachedAttributeValueForPath:attributePath]; - NSNumber * vidObj = basicInfoClusterData.attributes[@(MTRAttributeIDTypeClusterBasicInformationAttributeVendorIDID)][MTRValueKey]; - UInt16 vendorID = vidObj.unsignedShortValue; - NSNumber * pidObj = basicInfoClusterData.attributes[@(MTRAttributeIDTypeClusterBasicInformationAttributeProductIDID)][MTRValueKey]; - UInt16 productID = pidObj.unsignedShortValue; + auto * attrReport = [[MTRAttributeReport alloc] initWithResponseValue:@{ + MTRAttributePathKey : attributePath, + MTRDataKey : cachedData, + } error:nil]; + // REVIEWERS: is it worth logging the `error` above? - BOOL usesThread = [self _deviceUsesThread]; + return attrReport.value; +} + +- (NSNumber *)_informationalVendorID { + auto * vendorIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) + clusterID:@(MTRClusterIDTypeBasicInformationID) + attributeID:@(MTRClusterBasicAttributeVendorIDID)]; - return [[MTRDeviceInformationalAttributes alloc] initWithVendorID:vendorID productID:productID usesThread:usesThread]; + return [self _informationalNumberAtAttributePath:vendorIDPath]; +} + +- (NSNumber *)_informationalProductID { + auto * productIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) + clusterID:@(MTRClusterIDTypeBasicInformationID) + attributeID:@(MTRClusterBasicAttributeProductIDID)]; + + return [self _informationalNumberAtAttributePath:productIDPath]; +} + +- (void)_addInformationalAttributesToCurrentMetricScope { + using namespace chip::Tracing::DarwinFramework; + MATTER_LOG_METRIC(kMetricDeviceVendorID, [self _informationalVendorID].unsignedShortValue); + MATTER_LOG_METRIC(kMetricDeviceProductID, [self _informationalProductID].unsignedShortValue); + BOOL usesThread = [self _deviceUsesThread]; + MATTER_LOG_METRIC(kMetricDeviceUsesThread, usesThread); } @end From 131020c0760ee741b0ed7cadd5d023d393066cb4 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Fri, 24 May 2024 18:33:26 -0700 Subject: [PATCH 6/9] revert accidental whitespace change --- src/darwin/Framework/CHIP/MTRDevice.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 3f28d404c1cfc0..496999c7b5f79f 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1883,7 +1883,7 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f } [clusterData storeValue:value forAttribute:path.attribute]; - + if (value != nil && isFromSubscription && !_receivingPrimingReport From 7456e16eec41a8c281c62c696286f1a56cda74d8 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 28 May 2024 23:37:20 +0000 Subject: [PATCH 7/9] Restyled by whitespace --- src/darwin/Framework/CHIP/MTRDevice.mm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 496999c7b5f79f..4bc99244c37a43 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1891,13 +1891,13 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f // Do not persist new values for Changes Omitted Quality (aka C Quality) // attributes unless they're part of a Priming Report or from a read response. // (removals are OK) - + // log when a device violates expectations for Changes Omitted Quality attributes. using namespace chip::Tracing::DarwinFramework; MATTER_LOG_METRIC_BEGIN(kMetricUnexpectedCQualityUpdate); [self _addInformationalAttributesToCurrentMetricScope]; MATTER_LOG_METRIC_END(kMetricUnexpectedCQualityUpdate); - + return; } @@ -3655,13 +3655,13 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID - (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath { auto * cachedData = [self _cachedAttributeValueForPath:attributePath]; - + auto * attrReport = [[MTRAttributeReport alloc] initWithResponseValue:@{ MTRAttributePathKey : attributePath, MTRDataKey : cachedData, } error:nil]; // REVIEWERS: is it worth logging the `error` above? - + return attrReport.value; } @@ -3669,7 +3669,7 @@ - (NSNumber *)_informationalVendorID { auto * vendorIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID) attributeID:@(MTRClusterBasicAttributeVendorIDID)]; - + return [self _informationalNumberAtAttributePath:vendorIDPath]; } @@ -3677,7 +3677,7 @@ - (NSNumber *)_informationalProductID { auto * productIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID) attributeID:@(MTRClusterBasicAttributeProductIDID)]; - + return [self _informationalNumberAtAttributePath:productIDPath]; } From 8a5d306fa20749ac16808dd7a1ee301a6ca8f326 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 28 May 2024 23:37:45 +0000 Subject: [PATCH 8/9] Restyled by clang-format --- src/darwin/Framework/CHIP/MTRDevice.mm | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 4bc99244c37a43..602f65552dc835 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -3653,35 +3653,40 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID #pragma mark Log Help -- (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath { +- (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath +{ auto * cachedData = [self _cachedAttributeValueForPath:attributePath]; auto * attrReport = [[MTRAttributeReport alloc] initWithResponseValue:@{ MTRAttributePathKey : attributePath, MTRDataKey : cachedData, - } error:nil]; + } + error:nil]; // REVIEWERS: is it worth logging the `error` above? return attrReport.value; } -- (NSNumber *)_informationalVendorID { +- (NSNumber *)_informationalVendorID +{ auto * vendorIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) - clusterID:@(MTRClusterIDTypeBasicInformationID) - attributeID:@(MTRClusterBasicAttributeVendorIDID)]; + clusterID:@(MTRClusterIDTypeBasicInformationID) + attributeID:@(MTRClusterBasicAttributeVendorIDID)]; return [self _informationalNumberAtAttributePath:vendorIDPath]; } -- (NSNumber *)_informationalProductID { +- (NSNumber *)_informationalProductID +{ auto * productIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) - clusterID:@(MTRClusterIDTypeBasicInformationID) - attributeID:@(MTRClusterBasicAttributeProductIDID)]; + clusterID:@(MTRClusterIDTypeBasicInformationID) + attributeID:@(MTRClusterBasicAttributeProductIDID)]; return [self _informationalNumberAtAttributePath:productIDPath]; } -- (void)_addInformationalAttributesToCurrentMetricScope { +- (void)_addInformationalAttributesToCurrentMetricScope +{ using namespace chip::Tracing::DarwinFramework; MATTER_LOG_METRIC(kMetricDeviceVendorID, [self _informationalVendorID].unsignedShortValue); MATTER_LOG_METRIC(kMetricDeviceProductID, [self _informationalProductID].unsignedShortValue); From 364a2eb7c8445a81e89ba70b30fbce03eee86ecc Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Mon, 3 Jun 2024 09:57:44 -0700 Subject: [PATCH 9/9] make `nil` case possibility more obvious --- src/darwin/Framework/CHIP/MTRDevice.mm | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 602f65552dc835..dcb6a9acd8da64 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -3653,7 +3653,7 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID #pragma mark Log Help -- (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath +- (nullable NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath { auto * cachedData = [self _cachedAttributeValueForPath:attributePath]; @@ -3662,12 +3662,11 @@ - (NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributeP MTRDataKey : cachedData, } error:nil]; - // REVIEWERS: is it worth logging the `error` above? return attrReport.value; } -- (NSNumber *)_informationalVendorID +- (nullable NSNumber *)_informationalVendorID { auto * vendorIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID) @@ -3676,7 +3675,7 @@ - (NSNumber *)_informationalVendorID return [self _informationalNumberAtAttributePath:vendorIDPath]; } -- (NSNumber *)_informationalProductID +- (nullable NSNumber *)_informationalProductID { auto * productIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId) clusterID:@(MTRClusterIDTypeBasicInformationID)