Remove session accessors on non-concrete MTRDeviceController. (#36317)
CASE/PASE sessions only make sense for concrete controllers.
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index 3868060..0f86a76 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -233,11 +233,6 @@
@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
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h
index fa76646..4cf02eb 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h
@@ -19,6 +19,7 @@
#import <Foundation/Foundation.h>
#import "MTRDefines_Internal.h"
+#import "MTRDeviceController_Concrete.h"
#include <app/AttributePathParams.h>
#include <app/ConcreteAttributePath.h>
@@ -210,6 +211,11 @@
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion;
+/**
+ * Will return nil if our controller is not in fact a concrete controller.
+ */
+@property (nullable, nonatomic, strong, readonly) MTRDeviceController_Concrete * concreteController;
+
@end
@interface MTRClusterPath ()
diff --git a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h
index d9c9815..e8b0d1e 100644
--- a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h
+++ b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h
@@ -18,8 +18,10 @@
#import <Foundation/Foundation.h>
#import "MTRBaseDevice_Internal.h"
+#import "MTRDeviceController_Concrete.h"
#import "MTRDeviceController_Internal.h"
#import "MTRError_Internal.h"
+#import "MTRLogging_Internal.h"
#import "zap-generated/MTRBaseClusters.h"
#include <app/data-model/NullObject.h>
@@ -37,17 +39,6 @@
class MTRCallbackBridgeBase {
public:
/**
- * Run the given MTRActionBlock on the Matter thread, after getting a CASE
- * session (possibly pre-existing) to the given node ID on the fabric
- * represented by the given MTRDeviceController. On success, convert the
- * success value to whatever type it needs to be to call the callback type
- * we're templated over. Once this function has been called, on a callback
- * bridge allocated with `new`, the bridge object must not be accessed by
- * the caller. The action block will handle deleting the bridge.
- */
- void DispatchAction(chip::NodeId nodeID, MTRDeviceController * controller) && { ActionWithNodeID(nodeID, controller); }
-
- /**
* 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
@@ -60,7 +51,7 @@
if (device.isPASEDevice) {
ActionWithPASEDevice(device);
} else {
- ActionWithNodeID(device.nodeID, device.deviceController);
+ ActionWithNodeID(device.nodeID, device.concreteController);
}
}
@@ -81,22 +72,30 @@
{
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<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
- MaybeDoAction(exchangeManager, session, error);
- }];
+ MTRDeviceController_Concrete * _Nullable controller = device.concreteController;
+ if (!controller) {
+ MTR_LOG_ERROR("Trying to perform action with PASE device with a non-concrete controller");
+ MaybeDoAction(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
+ return;
+ }
+
+ [controller getSessionForCommissioneeDevice:device.nodeID
+ completion:^(chip::Messaging::ExchangeManager * exchangeManager,
+ const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
+ MaybeDoAction(exchangeManager, session, error);
+ }];
}
- void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController * controller)
+ void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController_Concrete * _Nullable controller)
{
LogRequestStart();
- // TODO: Figure out whether we can usefully get an MTRDeviceController_Concrete in here, so
- // we can move getSessionForNode off of MTRDeviceController_Internal.
+ if (!controller) {
+ MTR_LOG_ERROR("Trying to perform action on node ID 0x%016llX (%llu) with a non-concrete controller", nodeID, nodeID);
+ MaybeDoAction(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
+ return;
+ }
+
[controller getSessionForNode:nodeID
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
index 8b8295d..5850d88 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -535,18 +535,6 @@
return NO;
}
-- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
-{
- MTR_ABSTRACT_METHOD();
- completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
-}
-
-- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion
-{
- MTR_ABSTRACT_METHOD();
- completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
-}
-
- (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler
{
// TODO: Figure out how to get callsites to have an MTRDeviceController_Concrete.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
index 0cfb25f..80f5b22 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
@@ -121,6 +121,23 @@
- (void)clearPendingShutdown;
/**
+ * Ensure we have a CASE session to the given node ID and then call the provided
+ * connection callback. This may be called on any queue (including the Matter
+ * event queue) and on success 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, it will
+ * synchronously invoke the completion with an error, on whatever queue
+ * getSessionForNode was called on.
+ *
+ * If the controller is not running when the async dispatch on the Matter queue
+ * happens, the completion will be invoked with an error on the Matter queue.
+ */
+- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
+
+/**
* Since getSessionForNode now enqueues by the subscription pool for Thread
* devices, MTRDevice_Concrete needs a direct non-queued access because it already
* makes use of the subscription pool.
@@ -128,6 +145,23 @@
- (void)directlyGetSessionForNode:(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 on success 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, it will
+ * synchronously invoke the completion with an error, on whatever queue
+ * getSessionForCommissioneeDevice was called on.
+ *
+ * If the controller is not running when the async dispatch on the Matter queue
+ * happens, the completion will be invoked with an error on the Matter queue.
+ */
+- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion;
+
+/**
* Notify the controller that a new operational instance with the given node id
* and a compressed fabric id that matches this controller has been observed.
*/
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
index c26518d..145f329 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
@@ -122,40 +122,6 @@
@property (nonatomic, retain, nullable) NSData * rootPublicKey;
/**
- * Ensure we have a CASE session to the given node ID and then call the provided
- * connection callback. This may be called on any queue (including the Matter
- * event queue) and on success 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, it will
- * synchronously invoke the completion with an error, on whatever queue
- * getSessionForNode was called on.
- *
- * If the controller is not running when the async dispatch on the Matter queue
- * happens, the completion will be invoked with an error on the Matter queue.
- */
-- (void)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 on success 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, it will
- * synchronously invoke the completion with an error, on whatever queue
- * getSessionForCommissioneeDevice was called on.
- *
- * If the controller is not running when the async dispatch on the Matter queue
- * happens, the completion will be invoked with an error on the Matter queue.
- */
-- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion;
-
-/**
* 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
diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
index 0715663..1ead469 100644
--- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
@@ -2706,8 +2706,8 @@
// Given the base device read is happening on the 5th device, at the completion
// time of the first [pool size] subscriptions, the BaseDevice's request to
- // read can't have completed, as it should be gated on its call to the
- // MTRDeviceController's getSessionForNode: call.
+ // read can't have completed, as it should be gated on its call to
+ // MTRDeviceController_Concrete's getSessionForNode:.
if (subscriptionDequeueCount <= (orderedDeviceIDs.count - subscriptionPoolSize)) {
XCTAssertFalse(baseDeviceReadCompleted);
}
