Skip to content

Commit

Permalink
[Darwin] Fix Matter framework delegates to pass the delegating object…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
jtung-apple authored and bzbarsky-apple committed Nov 17, 2022
1 parent c6272c6 commit 8c3b9bc
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
27 changes: 20 additions & 7 deletions src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

Expand All @@ -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
Expand All @@ -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

Expand Down
31 changes: 23 additions & 8 deletions src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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];
}
}
}
});
Expand Down
19 changes: 9 additions & 10 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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];
}
Expand Down
8 changes: 5 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ typedef NS_ENUM(NSInteger, MTRCommissioningStatus) {
MTRCommissioningStatusDiscoveringMoreDevices = 3
} MTR_NEWLY_AVAILABLE;

@class MTRDeviceController;

/**
* The protocol definition for the MTRDeviceControllerDelegate.
*
Expand All @@ -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

Expand Down
8 changes: 5 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -39,6 +40,7 @@ class MTRDeviceControllerDelegateBridge : public chip::Controller::DevicePairing
void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR error) override;

private:
MTRDeviceController * _Nullable mController;
_Nullable id<MTRDeviceControllerDelegate> mDelegate;
_Nullable dispatch_queue_t mQueue;

Expand Down
18 changes: 11 additions & 7 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

#import "MTRDeviceControllerDelegateBridge.h"
#import "MTRDeviceController.h"
#import "MTRError_Internal.h"

MTRDeviceControllerDelegateBridge::MTRDeviceControllerDelegateBridge(void)
Expand All @@ -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;
}
Expand Down Expand Up @@ -58,11 +62,11 @@
NSLog(@"DeviceControllerDelegate status updated: %d", status);

id<MTRDeviceControllerDelegate> strongDelegate = mDelegate;
if ([strongDelegate respondsToSelector:@selector(onStatusUpdate:)]) {
if ([strongDelegate respondsToSelector:@selector(controller:statusUpdate:)]) {
if (strongDelegate && mQueue) {
MTRCommissioningStatus commissioningStatus = MapStatus(status);
dispatch_async(mQueue, ^{
[strongDelegate onStatusUpdate:commissioningStatus];
[strongDelegate controller:mController statusUpdate:commissioningStatus];
});
}
}
Expand All @@ -73,11 +77,11 @@
NSLog(@"DeviceControllerDelegate Pairing complete. Status %s", chip::ErrorStr(error));

id<MTRDeviceControllerDelegate> strongDelegate = mDelegate;
if ([strongDelegate respondsToSelector:@selector(onCommissioningSessionEstablishmentDone:)]) {
if ([strongDelegate respondsToSelector:@selector(controller:commissioningSessionEstablishmentDone:)]) {
if (strongDelegate && mQueue) {
dispatch_async(mQueue, ^{
NSError * nsError = [MTRError errorForCHIPErrorCode:error];
[strongDelegate onCommissioningSessionEstablishmentDone:nsError];
[strongDelegate controller:mController commissioningSessionEstablishmentDone:nsError];
});
}
}
Expand All @@ -95,11 +99,11 @@
NSLog(@"DeviceControllerDelegate Commissioning complete. NodeId %llu Status %s", nodeId, chip::ErrorStr(error));

id<MTRDeviceControllerDelegate> strongDelegate = mDelegate;
if ([strongDelegate respondsToSelector:@selector(onCommissioningComplete:)]) {
if ([strongDelegate respondsToSelector:@selector(controller:commissioningComplete:)]) {
if (strongDelegate && mQueue) {
dispatch_async(mQueue, ^{
NSError * nsError = [MTRError errorForCHIPErrorCode:error];
[strongDelegate onCommissioningComplete:nsError];
[strongDelegate controller:mController commissioningComplete:nsError];
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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];
Expand Down
Loading

0 comments on commit 8c3b9bc

Please sign in to comment.