From 6f7f1703045e8665bf7591a28f2f81d3e01ae29f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 7 Oct 2024 18:13:38 -0400 Subject: [PATCH 1/2] Make MTRBaseDevice be explicit about when it's assuming a concrete controller. This allows us to move several selectors from MTRDeviceController_Internal to MTRDeviceController_Concrete, and get rid of the never-actually-set _cppCommissioner ivar on MTRDeviceController. --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 304 +++++++++++------- .../Framework/CHIP/MTRCallbackBridgeBase.h | 5 + .../Framework/CHIP/MTRDeviceController.mm | 59 +--- .../CHIP/MTRDeviceController_Concrete.h | 38 +++ .../CHIP/MTRDeviceController_Concrete.mm | 4 +- .../CHIP/MTRDeviceController_Internal.h | 37 --- 6 files changed, 231 insertions(+), 216 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index f00c57a61be18c..c11e59eb1531c8 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -232,6 +232,11 @@ - (void)onDone @end +@interface MTRBaseDevice () +// Will return nil if our controller is not in fact a concrete controller. +@property (nullable, nonatomic, strong, readonly) MTRDeviceController_Concrete * concreteController; +@end + @implementation MTRBaseDevice - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDeviceController *)controller @@ -261,9 +266,23 @@ + (MTRBaseDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceCont return [controller baseDeviceForNodeID:nodeID]; } +- (nullable MTRDeviceController_Concrete *)concreteController +{ + auto * controller = self.deviceController; + if ([controller isKindOfClass:MTRDeviceController_Concrete.class]) { + return static_cast(controller); + } + + return nil; +} + - (MTRTransportType)sessionTransportType { - return [self.deviceController sessionTransportTypeForDevice:self]; + auto * concreteController = self.concreteController; + if (concreteController == nil) { + return MTRTransportTypeUndefined; + } + return [concreteController sessionTransportTypeForDevice:self]; } - (void)invalidateCASESession @@ -272,7 +291,13 @@ - (void)invalidateCASESession return; } - [self.deviceController invalidateCASESessionForNode:self.nodeID]; + auto * concreteController = self.concreteController; + if (concreteController == nil) { + // Nothing we can do here. + return; + } + + [concreteController invalidateCASESessionForNode:@(self.nodeID)]; } namespace { @@ -311,121 +336,130 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue return; } + auto * concreteController = self.concreteController; + if (concreteController == nil) { + // No subscriptions (or really any MTRBaseDevice use) with XPC controllers. + dispatch_async(queue, ^{ + errorHandler([MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + }); + return; + } + // Copy params before going async. params = [params copy]; - [self.deviceController getSessionForNode:self.nodeID - completion:^(ExchangeManager * _Nullable exchangeManager, const Optional & session, - NSError * _Nullable error, NSNumber * _Nullable retryDelay) { - if (error != nil) { - dispatch_async(queue, ^{ - errorHandler(error); - }); - return; - } - - // Wildcard endpoint, cluster, attribute, event. - auto attributePath = std::make_unique(); - auto eventPath = std::make_unique(); - eventPath->mIsUrgentEvent = params.reportEventsUrgently; - ReadPrepareParams readParams(session.Value()); - [params toReadPrepareParams:readParams]; - readParams.mpAttributePathParamsList = attributePath.get(); - readParams.mAttributePathParamsListSize = 1; - readParams.mpEventPathParamsList = eventPath.get(); - readParams.mEventPathParamsListSize = 1; - - std::unique_ptr clusterStateCache; - ReadClient::Callback * callbackForReadClient = nullptr; - OnDoneHandler onDoneHandler = nil; - - if (clusterStateCacheContainer) { - __weak MTRClusterStateCacheContainer * weakPtr = clusterStateCacheContainer; - onDoneHandler = ^{ - // This, like all manipulation of cppClusterStateCache, needs to run on the Matter - // queue. - MTRClusterStateCacheContainer * container = weakPtr; - if (container) { - container.cppClusterStateCache = nullptr; - container.baseDevice = nil; - } - }; - } - - auto callback = std::make_unique( - ^(NSArray * value) { - dispatch_async(queue, ^{ - if (attributeReportHandler != nil) { - attributeReportHandler(value); - } - }); - }, - ^(NSArray * value) { - dispatch_async(queue, ^{ - if (eventReportHandler != nil) { - eventReportHandler(value); - } - }); - }, - ^(NSError * error) { - dispatch_async(queue, ^{ - errorHandler(error); - }); - }, - ^(NSError * error, NSNumber * resubscriptionDelay) { - dispatch_async(queue, ^{ - if (resubscriptionScheduled != nil) { - resubscriptionScheduled(error, resubscriptionDelay); - } - }); - }, - ^(void) { - dispatch_async(queue, ^{ - if (subscriptionEstablished != nil) { - subscriptionEstablished(); - } - }); - }, - onDoneHandler); - - if (clusterStateCacheContainer) { - clusterStateCache = std::make_unique(*callback.get()); - callbackForReadClient = &clusterStateCache->GetBufferedCallback(); - } else { - callbackForReadClient = &callback->GetBufferedCallback(); - } - - auto readClient = std::make_unique(InteractionModelEngine::GetInstance(), - exchangeManager, *callbackForReadClient, ReadClient::InteractionType::Subscribe); - - CHIP_ERROR err; - if (!params.resubscribeAutomatically) { - err = readClient->SendRequest(readParams); - } else { - // SendAutoResubscribeRequest cleans up the params, even on failure. - attributePath.release(); - eventPath.release(); - err = readClient->SendAutoResubscribeRequest(std::move(readParams)); - } - - if (err != CHIP_NO_ERROR) { - dispatch_async(queue, ^{ - errorHandler([MTRError errorForCHIPErrorCode:err]); - }); - - return; - } - - if (clusterStateCacheContainer) { - clusterStateCacheContainer.cppClusterStateCache = clusterStateCache.get(); - // ClusterStateCache will be deleted when OnDone is called. - callback->AdoptClusterStateCache(std::move(clusterStateCache)); - clusterStateCacheContainer.baseDevice = self; - } - // Callback and ReadClient will be deleted when OnDone is called. - callback->AdoptReadClient(std::move(readClient)); - callback.release(); - }]; + [concreteController getSessionForNode:self.nodeID + completion:^(ExchangeManager * _Nullable exchangeManager, const Optional & session, + NSError * _Nullable error, NSNumber * _Nullable retryDelay) { + if (error != nil) { + dispatch_async(queue, ^{ + errorHandler(error); + }); + return; + } + + // Wildcard endpoint, cluster, attribute, event. + auto attributePath = std::make_unique(); + auto eventPath = std::make_unique(); + eventPath->mIsUrgentEvent = params.reportEventsUrgently; + ReadPrepareParams readParams(session.Value()); + [params toReadPrepareParams:readParams]; + readParams.mpAttributePathParamsList = attributePath.get(); + readParams.mAttributePathParamsListSize = 1; + readParams.mpEventPathParamsList = eventPath.get(); + readParams.mEventPathParamsListSize = 1; + + std::unique_ptr clusterStateCache; + ReadClient::Callback * callbackForReadClient = nullptr; + OnDoneHandler onDoneHandler = nil; + + if (clusterStateCacheContainer) { + __weak MTRClusterStateCacheContainer * weakPtr = clusterStateCacheContainer; + onDoneHandler = ^{ + // This, like all manipulation of cppClusterStateCache, needs to run on the Matter + // queue. + MTRClusterStateCacheContainer * container = weakPtr; + if (container) { + container.cppClusterStateCache = nullptr; + container.baseDevice = nil; + } + }; + } + + auto callback = std::make_unique( + ^(NSArray * value) { + dispatch_async(queue, ^{ + if (attributeReportHandler != nil) { + attributeReportHandler(value); + } + }); + }, + ^(NSArray * value) { + dispatch_async(queue, ^{ + if (eventReportHandler != nil) { + eventReportHandler(value); + } + }); + }, + ^(NSError * error) { + dispatch_async(queue, ^{ + errorHandler(error); + }); + }, + ^(NSError * error, NSNumber * resubscriptionDelay) { + dispatch_async(queue, ^{ + if (resubscriptionScheduled != nil) { + resubscriptionScheduled(error, resubscriptionDelay); + } + }); + }, + ^(void) { + dispatch_async(queue, ^{ + if (subscriptionEstablished != nil) { + subscriptionEstablished(); + } + }); + }, + onDoneHandler); + + if (clusterStateCacheContainer) { + clusterStateCache = std::make_unique(*callback.get()); + callbackForReadClient = &clusterStateCache->GetBufferedCallback(); + } else { + callbackForReadClient = &callback->GetBufferedCallback(); + } + + auto readClient = std::make_unique(InteractionModelEngine::GetInstance(), + exchangeManager, *callbackForReadClient, ReadClient::InteractionType::Subscribe); + + CHIP_ERROR err; + if (!params.resubscribeAutomatically) { + err = readClient->SendRequest(readParams); + } else { + // SendAutoResubscribeRequest cleans up the params, even on failure. + attributePath.release(); + eventPath.release(); + err = readClient->SendAutoResubscribeRequest(std::move(readParams)); + } + + if (err != CHIP_NO_ERROR) { + dispatch_async(queue, ^{ + errorHandler([MTRError errorForCHIPErrorCode:err]); + }); + + return; + } + + if (clusterStateCacheContainer) { + clusterStateCacheContainer.cppClusterStateCache = clusterStateCache.get(); + // ClusterStateCache will be deleted when OnDone is called. + callback->AdoptClusterStateCache(std::move(clusterStateCache)); + clusterStateCacheContainer.baseDevice = self; + } + // Callback and ReadClient will be deleted when OnDone is called. + callback->AdoptReadClient(std::move(readClient)); + callback.release(); + }]; } static NSDictionary * _MakeDataValueDictionary(NSString * type, id _Nullable value, NSNumber * _Nullable dataVersion) @@ -1616,6 +1650,15 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl return; } + auto * concreteController = self.concreteController; + if (concreteController == nil) { + // No subscriptions (or really any MTRBaseDevice use) with XPC controllers. + dispatch_async(queue, ^{ + reportHandler(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + }); + return; + } + // Copy params before going async. NSArray * attributes = nil; if (attributePaths != nil) { @@ -1629,7 +1672,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl params = (params == nil) ? nil : [params copy]; - [self.deviceController + [concreteController getSessionForNode:self.nodeID completion:^(ExchangeManager * _Nullable exchangeManager, const Optional & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) { @@ -1890,6 +1933,16 @@ - (void)_openCommissioningWindowWithSetupPasscode:(nullable NSNumber *)setupPass return; } + auto * concreteController = self.concreteController; + if (concreteController == nil) { + MTR_LOG_ERROR("Can't open a commissioning window via MTRBaseDevice against an XPC Controller"); + dispatch_async(queue, ^{ + MATTER_LOG_METRIC_END(kMetricOpenPairingWindow, CHIP_ERROR_INCORRECT_STATE); + completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + }); + return; + } + unsigned long long durationVal = [duration unsignedLongLongValue]; if (!CanCastTo(durationVal)) { MTR_LOG_ERROR("Error: Duration %llu is too large.", durationVal); @@ -1925,7 +1978,7 @@ - (void)_openCommissioningWindowWithSetupPasscode:(nullable NSNumber *)setupPass passcode.Emplace(static_cast(passcodeVal)); } - [self.deviceController + [concreteController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) { auto resultCallback = ^(CHIP_ERROR status, const SetupPayload & payload) { if (status != CHIP_NO_ERROR) { @@ -2175,11 +2228,20 @@ - (void)downloadLogOfType:(MTRDiagnosticLogType)type queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion { - [_deviceController downloadLogFromNodeWithID:@(_nodeID) - type:type - timeout:timeout - queue:queue - completion:completion]; + auto * concreteController = self.concreteController; + if (concreteController == nil) { + MTR_LOG_ERROR("Can't download logs via MTRBaseDevice against an XPC Controller"); + dispatch_async(queue, ^{ + completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + }); + return; + } + + [concreteController downloadLogFromNodeWithID:@(_nodeID) + type:type + timeout:timeout + queue:queue + completion:completion]; } - (NSString *)description diff --git a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h index 6631e2d132bb54..d9c98157c2fe1d 100644 --- a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h +++ b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h @@ -81,6 +81,9 @@ class MTRCallbackBridgeBase { { LogRequestStart(); + // TODO: Figure out whether we can usefully get an MTRDeviceController_Concrete in here, so + // we can move getSessionForCommissioneeDevice off of MTRDeviceController_Internal. Ideally + // without bloating this inline method too much. [device.deviceController getSessionForCommissioneeDevice:device.nodeID completion:^(chip::Messaging::ExchangeManager * exchangeManager, const chip::Optional & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) { @@ -92,6 +95,8 @@ class MTRCallbackBridgeBase { { LogRequestStart(); + // TODO: Figure out whether we can usefully get an MTRDeviceController_Concrete in here, so + // we can move getSessionForNode off of MTRDeviceController_Internal. [controller getSessionForNode:nodeID completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager, const chip::Optional & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index fb13db9b006bc1..013e892873d25f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -119,7 +119,6 @@ - (instancetype)initWithDelegate:(id)delegate queue @end @implementation MTRDeviceController { - chip::Controller::DeviceCommissioner * _cppCommissioner; chip::Credentials::PartialDACVerifier * _partialDACVerifier; chip::Credentials::DefaultDACVerifier * _defaultDACVerifier; MTRDeviceControllerDelegateBridge * _deviceControllerDelegateBridge; @@ -206,7 +205,8 @@ - (NSString *)description - (BOOL)isRunning { - return _cppCommissioner != nullptr; + MTR_ABSTRACT_METHOD(); + return NO; } #pragma mark - Suspend/resume support @@ -547,37 +547,9 @@ - (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRIn completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil); } -- (MTRTransportType)sessionTransportTypeForDevice:(MTRBaseDevice *)device -{ - VerifyOrReturnValue([self checkIsRunning], MTRTransportTypeUndefined); - - __block MTRTransportType result = MTRTransportTypeUndefined; - dispatch_sync(_chipWorkQueue, ^{ - VerifyOrReturn([self checkIsRunning]); - - if (device.isPASEDevice) { - chip::CommissioneeDeviceProxy * deviceProxy; - VerifyOrReturn(CHIP_NO_ERROR == self->_cppCommissioner->GetDeviceBeingCommissioned(device.nodeID, &deviceProxy)); - result = MTRMakeTransportType(deviceProxy->GetDeviceTransportType()); - } else { - auto scopedNodeID = self->_cppCommissioner->GetPeerScopedId(device.nodeID); - auto sessionHandle = self->_cppCommissioner->SessionMgr()->FindSecureSessionForNode(scopedNodeID); - VerifyOrReturn(sessionHandle.HasValue()); - result = MTRMakeTransportType(sessionHandle.Value()->AsSecureSession()->GetPeerAddress().GetTransportType()); - } - }); - return result; -} - -- (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block - errorHandler:(nullable MTRDeviceErrorHandler)errorHandler -{ - MTR_ABSTRACT_METHOD(); - errorHandler([MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); -} - - (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler { + // TODO: Figure out how to get callsites to have an MTRDeviceController_Concrete. MTR_ABSTRACT_METHOD(); errorHandler([MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); } @@ -627,31 +599,6 @@ - (nullable NSNumber *)compressedFabricID return storedValue.has_value() ? @(storedValue.value()) : nil; } -- (void)invalidateCASESessionForNode:(chip::NodeId)nodeID; -{ - auto block = ^{ - auto sessionMgr = self->_cppCommissioner->SessionMgr(); - VerifyOrDie(sessionMgr != nullptr); - - sessionMgr->MarkSessionsAsDefunct( - self->_cppCommissioner->GetPeerScopedId(nodeID), chip::MakeOptional(chip::Transport::SecureSession::Type::kCASE)); - }; - - [self syncRunOnWorkQueue:block error:nil]; -} - -- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID - type:(MTRDiagnosticLogType)type - timeout:(NSTimeInterval)timeout - queue:(dispatch_queue_t)queue - completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion -{ - MTR_ABSTRACT_METHOD(); - dispatch_async(queue, ^{ - completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); - }); -} - #ifdef DEBUG + (void)forceLocalhostAdvertisingOnly { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h index 5df5f501b13b8d..0cfb25f4944fc9 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h @@ -18,6 +18,7 @@ #import #import +#import #import #import #import @@ -149,6 +150,43 @@ NS_ASSUME_NONNULL_BEGIN */ - (nullable NSNumber *)neededReadPrivilegeForClusterID:(NSNumber *)clusterID attributeID:(NSNumber *)attributeID; +/** + * Try to asynchronously dispatch the given block on the Matter queue. If the + * controller is not running either at call time or when the block would be + * about to run, the provided error handler will be called with an error. Note + * that this means the error handler might be called on an arbitrary queue, and + * might be called before this function returns or after it returns. + * + * The DeviceCommissioner pointer passed to the callback should only be used + * synchronously during the callback invocation. + * + * If the error handler is nil, failure to run the block will be silent. + */ +- (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block + errorHandler:(nullable MTRDeviceErrorHandler)errorHandler; + +/** + * Returns the transport used by the current session with the given device, + * or `MTRTransportTypeUndefined` if no session is currently active. + */ +- (MTRTransportType)sessionTransportTypeForDevice:(MTRBaseDevice *)device; + +/** + * 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 + * the Matter event queue. + */ +- (void)invalidateCASESessionForNode:(NSNumber *)nodeID; + +/** + * Download log of the desired type from the device. + */ +- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID + type:(MTRDiagnosticLogType)type + timeout:(NSTimeInterval)timeout + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion; + @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index 47c373ac884188..42ee83d5234ef7 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -1586,14 +1586,14 @@ - (CHIP_ERROR)isRunningOnFabric:(chip::FabricTable *)fabricTable return CHIP_NO_ERROR; } -- (void)invalidateCASESessionForNode:(chip::NodeId)nodeID; +- (void)invalidateCASESessionForNode:(NSNumber *)nodeID; { auto block = ^{ auto sessionMgr = self->_cppCommissioner->SessionMgr(); VerifyOrDie(sessionMgr != nullptr); sessionMgr->MarkSessionsAsDefunct( - self->_cppCommissioner->GetPeerScopedId(nodeID), chip::MakeOptional(chip::Transport::SecureSession::Type::kCASE)); + self->_cppCommissioner->GetPeerScopedId(nodeID.unsignedLongLongValue), chip::MakeOptional(chip::Transport::SecureSession::Type::kCASE)); }; [self syncRunOnWorkQueue:block error:nil]; diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 84a8b536ec54e8..8fd6e4cb4bfccc 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -154,34 +154,6 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion; -/** - * Returns the transport used by the current session with the given device, - * or `MTRTransportTypeUndefined` if no session is currently active. - */ -- (MTRTransportType)sessionTransportTypeForDevice:(MTRBaseDevice *)device; - -/** - * 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 - * the Matter event queue. - */ -- (void)invalidateCASESessionForNode:(chip::NodeId)nodeID; - -/** - * Try to asynchronously dispatch the given block on the Matter queue. If the - * controller is not running either at call time or when the block would be - * about to run, the provided error handler will be called with an error. Note - * that this means the error handler might be called on an arbitrary queue, and - * might be called before this function returns or after it returns. - * - * The DeviceCommissioner pointer passed to the callback should only be used - * synchronously during the callback invocation. - * - * If the error handler is nil, failure to run the block will be silent. - */ -- (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block - errorHandler:(nullable MTRDeviceErrorHandler)errorHandler; - /** * Try to asynchronously dispatch the given block on the Matter queue. If the * controller is not running either at call time or when the block would be @@ -200,15 +172,6 @@ NS_ASSUME_NONNULL_BEGIN */ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID; -/** - * Download log of the desired type from the device. - */ -- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID - type:(MTRDiagnosticLogType)type - timeout:(NSTimeInterval)timeout - queue:(dispatch_queue_t)queue - completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion; - #pragma mark - Device-specific data and SDK access // DeviceController will act as a central repository for this opaque dictionary that MTRDevice manages - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID; From 46bac61b1b709d106413d84c1f4042db3d4da86f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 8 Oct 2024 12:45:41 -0400 Subject: [PATCH 2/2] Address review comment: add logs. --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index c11e59eb1531c8..89f7aab4250a0c 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -280,6 +280,7 @@ - (MTRTransportType)sessionTransportType { auto * concreteController = self.concreteController; if (concreteController == nil) { + MTR_LOG_ERROR("Unable to determine session transport type for MTRBaseDevice created with an XPC controller"); return MTRTransportTypeUndefined; } return [concreteController sessionTransportTypeForDevice:self]; @@ -294,6 +295,7 @@ - (void)invalidateCASESession auto * concreteController = self.concreteController; if (concreteController == nil) { // Nothing we can do here. + MTR_LOG_ERROR("Unable invalidate CASE session for MTRBaseDevice created with an XPC controller"); return; } @@ -339,6 +341,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue auto * concreteController = self.concreteController; if (concreteController == nil) { // No subscriptions (or really any MTRBaseDevice use) with XPC controllers. + MTR_LOG_ERROR("Unable to create subscription for MTRBaseDevice created with an XPC controller"); dispatch_async(queue, ^{ errorHandler([MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); }); @@ -1653,6 +1656,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl auto * concreteController = self.concreteController; if (concreteController == nil) { // No subscriptions (or really any MTRBaseDevice use) with XPC controllers. + MTR_LOG_ERROR("Unable to create subscription for MTRBaseDevice created with an XPC controller"); dispatch_async(queue, ^{ reportHandler(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); }); @@ -1935,7 +1939,7 @@ - (void)_openCommissioningWindowWithSetupPasscode:(nullable NSNumber *)setupPass auto * concreteController = self.concreteController; if (concreteController == nil) { - MTR_LOG_ERROR("Can't open a commissioning window via MTRBaseDevice against an XPC Controller"); + MTR_LOG_ERROR("Can't open a commissioning window via MTRBaseDevice created with an XPC controller"); dispatch_async(queue, ^{ MATTER_LOG_METRIC_END(kMetricOpenPairingWindow, CHIP_ERROR_INCORRECT_STATE); completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); @@ -2230,7 +2234,7 @@ - (void)downloadLogOfType:(MTRDiagnosticLogType)type { auto * concreteController = self.concreteController; if (concreteController == nil) { - MTR_LOG_ERROR("Can't download logs via MTRBaseDevice against an XPC Controller"); + MTR_LOG_ERROR("Can't download logs via MTRBaseDevice created with an XPC controller"); dispatch_async(queue, ^{ completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); });