From 2719842cca03e7f788e72b2334f3e3ef6c92d1b5 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 18 Nov 2022 00:04:13 -0500 Subject: [PATCH] [Darwin] Fix Matter framework delegates to pass the delegating object as the first arg to their methods (#23665) * [Darwin] Fix Matter framework delegates to pass the delegating object as the first arg to their methods This is a re-landing of PR #22682 and PR #22690 but with backwards compat shims in place. The changes to MTRDeviceControllerDelegate are OK because this protocol is newly introduced and has not shipped yet. * Address review comment. Co-authored-by: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> --- .../pairing/DeviceControllerDelegateBridge.h | 6 ++-- .../pairing/DeviceControllerDelegateBridge.mm | 4 +-- .../commands/tests/TestCommandBridge.h | 12 +++---- .../CHIP/MTRDeviceAttestationDelegate.h | 27 +++++++++++----- .../MTRDeviceAttestationDelegateBridge.mm | 31 ++++++++++++++----- .../Framework/CHIP/MTRDeviceController.mm | 19 ++++++------ .../CHIP/MTRDeviceControllerDelegate.h | 8 +++-- .../CHIP/MTRDeviceControllerDelegateBridge.h | 8 +++-- .../CHIP/MTRDeviceControllerDelegateBridge.mm | 27 ++++++++++------ .../Framework/CHIPTests/MTRDeviceTests.m | 6 ++-- .../CHIPTests/MTRXPCListenerSampleTests.m | 6 ++-- 11 files changed, 97 insertions(+), 57 deletions(-) diff --git a/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h b/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h index 7d62b1ba960fb4..334d2339536b15 100644 --- a/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h +++ b/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h @@ -20,13 +20,15 @@ #import +@class MTRDeviceController; + @interface CHIPToolDeviceControllerDelegate : NSObject @property PairingCommandBridge * commandBridge; @property chip::NodeId deviceID; @property MTRDeviceController * commissioner; @property MTRCommissioningParameters * params; -- (void)onCommissioningSessionEstablishmentDone:(NSError *)error; -- (void)onCommissioningComplete:(NSError *)error; +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError *)error; +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error; @end diff --git a/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm b/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm index 06dd78a7d8ffc8..dc53280b2d2553 100644 --- a/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm +++ b/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm @@ -44,7 +44,7 @@ - (void)onStatusUpdate:(MTRCommissioningStatus)status } } -- (void)onCommissioningSessionEstablishmentDone:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError *)error { if (error != nil) { ChipLogProgress(chipTool, "PASE establishment failed"); @@ -61,7 +61,7 @@ - (void)onCommissioningSessionEstablishmentDone:(NSError *)error } } -- (void)onCommissioningComplete:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error { _commandBridge->SetCommandExitStatus(error, "Pairing Commissioning Complete"); } diff --git a/examples/darwin-framework-tool/commands/tests/TestCommandBridge.h b/examples/darwin-framework-tool/commands/tests/TestCommandBridge.h index 8f031981bd82b3..3873ea8fafa29f 100644 --- a/examples/darwin-framework-tool/commands/tests/TestCommandBridge.h +++ b/examples/darwin-framework-tool/commands/tests/TestCommandBridge.h @@ -48,9 +48,9 @@ constexpr const char * kDefaultKey = "default"; @property chip::NodeId deviceId; @property BOOL active; // Whether to pass on notifications to the commandBridge -- (void)onStatusUpdate:(MTRCommissioningStatus)status; -- (void)onCommissioningSessionEstablishmentDone:(NSError * _Nullable)error; -- (void)onCommissioningComplete:(NSError * _Nullable)error; +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status; +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error; +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError * _Nullable)error; - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithTestCommandBridge:(TestCommandBridge *)commandBridge; @@ -529,7 +529,7 @@ class TestCommandBridge : public CHIPCommandBridge, NS_ASSUME_NONNULL_BEGIN @implementation TestDeviceControllerDelegate -- (void)onStatusUpdate:(MTRCommissioningStatus)status +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status { if (_active) { if (status == MTRCommissioningStatusSuccess) { @@ -542,7 +542,7 @@ NS_ASSUME_NONNULL_BEGIN } } -- (void)onCommissioningSessionEstablishmentDone:(NSError * _Nullable)error +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error { if (_active) { if (error != nil) { @@ -556,7 +556,7 @@ NS_ASSUME_NONNULL_BEGIN } } -- (void)onCommissioningComplete:(NSError * _Nullable)error +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError * _Nullable)error { if (_active) { _active = NO; diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h index 9f30053a945ad0..d54fdeb577adb7 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h @@ -39,10 +39,10 @@ NS_ASSUME_NONNULL_BEGIN /** * Only one of the following delegate callbacks should be implemented. * - * If -deviceAttestation:failedForDevice:error: is implemented, then it will be called when device + * If -deviceAttestationFailedForController:device:error: is implemented, then it will be called when device * attestation fails, and the client can decide to continue or stop the commissioning. * - * If -deviceAttestation:completedForDevice:attestationDeviceInfo:error: is implemented, then it + * If -deviceAttestationCompletedForController:device:attestationDeviceInfo:error: is implemented, then it * will always be called when device attestation completes. */ @@ -58,10 +58,10 @@ NS_ASSUME_NONNULL_BEGIN * @param attestationDeviceInfo Attestation information for the device * @param error NSError representing the error code on attestation failure. Nil if success. */ -- (void)deviceAttestation:(MTRDeviceController *)controller - completedForDevice:(void *)device - attestationDeviceInfo:(MTRDeviceAttestationDeviceInfo *)attestationDeviceInfo - error:(NSError * _Nullable)error; +- (void)deviceAttestationCompletedForController:(MTRDeviceController *)controller + device:(void *)device + attestationDeviceInfo:(MTRDeviceAttestationDeviceInfo *)attestationDeviceInfo + error:(NSError * _Nullable)error MTR_NEWLY_AVAILABLE; /** * Notify the delegate when device attestation fails @@ -70,7 +70,20 @@ NS_ASSUME_NONNULL_BEGIN * @param device Handle of device being commissioned * @param error NSError representing the error code for the failure */ -- (void)deviceAttestation:(MTRDeviceController *)controller failedForDevice:(void *)device error:(NSError * _Nonnull)error; +- (void)deviceAttestationFailedForController:(MTRDeviceController *)controller + device:(void *)device + error:(NSError * _Nonnull)error MTR_NEWLY_AVAILABLE; + +- (void)deviceAttestation:(MTRDeviceController *)controller + completedForDevice:(void *)device + attestationDeviceInfo:(MTRDeviceAttestationDeviceInfo *)attestationDeviceInfo + error:(NSError * _Nullable)error + MTR_NEWLY_DEPRECATED("Please implement deviceAttestationCompletedForController:device:attestationDeviceInfo:error:"); + +- (void)deviceAttestation:(MTRDeviceController *)controller + failedForDevice:(void *)device + error:(NSError * _Nonnull)error + MTR_NEWLY_DEPRECATED("Please implement deviceAttestationFailedForController:device:error:"); @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm index 528148d3e0cf8d..4a489aa8ddef7b 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm @@ -30,7 +30,9 @@ mResult = attestationResult; id strongDelegate = mDeviceAttestationDelegate; - if ([strongDelegate respondsToSelector:@selector(deviceAttestation:completedForDevice:attestationDeviceInfo:error:)]) { + if ([strongDelegate respondsToSelector:@selector(deviceAttestationCompletedForController: + device:attestationDeviceInfo:error:)] + || [strongDelegate respondsToSelector:@selector(deviceAttestation:completedForDevice:attestationDeviceInfo:error:)]) { MTRDeviceController * strongController = mDeviceController; if (strongController) { NSData * dacData = AsData(info.dacDerBuffer()); @@ -43,18 +45,31 @@ NSError * error = (attestationResult == chip::Credentials::AttestationVerificationResult::kSuccess) ? nil : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTEGRITY_CHECK_FAILED]; - [strongDelegate deviceAttestation:mDeviceController - completedForDevice:device - attestationDeviceInfo:deviceInfo - error:error]; + if ([strongDelegate respondsToSelector:@selector + (deviceAttestationCompletedForController:device:attestationDeviceInfo:error:)]) { + [strongDelegate deviceAttestationCompletedForController:mDeviceController + device:device + attestationDeviceInfo:deviceInfo + error:error]; + } else { + [strongDelegate deviceAttestation:mDeviceController + completedForDevice:device + attestationDeviceInfo:deviceInfo + error:error]; + } } - } else if ((attestationResult != chip::Credentials::AttestationVerificationResult::kSuccess) && - [strongDelegate respondsToSelector:@selector(deviceAttestation:failedForDevice:error:)]) { + } else if ((attestationResult != chip::Credentials::AttestationVerificationResult::kSuccess) + && ([strongDelegate respondsToSelector:@selector(deviceAttestationFailedForController:device:error:)] || + [strongDelegate respondsToSelector:@selector(deviceAttestation:failedForDevice:error:)])) { MTRDeviceController * strongController = mDeviceController; if (strongController) { NSError * error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTEGRITY_CHECK_FAILED]; - [strongDelegate deviceAttestation:mDeviceController failedForDevice:device error:error]; + if ([strongDelegate respondsToSelector:@selector(deviceAttestationFailedForController:device:error:)]) { + [strongDelegate deviceAttestationFailedForController:mDeviceController device:device error:error]; + } else { + [strongDelegate deviceAttestation:mDeviceController failedForDevice:device error:error]; + } } } }); diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index c183353d70658e..c8722212a0d765 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -433,6 +433,8 @@ - (BOOL)commissionNodeWithID:(NSNumber *)nodeID } BOOL shouldWaitAfterDeviceAttestation = NO; if ([commissioningParams.deviceAttestationDelegate + respondsToSelector:@selector(deviceAttestationCompletedForController:device:attestationDeviceInfo:error:)] + || [commissioningParams.deviceAttestationDelegate respondsToSelector:@selector(deviceAttestation:completedForDevice:attestationDeviceInfo:error:)]) { shouldWaitAfterDeviceAttestation = YES; } @@ -542,7 +544,7 @@ - (void)setDeviceControllerDelegate:(id)delegate qu dispatch_async(_chipWorkQueue, ^{ VerifyOrReturn([self checkIsRunning]); - self->_deviceControllerDelegateBridge->setDelegate(delegate, queue); + self->_deviceControllerDelegateBridge->setDelegate(self, delegate, queue); }); } @@ -843,35 +845,32 @@ - (instancetype)initWithDelegate:(id)delegate - (BOOL)respondsToSelector:(SEL)selector { - // This logic will need to change a bit when the MTRDeviceControllerDelegate - // signatures change. It's shaped the way it is to make those changes - // easier. - if (selector == @selector(onStatusUpdate:)) { + if (selector == @selector(controller:statusUpdate:)) { return [self.delegate respondsToSelector:@selector(onStatusUpdate:)]; } - if (selector == @selector(onCommissioningSessionEstablishmentDone:)) { + if (selector == @selector(controller:commissioningSessionEstablishmentDone:)) { return [self.delegate respondsToSelector:@selector(onPairingComplete:)]; } - if (selector == @selector(onCommissioningComplete:)) { + if (selector == @selector(controller:commissioningComplete:)) { return [self.delegate respondsToSelector:@selector(onCommissioningComplete:)]; } return [super respondsToSelector:selector]; } -- (void)onStatusUpdate:(MTRCommissioningStatus)status +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status { [self.delegate onStatusUpdate:static_cast(status)]; } -- (void)onCommissioningSessionEstablishmentDone:(NSError * _Nullable)error +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error { [self.delegate onPairingComplete:error]; } -- (void)onCommissioningComplete:(NSError * _Nullable)error +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError * _Nullable)error { [self.delegate onCommissioningComplete:error]; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h index 3b549c45c674cf..d955c653ada3b1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h @@ -26,6 +26,8 @@ typedef NS_ENUM(NSInteger, MTRCommissioningStatus) { MTRCommissioningStatusDiscoveringMoreDevices = 3 } MTR_NEWLY_AVAILABLE; +@class MTRDeviceController; + /** * The protocol definition for the MTRDeviceControllerDelegate. * @@ -37,18 +39,18 @@ MTR_NEWLY_AVAILABLE /** * Notify the delegate when commissioning status gets updated. */ -- (void)onStatusUpdate:(MTRCommissioningStatus)status; +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status; /** * Notify the delegate when a commissioning session is established or the * establishment has errored out. */ -- (void)onCommissioningSessionEstablishmentDone:(NSError * _Nullable)error; +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error; /** * Notify the delegate when commissioning is completed. */ -- (void)onCommissioningComplete:(NSError * _Nullable)error; +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError * _Nullable)error; @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.h index 299e85b6a3b5ae..464f0c91093d8d 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.h @@ -22,13 +22,14 @@ NS_ASSUME_NONNULL_BEGIN -class MTRDeviceControllerDelegateBridge : public chip::Controller::DevicePairingDelegate -{ +@class MTRDeviceController; + +class MTRDeviceControllerDelegateBridge : public chip::Controller::DevicePairingDelegate { public: MTRDeviceControllerDelegateBridge(); ~MTRDeviceControllerDelegateBridge(); - void setDelegate(id delegate, dispatch_queue_t queue); + void setDelegate(MTRDeviceController * controller, id delegate, dispatch_queue_t queue); void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override; @@ -39,6 +40,7 @@ class MTRDeviceControllerDelegateBridge : public chip::Controller::DevicePairing void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR error) override; private: + MTRDeviceController * __weak mController; _Nullable id mDelegate; _Nullable dispatch_queue_t mQueue; diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm index f84a99e4d2664b..3ac0883dc61815 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm @@ -16,6 +16,7 @@ */ #import "MTRDeviceControllerDelegateBridge.h" +#import "MTRDeviceController.h" #import "MTRError_Internal.h" MTRDeviceControllerDelegateBridge::MTRDeviceControllerDelegateBridge(void) @@ -25,12 +26,15 @@ MTRDeviceControllerDelegateBridge::~MTRDeviceControllerDelegateBridge(void) {} -void MTRDeviceControllerDelegateBridge::setDelegate(id delegate, dispatch_queue_t queue) +void MTRDeviceControllerDelegateBridge::setDelegate( + MTRDeviceController * controller, id delegate, dispatch_queue_t queue) { if (delegate && queue) { + mController = controller; mDelegate = delegate; mQueue = queue; } else { + mController = nil; mDelegate = nil; mQueue = nil; } @@ -58,11 +62,12 @@ NSLog(@"DeviceControllerDelegate status updated: %d", status); id strongDelegate = mDelegate; - if ([strongDelegate respondsToSelector:@selector(onStatusUpdate:)]) { - if (strongDelegate && mQueue) { + MTRDeviceController * strongController = mController; + if (strongDelegate && mQueue && strongController) { + if ([strongDelegate respondsToSelector:@selector(controller:statusUpdate:)]) { MTRCommissioningStatus commissioningStatus = MapStatus(status); dispatch_async(mQueue, ^{ - [strongDelegate onStatusUpdate:commissioningStatus]; + [strongDelegate controller:strongController statusUpdate:commissioningStatus]; }); } } @@ -73,11 +78,12 @@ NSLog(@"DeviceControllerDelegate Pairing complete. Status %s", chip::ErrorStr(error)); id strongDelegate = mDelegate; - if ([strongDelegate respondsToSelector:@selector(onCommissioningSessionEstablishmentDone:)]) { - if (strongDelegate && mQueue) { + MTRDeviceController * strongController = mController; + if (strongDelegate && mQueue && strongController) { + if ([strongDelegate respondsToSelector:@selector(controller:commissioningSessionEstablishmentDone:)]) { dispatch_async(mQueue, ^{ NSError * nsError = [MTRError errorForCHIPErrorCode:error]; - [strongDelegate onCommissioningSessionEstablishmentDone:nsError]; + [strongDelegate controller:strongController commissioningSessionEstablishmentDone:nsError]; }); } } @@ -95,11 +101,12 @@ NSLog(@"DeviceControllerDelegate Commissioning complete. NodeId %llu Status %s", nodeId, chip::ErrorStr(error)); id strongDelegate = mDelegate; - if ([strongDelegate respondsToSelector:@selector(onCommissioningComplete:)]) { - if (strongDelegate && mQueue) { + MTRDeviceController * strongController = mController; + if (strongDelegate && mQueue && strongController) { + if ([strongDelegate respondsToSelector:@selector(controller:commissioningComplete:)]) { dispatch_async(mQueue, ^{ NSError * nsError = [MTRError errorForCHIPErrorCode:error]; - [strongDelegate onCommissioningComplete:nsError]; + [strongDelegate controller:strongController commissioningComplete:nsError]; }); } } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index db8e1050622e19..1b5e64dd7728dd 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -94,7 +94,7 @@ - (id)initWithExpectation:(XCTestExpectation *)expectation return self; } -- (void)onCommissioningSessionEstablishmentDone:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError *)error { XCTAssertEqual(error.code, 0); @@ -104,10 +104,10 @@ - (void)onCommissioningSessionEstablishmentDone:(NSError *)error error:&commissionError]; XCTAssertNil(commissionError); - // Keep waiting for onCommissioningComplete + // Keep waiting for controller:MTRXPCListenerSampleTests.mcommissioningComplete } -- (void)onCommissioningComplete:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error { XCTAssertEqual(error.code, 0); [_expectation fulfill]; diff --git a/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m b/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m index ab6395921e54eb..3d476bebc7e4f9 100644 --- a/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m +++ b/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m @@ -419,7 +419,7 @@ - (id)initWithExpectation:(XCTestExpectation *)expectation return self; } -- (void)onCommissioningSessionEstablishmentDone:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError *)error { XCTAssertEqual(error.code, 0); NSError * commissionError = nil; @@ -428,10 +428,10 @@ - (void)onCommissioningSessionEstablishmentDone:(NSError *)error error:&commissionError]; XCTAssertNil(commissionError); - // Keep waiting for onCommissioningComplete + // Keep waiting for controller:commissioningComplete } -- (void)onCommissioningComplete:(NSError *)error +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error { XCTAssertEqual(error.code, 0); [_expectation fulfill];