diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index cece722a25de5d..a2d6a22a475126 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -74,15 +74,6 @@ class MTRDataValueDictionaryCallbackBridge; -@interface MTRBaseDevice () - -@property (nonatomic, readonly, assign, nullable) chip::DeviceProxy * cppPASEDevice; -@property (nonatomic, readwrite) NSMutableDictionary * reportHandlerBridges; - -- (chip::NodeId)deviceID; -- (BOOL)isPASEDevice; -@end - @interface MTRReadClientContainer : NSObject @property (nonatomic, readwrite) app::ReadClient * readClientPtr; @property (nonatomic, readwrite) app::AttributePathParams * pathParams; @@ -223,19 +214,8 @@ @implementation MTRBaseDevice - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDeviceController *)controller { if (self = [super init]) { - chip::Optional session = device->GetSecureSession(); - if (!session.HasValue()) { - MTR_LOG_ERROR("Failing to initialize MTRBaseDevice: no secure session"); - return nil; - } - - if (!session.Value()->AsSecureSession()->IsPASESession()) { - MTR_LOG_ERROR("Failing to initialize MTRBaseDevice: not a PASE session"); - return nil; - } - - _cppPASEDevice = device; - _nodeID = kUndefinedNodeId; + _isPASEDevice = YES; + _nodeID = device->GetDeviceId(); _deviceController = controller; } return self; @@ -244,7 +224,7 @@ - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDe - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller { if (self = [super init]) { - _cppPASEDevice = nil; + _isPASEDevice = NO; _nodeID = [nodeID unsignedLongLongValue]; _deviceController = controller; } @@ -258,32 +238,13 @@ + (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControl return [controller baseDeviceForNodeID:nodeID]; } -- (chip::DeviceProxy * _Nullable)paseDevice -{ - return _cppPASEDevice; -} - -- (chip::NodeId)deviceID -{ - if (_cppPASEDevice != nullptr) { - return _cppPASEDevice->GetDeviceId(); - } - - return self.nodeID; -} - -- (BOOL)isPASEDevice -{ - return _cppPASEDevice != nullptr; -} - - (void)invalidateCASESession { if (self.isPASEDevice) { return; } - [self.deviceController invalidateCASESessionForNode:self.deviceID]; + [self.deviceController invalidateCASESessionForNode:self.nodeID]; } namespace { @@ -1195,7 +1156,7 @@ - (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID }; MTRReadClientContainer * container = [[MTRReadClientContainer alloc] init]; - container.deviceID = [self deviceID]; + container.deviceID = self.nodeID; container.pathParams = Platform::New(); if (endpointID) { container.pathParams->mEndpointId = static_cast([endpointID unsignedShortValue]); @@ -1261,7 +1222,7 @@ - (void)deregisterReportHandlersWithQueue:(dispatch_queue_t)queue completion:(di { // This method must only be used for MTRDeviceOverXPC. However, for unit testing purpose, the method purges all read clients. MTR_LOG_DEBUG("Unexpected call to deregister report handlers"); - PurgeReadClientContainers([self deviceID], queue, completion); + PurgeReadClientContainers(self.nodeID, queue, completion); } namespace { @@ -1407,7 +1368,7 @@ - (void)openCommissioningWindowWithSetupPasscode:(NSNumber *)setupPasscode - (void)failSubscribers:(dispatch_queue_t)queue completion:(void (^)(void))completion { MTR_LOG_DEBUG("Causing failure in subscribers on purpose"); - CauseReadClientFailure([self deviceID], queue, completion); + CauseReadClientFailure(self.nodeID, queue, completion); } #endif diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h index 3dc8cb9bf875bf..c6fb24742f648b 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h +++ b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h @@ -29,12 +29,6 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDeviceController *)controller; -/** - * Only returns non-nil if the device is using a PASE session. Otherwise, the - * deviceController + nodeId should be used to get a CASE session. - */ -- (chip::DeviceProxy * _Nullable)paseDevice; - /** * Invalidate the CASE session, so an attempt to getConnectedDevice for this * device id will have to create a new CASE session. Ideally this API will go @@ -42,14 +36,20 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)invalidateCASESession; +/** + * Whether this device represents a PASE session or not. + */ +@property (nonatomic, assign, readonly) BOOL isPASEDevice; + /** * Controller that that this MTRDevice was gotten from. */ @property (nonatomic, strong, readonly) MTRDeviceController * deviceController; /** - * Node id for this MTRDevice. Only set to a usable value if this device - * represents a CASE session. + * Node id for this MTRDevice. If this device represents a CASE session, this + * is set to the node ID of the target node. If this device represents a PASE + * session, this is set to the device id of the PASE device. */ @property (nonatomic, assign, readonly) chip::NodeId nodeID; @@ -60,10 +60,10 @@ NS_ASSUME_NONNULL_BEGIN + (instancetype)new NS_UNAVAILABLE; /** - * Initialize the device object with the given node id and controller. This - * will always succeed, even if there is no such node id on the controller's - * fabric, but attempts to actually use the MTRBaseDevice will fail - * (asynchronously) in that case. + * Initialize the device object as a CASE device with the given node id and + * controller. This will always succeed, even if there is no such node id on + * the controller's fabric, but attempts to actually use the MTRBaseDevice will + * fail (asynchronously) in that case. */ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller; diff --git a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h index e0e53e3c9b21d9..3ea7d4c0fb8260 100644 --- a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h +++ b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h @@ -96,11 +96,10 @@ template class MTRCallbackBridge { } /** - * Run the given MTRActionBlock on the Matter thread (possibly - * synchronously, if the MTRBaseDevice represents a PASE connection), after - * getting a secure session corresponding to the given MTRBaseDevice. On - * success, convert the success value to whatever type it needs to be to - * call the callback type we're templated over. + * Run the given MTRActionBlock on the Matter thread after getting a secure + * session corresponding to the given MTRBaseDevice. On success, convert + * the success value to whatever type it needs to be to call the callback + * type we're templated over. */ MTRCallbackBridge(dispatch_queue_t queue, MTRBaseDevice * device, ResponseHandler handler, MTRActionBlock action, T OnSuccessFn, bool keepAlive) @@ -111,23 +110,27 @@ template class MTRCallbackBridge { , mSuccess(OnSuccessFn, this) , mFailure(OnFailureFn, this) { - auto * paseDevice = [device paseDevice]; - if (paseDevice != nullptr) { - ActionWithDevice(paseDevice); + if (device.isPASEDevice) { + ActionWithPASEDevice(device); } else { ActionWithNodeID(device.nodeID, device.deviceController); } }; - void ActionWithDevice(chip::DeviceProxy * device) + void ActionWithPASEDevice(MTRBaseDevice * device) { LogRequestStart(); - // Keep doing dispatch_sync here for now, because we don't want device - // to become stale. - dispatch_sync(chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue(), ^{ - MaybeDoAction(device->GetExchangeManager(), device->GetSecureSession(), nil); - }); + BOOL ok = [device.deviceController + getSessionForCommissioneeDevice:device.nodeID + completion:^(chip::Messaging::ExchangeManager * exchangeManager, + const chip::Optional & session, NSError * error) { + MaybeDoAction(exchangeManager, session, error); + }]; + + if (ok == NO) { + OnFailureFn(this, CHIP_ERROR_INCORRECT_STATE); + } } void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController * controller) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 43c2d0e7084fdc..1a56818233e2be 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -489,18 +489,19 @@ - (MTRBaseDevice *)deviceBeingCommissionedWithNodeID:(NSNumber *)nodeID error:(N { VerifyOrReturnValue([self checkIsRunning:error], nil); - __block chip::CommissioneeDeviceProxy * deviceProxy; - + __block MTRBaseDevice * device; __block BOOL success = NO; dispatch_sync(_chipWorkQueue, ^{ VerifyOrReturn([self checkIsRunning:error]); + chip::CommissioneeDeviceProxy * deviceProxy; auto errorCode = self->_cppCommissioner->GetDeviceBeingCommissioned([nodeID unsignedLongLongValue], &deviceProxy); success = ![MTRDeviceController checkForError:errorCode logMsg:kErrorStopPairing error:error]; + device = [[MTRBaseDevice alloc] initWithPASEDevice:deviceProxy controller:self]; }); VerifyOrReturnValue(success, nil); - return [[MTRBaseDevice alloc] initWithPASEDevice:deviceProxy controller:self]; + return device; } - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID @@ -704,6 +705,38 @@ - (BOOL)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConn return YES; } +- (BOOL)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion +{ + if (![self checkIsRunning]) { + return NO; + } + + dispatch_async(_chipWorkQueue, ^{ + NSError * error; + if (![self checkIsRunning:&error]) { + completion(nullptr, chip::NullOptional, error); + return; + } + + chip::CommissioneeDeviceProxy * deviceProxy; + CHIP_ERROR err = self->_cppCommissioner->GetDeviceBeingCommissioned(deviceID, &deviceProxy); + if (err != CHIP_NO_ERROR) { + completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err]); + return; + } + + chip::Optional session = deviceProxy->GetSecureSession(); + if (!session.HasValue() || !session.Value()->AsSecureSession()->IsPASESession()) { + completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + return; + } + + completion(deviceProxy->GetExchangeManager(), session, nil); + }); + + return YES; +} + - (void)asyncDispatchToMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block errorHandler:(void (^)(NSError *))errorHandler { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 6b69b4a579077e..602713793bd70f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -120,6 +120,20 @@ NS_ASSUME_NONNULL_BEGIN */ - (BOOL)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion; +/** + * Get a session for the commissionee device with the given device id. This may + * be called on any queue (including the Matter event queue) and will always + * call the provided connection callback on the Matter queue, asynchronously. + * Consumers must be prepared to run on the Matter queue (an in particular must + * not use any APIs that will try to do sync dispatch to the Matter queue). + * + * If the controller is not running when this function is called, will return NO + * and never invoke the completion. If the controller is not running when the + * async dispatch on the Matter queue would happen, an error will be dispatched + * to the completion handler. + */ +- (BOOL)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion; + /** * Invalidate the CASE session for the given node ID. This is a temporary thing * just to support MTRBaseDevice's invalidateCASESession. Must not be called on