From 2d7ea48b646f1c37a368d9a29b2ab5d6c8dec2cf Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 18 Sep 2024 14:43:03 -0400 Subject: [PATCH 1/2] Clean up the "subclasses must implement this" machinery in MTRDevice. We had a lot of copy/paste that we can factor out. --- src/darwin/Framework/CHIP/MTRDevice.mm | 47 ++++++++++---------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 46fc8faa40ee1d..7ec090e6290200 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -55,6 +55,18 @@ // Disabling pending crashes #define ENABLE_CONNECTIVITY_MONITORING 0 +#ifdef DEBUG +#define MTR_REQUIRE_SUBCLASS_IMPL_PASTED(message) \ + MTR_LOG_ERROR(message); \ + NSAssert(NO, @message) +#else // DEBUG +#debug MTR_REQUIRE_SUBCLASS_IMPL_PASTED(message) \ + MTR_LOG_ERROR(message) +#endif // DEBUG + +#define MTR_REQUIRE_SUBCLASS_IMPL(selectorString) \ + MTR_REQUIRE_SUBCLASS_IMPL_PASTED("MTRDevice " selectorString " must be handled by subclasses") + @implementation MTRDeviceDelegateInfo - (instancetype)initWithDelegate:(id)delegate queue:(dispatch_queue_t)queue interestedPathsForAttributes:(NSArray * _Nullable)interestedPathsForAttributes interestedPathsForEvents:(NSArray * _Nullable)interestedPathsForEvents { @@ -1248,12 +1260,7 @@ - (NSUInteger)unitTestNonnullDelegateCount attributeID:(NSNumber *)attributeID params:(MTRReadParams * _Nullable)params { -#define MTRDeviceErrorStr "MTRDevice readAttributeWithEndpointID:clusterID:attributeID:params: must be handled by subclasses" - MTR_LOG_ERROR(MTRDeviceErrorStr); -#ifdef DEBUG - NSAssert(NO, @MTRDeviceErrorStr); -#endif // DEBUG -#undef MTRDeviceErrorStr + MTR_REQUIRE_SUBCLASS_IMPL("readAttributeWithEndpointID:clusterID:attributeID:params:"); return nil; } @@ -1264,22 +1271,12 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID expectedValueInterval:(NSNumber *)expectedValueInterval timedWriteTimeout:(NSNumber * _Nullable)timeout { -#define MTRDeviceErrorStr "MTRDevice writeAttributeWithEndpointID:clusterID:attributeID:value:expectedValueInterval:timedWriteTimeout: must be handled by subclasses" - MTR_LOG_ERROR(MTRDeviceErrorStr); -#ifdef DEBUG - NSAssert(NO, @MTRDeviceErrorStr); -#endif // DEBUG -#undef MTRDeviceErrorStr + MTR_REQUIRE_SUBCLASS_IMPL("writeAttributeWithEndpointID:clusterID:attributeID:value:expectedValueInterval:timedWriteTimeout:"); } - (NSArray *> *)readAttributePaths:(NSArray *)attributePaths { -#define MTRDeviceErrorStr "MTRDevice readAttributePaths: must be handled by subclasses" - MTR_LOG_ERROR(MTRDeviceErrorStr); -#ifdef DEBUG - NSAssert(NO, @MTRDeviceErrorStr); -#endif // DEBUG -#undef MTRDeviceErrorStr + MTR_REQUIRE_SUBCLASS_IMPL("readAttributePaths:"); return [NSArray array]; } @@ -1362,12 +1359,7 @@ - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID queue:(dispatch_queue_t)queue completion:(MTRDeviceResponseHandler)completion { -#define MTRDeviceErrorStr "MTRDevice _invokeCommandWithEndpointID: must be handled by subclasses" - MTR_LOG_ERROR(MTRDeviceErrorStr); -#ifdef DEBUG - NSAssert(NO, @MTRDeviceErrorStr); -#endif // DEBUG -#undef MTRDeviceErrorStr + MTR_REQUIRE_SUBCLASS_IMPL("_invokeCommandWithEndpointID:clusterID:commandID:commandFields:expectedValues:expectedValueInterval:timedInvokeTimeout:serverSideProcessingTimeout:queue:completion:"); } - (void)_invokeKnownCommandWithEndpointID:(NSNumber *)endpointID @@ -1476,12 +1468,7 @@ - (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue - (NSArray *> *)getAllAttributesReport { -#define MTRDeviceErrorStr "MTRDevice getAllAttributesReport must be handled by subclasses that support it" - MTR_LOG_ERROR(MTRDeviceErrorStr); -#ifdef DEBUG - NSAssert(NO, @MTRDeviceErrorStr); -#endif // DEBUG -#undef MTRDeviceErrorStr + MTR_REQUIRE_SUBCLASS_IMPL("getAllAttributesReport"); return nil; } From eb4db0fb9a7c2006b31d48de3d2000f8981a084a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 18 Sep 2024 23:35:28 -0400 Subject: [PATCH 2/2] Address review comments. --- .../Framework/CHIP/MTRDefines_Internal.h | 14 ++++++++++++ src/darwin/Framework/CHIP/MTRDevice.mm | 22 +++++-------------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDefines_Internal.h b/src/darwin/Framework/CHIP/MTRDefines_Internal.h index 14683f34c553f9..5a450ece8ec7f3 100644 --- a/src/darwin/Framework/CHIP/MTRDefines_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDefines_Internal.h @@ -152,3 +152,17 @@ typedef struct {} variable_hidden_by_mtr_hide; #endif #define MTR_YES_NO(x) ((x) ? @"YES" : @"NO") + +#ifdef DEBUG +#define _MTR_ABSTRACT_METHOD_IMPL(message, ...) \ + do { \ + MTR_LOG_ERROR(message, __VA_ARGS__); \ + @throw [NSException exceptionWithName:NSInternalInconsistencyException reason:[NSString stringWithFormat:@message, __VA_ARGS__] userInfo:nil]; \ + } while (0) +#else // DEBUG +#define _MTR_ABSTRACT_METHOD_IMPL(message, ...) \ + MTR_LOG_ERROR(message, __VA_ARGS__) +#endif // DEBUG + +#define MTR_ABSTRACT_METHOD() \ + _MTR_ABSTRACT_METHOD_IMPL("%@ or some ancestor must implement %@", self.class, NSStringFromSelector(_cmd)) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 7ec090e6290200..cabcb448ec348e 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -55,18 +55,6 @@ // Disabling pending crashes #define ENABLE_CONNECTIVITY_MONITORING 0 -#ifdef DEBUG -#define MTR_REQUIRE_SUBCLASS_IMPL_PASTED(message) \ - MTR_LOG_ERROR(message); \ - NSAssert(NO, @message) -#else // DEBUG -#debug MTR_REQUIRE_SUBCLASS_IMPL_PASTED(message) \ - MTR_LOG_ERROR(message) -#endif // DEBUG - -#define MTR_REQUIRE_SUBCLASS_IMPL(selectorString) \ - MTR_REQUIRE_SUBCLASS_IMPL_PASTED("MTRDevice " selectorString " must be handled by subclasses") - @implementation MTRDeviceDelegateInfo - (instancetype)initWithDelegate:(id)delegate queue:(dispatch_queue_t)queue interestedPathsForAttributes:(NSArray * _Nullable)interestedPathsForAttributes interestedPathsForEvents:(NSArray * _Nullable)interestedPathsForEvents { @@ -1260,7 +1248,7 @@ - (NSUInteger)unitTestNonnullDelegateCount attributeID:(NSNumber *)attributeID params:(MTRReadParams * _Nullable)params { - MTR_REQUIRE_SUBCLASS_IMPL("readAttributeWithEndpointID:clusterID:attributeID:params:"); + MTR_ABSTRACT_METHOD(); return nil; } @@ -1271,12 +1259,12 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID expectedValueInterval:(NSNumber *)expectedValueInterval timedWriteTimeout:(NSNumber * _Nullable)timeout { - MTR_REQUIRE_SUBCLASS_IMPL("writeAttributeWithEndpointID:clusterID:attributeID:value:expectedValueInterval:timedWriteTimeout:"); + MTR_ABSTRACT_METHOD(); } - (NSArray *> *)readAttributePaths:(NSArray *)attributePaths { - MTR_REQUIRE_SUBCLASS_IMPL("readAttributePaths:"); + MTR_ABSTRACT_METHOD(); return [NSArray array]; } @@ -1359,7 +1347,7 @@ - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID queue:(dispatch_queue_t)queue completion:(MTRDeviceResponseHandler)completion { - MTR_REQUIRE_SUBCLASS_IMPL("_invokeCommandWithEndpointID:clusterID:commandID:commandFields:expectedValues:expectedValueInterval:timedInvokeTimeout:serverSideProcessingTimeout:queue:completion:"); + MTR_ABSTRACT_METHOD(); } - (void)_invokeKnownCommandWithEndpointID:(NSNumber *)endpointID @@ -1468,7 +1456,7 @@ - (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue - (NSArray *> *)getAllAttributesReport { - MTR_REQUIRE_SUBCLASS_IMPL("getAllAttributesReport"); + MTR_ABSTRACT_METHOD(); return nil; }