[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 <[email protected]>
project-chip · Nov 18, 2022 · 3198f01 · 3198f01
1 parent 6978de7
commit 3198f01
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 57 deletions.
diff --git a/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h b/examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.h
@@ -20,13 +20,15 @@
 
 #import <Matter/Matter.h>
 
+@class MTRDeviceController;
+
 @interface CHIPToolDeviceControllerDelegate : NSObject <MTRDeviceControllerDelegate>
 @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
@@ -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
@@ -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
@@ -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
@@ -30,7 +30,9 @@
         mResult = attestationResult;
 
         id<MTRDeviceAttestationDelegate> 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
@@ -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<MTRDeviceControllerDelegate>)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<MTRDevicePairingDelegate>)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<MTRPairingStatus>(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
@@ -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
@@ -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<MTRDeviceControllerDelegate> delegate, dispatch_queue_t queue);
+    void setDelegate(MTRDeviceController * controller, id<MTRDeviceControllerDelegate> 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<MTRDeviceControllerDelegate> mDelegate;
     _Nullable dispatch_queue_t mQueue;
 

diff --git 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<MTRDeviceControllerDelegate> delegate, dispatch_queue_t queue)
+void MTRDeviceControllerDelegateBridge::setDelegate(
+    MTRDeviceController * controller, id<MTRDeviceControllerDelegate> 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<MTRDeviceControllerDelegate> 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<MTRDeviceControllerDelegate> 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<MTRDeviceControllerDelegate> 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
@@ -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];