Merge branch 'master' into master

src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm

@@ -461,7 +461,7 @@ - (void)stopControllerFactory
  * The provided controller is expected to have just been allocated and to not be
  * initialized yet.
  */
-- (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *)controller
+- (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController_Concrete *)controller
                                             startupParams:(id)startupParams
                                             fabricChecker:(MTRDeviceControllerStartupParamsInternal * (^)(FabricTable * fabricTable,
                                                               MTRDeviceController * controller,
@@ -834,7 +834,7 @@ - (BOOL)findMatchingFabric:(FabricTable &)fabricTable
 // Returns nil on failure, the input controller on success.
 // If the provider has been initialized already, it is not considered as a failure.
 //
-- (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceController * _Nonnull)controller
+- (MTRDeviceController_Concrete * _Nullable)maybeInitializeOTAProvider:(MTRDeviceController_Concrete * _Nonnull)controller
 {
     [self _assertCurrentQueueIsNotMatterQueue];
 
@@ -1144,6 +1144,9 @@ - (nullable MTRDeviceController *)_findPendingShutdownControllerWithOperationalC
 {
     std::lock_guard lock(_controllersLock);
     for (MTRDeviceController * controller in _controllers) {
+        // TODO: Once we know our controllers are MTRDeviceController_Concrete, move
+        // matchesPendingShutdownControllerWithOperationalCertificate and clearPendingShutdown to that
+        // interface and remove them from base MTRDeviceController_Internal.
         if ([controller matchesPendingShutdownControllerWithOperationalCertificate:operationalCertificate andRootCertificate:rootCertificate]) {
             MTR_LOG("%@ Found existing controller %@ that is pending shutdown and matching parameters, re-using it", self, controller);
             [controller clearPendingShutdown];
@@ -1153,7 +1156,7 @@ - (nullable MTRDeviceController *)_findPendingShutdownControllerWithOperationalC
     return nil;
 }
 
-- (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)controller
+- (nullable MTRDeviceController *)initializeController:(MTRDeviceController_Concrete *)controller
                                         withParameters:(MTRDeviceControllerParameters *)parameters
                                                  error:(NSError * __autoreleasing *)error
 {

src/darwin/Framework/CHIP/MTRDeviceControllerFactory_Internal.h

@@ -30,6 +30,7 @@
 
 #import "MTRDefines_Internal.h"
 #import "MTRDeviceControllerFactory.h"
+#import "MTRDeviceController_Concrete.h"
 #import "MTROperationalBrowser.h"
 
 #include <credentials/FabricTable.h>
@@ -89,9 +90,9 @@ MTR_DIRECT_MEMBERS
                        completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion;
 
 /**
- * Initialize an MTRDeviceController with the given parameters.
+ * Initialize an MTRDeviceController_Concrete with the given parameters.
  */
-- (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)controller
+- (nullable MTRDeviceController *)initializeController:(MTRDeviceController_Concrete *)controller
                                         withParameters:(MTRDeviceControllerParameters *)parameters
                                                  error:(NSError * __autoreleasing *)error;
 

src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC.h

@@ -16,15 +16,16 @@
  */
 
 #import <Matter/MTRDefines.h>
-#import <Matter/MTRDeviceController.h>
+
+#import "MTRDeviceController_Concrete.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
 @class MTRDeviceControllerXPCConnection;
 
 typedef NSXPCConnection * _Nonnull (^MTRXPCConnectBlock)(void);
 
-@interface MTRDeviceControllerOverXPC : MTRDeviceController
+@interface MTRDeviceControllerOverXPC : MTRDeviceController_Concrete
 
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;

src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

@@ -166,7 +166,7 @@ MTR_NEWLY_AVAILABLE
 /**
  * A controller created from this way will connect to a remote instance of an MTRDeviceController loaded in an XPC Service
  *
- * @param xpcConnectionBlock The XPC Connection block that will return an NSXPCConnection to the indended listener.
+ * @param xpcConnectionBlock The XPC Connection block that will return an NSXPCConnection to the intended listener.
  *
  * @param uniqueIdentifier The unique id to assign to the controller.
  *

src/darwin/Framework/CHIP/MTRDeviceControllerXPCParameters.h

@@ -18,11 +18,11 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@interface MTRDeviceControllerXPCParameters : MTRDeviceControllerParameters
-@end
+@interface MTRDeviceControllerMachServiceXPCParameters : MTRDeviceControllerAbstractParameters
 
-@interface MTRDeviceControllerMachServiceXPCParameters : MTRDeviceControllerXPCParameters
+- (nullable instancetype)initWithUniqueIdentifier:(NSUUID *)uniqueIdentifier;
 
+@property (atomic, retain) NSUUID * uniqueIdentifier;
 @property (atomic, retain) NSString * machServiceName;
 @property (atomic, readwrite) NSXPCConnectionOptions connectionOptions;
 

src/darwin/Framework/CHIP/MTRDeviceControllerXPCParameters.mm

@@ -15,17 +15,19 @@
  */
 
 #import "MTRDeviceControllerXPCParameters.h"
+#import "MTRDeviceControllerStartupParams_Internal.h"
 
-@implementation MTRDeviceControllerXPCParameters
+@implementation MTRDeviceControllerMachServiceXPCParameters
 
-+ (BOOL)supportsSecureCoding
+- (nullable instancetype)initWithUniqueIdentifier:(NSUUID *)uniqueIdentifier
 {
-    return YES;
-}
+    if (!(self = [super _initInternal])) {
+        return nil;
+    }
 
-@end
-
-@implementation MTRDeviceControllerMachServiceXPCParameters
+    _uniqueIdentifier = uniqueIdentifier;
+    return self;
+}
 
 + (BOOL)supportsSecureCoding
 {

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h

@@ -20,9 +20,54 @@
 #import <Matter/MTRDefines.h>
 #import <Matter/MTRDeviceController.h>
 
+#import "MTRDeviceControllerStartupParams_Internal.h"
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface MTRDeviceController_Concrete : MTRDeviceController
+
+/**
+ * Init a newly created controller.
+ *
+ * Only MTRDeviceControllerFactory should be calling this.
+ */
+- (nullable instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
+                                   queue:(dispatch_queue_t)queue
+                         storageDelegate:(id<MTRDeviceControllerStorageDelegate> _Nullable)storageDelegate
+                    storageDelegateQueue:(dispatch_queue_t _Nullable)storageDelegateQueue
+                     otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
+                otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
+                        uniqueIdentifier:(NSUUID *)uniqueIdentifier
+          concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
+            storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
+                          startSuspended:(BOOL)startSuspended;
+
+/**
+ * Start a new controller.  Returns whether startup succeeded.  If this fails,
+ * it guarantees that it has called controllerShuttingDown on the
+ * MTRDeviceControllerFactory.
+ *
+ * The return value will always match [controller isRunning] for this
+ * controller.
+ *
+ * Only MTRDeviceControllerFactory should be calling this.
+ */
+- (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams;
+
+/**
+ * Takes an assertion to keep the controller running. If `-[MTRDeviceController shutdown]` is called while an assertion
+ * is held, the shutdown will be honored only after all assertions are released. Invoking this method multiple times increases
+ * the number of assertions and needs to be matched with equal amount of '-[MTRDeviceController removeRunAssertion]` to release
+ * the assertion.
+ */
+- (void)addRunAssertion;
+
+/**
+ * Removes an assertion to allow the controller to shutdown once all assertions have been released.
+ * Invoking this method once all assertions have been released in a noop.
+ */
+- (void)removeRunAssertion;
+
 @end
 
 NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

@@ -31,10 +31,7 @@
 #import "MTRDeviceControllerLocalTestStorage.h"
 #import "MTRDeviceControllerStartupParams.h"
 #import "MTRDeviceControllerStartupParams_Internal.h"
-#import "MTRDeviceControllerXPCParameters.h"
 #import "MTRDeviceController_Concrete.h"
-#import "MTRDeviceController_XPC.h"
-#import "MTRDeviceController_XPC_Internal.h"
 #import "MTRDevice_Concrete.h"
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
@@ -148,61 +145,34 @@ @implementation MTRDeviceController_Concrete {
 - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters
                                       error:(NSError * __autoreleasing *)error
 {
-    /// IF YOU ARE ALARMED BY TYPES:  You are right to be alarmed, but do not panic.
-    /// _ORDER MATTERS HERE:_ XPC parameters are a subclass of `MTRDeviceControllerParameters`
-    /// because of the enormous overlap of params.
-    if ([parameters isKindOfClass:MTRDeviceControllerXPCParameters.class]) {
-        if ([parameters isKindOfClass:MTRDeviceControllerMachServiceXPCParameters.class]) {
-            MTRDeviceControllerMachServiceXPCParameters * xpcParameters = (MTRDeviceControllerMachServiceXPCParameters *) parameters;
-            MTR_LOG_DEBUG("%s: got XPC parameters, getting XPC device controller", __PRETTY_FUNCTION__);
-
-            NSString * machServiceName = xpcParameters.machServiceName;
-            MTR_LOG_DEBUG("%s: machServiceName %@", __PRETTY_FUNCTION__, machServiceName);
-
-            MTRDeviceController * xpcDeviceController = [[MTRDeviceController_XPC alloc] initWithMachServiceName:machServiceName options:xpcParameters.connectionOptions];
-
-            /// Being of sound mind, I willfully and voluntarily make this static cast.
-            return static_cast<MTRDeviceController_Concrete *>(xpcDeviceController);
-        } else {
-            MTR_LOG_ERROR("%s: unrecognized XPC parameters class %@", __PRETTY_FUNCTION__, NSStringFromClass(parameters.class));
-
-            // TODO:  there's probably a more appropriate error here.
-            if (error) {
-                *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_NOT_IMPLEMENTED];
-            }
-
-            return nil;
-        }
-    } else if ([parameters isKindOfClass:MTRDeviceControllerParameters.class]) {
-        MTR_LOG_DEBUG("%s: got standard parameters, getting standard device controller from factory", __PRETTY_FUNCTION__);
-        auto * controllerParameters = static_cast<MTRDeviceControllerParameters *>(parameters);
-
-        // Start us up normally. MTRDeviceControllerFactory will auto-start in per-controller-storage mode if necessary.
-        MTRDeviceControllerFactory * factory = MTRDeviceControllerFactory.sharedInstance;
-        id controller = [factory initializeController:self
-                                       withParameters:controllerParameters
-                                                error:error];
-        return controller;
-    } else {
-        // way out of our league
-        MTR_LOG_ERROR("Unsupported type of MTRDeviceControllerAbstractParameters: %@", parameters);
+    if (![parameters isKindOfClass:MTRDeviceControllerParameters.class]) {
+        MTR_LOG_ERROR("Expected MTRDeviceControllerParameters but got: %@", parameters);
         if (error) {
             *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
         }
         return nil;
     }
+
+    auto * controllerParameters = static_cast<MTRDeviceControllerParameters *>(parameters);
+
+    // Start us up normally. MTRDeviceControllerFactory will auto-start in per-controller-storage mode if necessary.
+    MTRDeviceControllerFactory * factory = MTRDeviceControllerFactory.sharedInstance;
+    id controller = [factory initializeController:self
+                                   withParameters:controllerParameters
+                                            error:error];
+    return controller;
 }
 
-- (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
-                             queue:(dispatch_queue_t)queue
-                   storageDelegate:(id<MTRDeviceControllerStorageDelegate> _Nullable)storageDelegate
-              storageDelegateQueue:(dispatch_queue_t _Nullable)storageDelegateQueue
-               otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
-          otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
-                  uniqueIdentifier:(NSUUID *)uniqueIdentifier
-    concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
-      storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
-                    startSuspended:(BOOL)startSuspended
+- (nullable instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
+                                   queue:(dispatch_queue_t)queue
+                         storageDelegate:(id<MTRDeviceControllerStorageDelegate> _Nullable)storageDelegate
+                    storageDelegateQueue:(dispatch_queue_t _Nullable)storageDelegateQueue
+                     otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
+                otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
+                        uniqueIdentifier:(NSUUID *)uniqueIdentifier
+          concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
+            storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
+                          startSuspended:(BOOL)startSuspended
 {
     if (self = [super initForSubclasses:startSuspended]) {
         // Make sure our storage is all set up to work as early as possible,
@@ -1223,30 +1193,6 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
     return deviceToReturn;
 }
 
-- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
-{
-    std::lock_guard lock(*self.deviceMapLock);
-    MTRDevice * deviceToReturn = [self.nodeIDToDeviceMap objectForKey:nodeID];
-    if (!deviceToReturn) {
-        deviceToReturn = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:nil];
-    }
-
-    return deviceToReturn;
-}
-
-- (void)removeDevice:(MTRDevice *)device
-{
-    std::lock_guard lock(*self.deviceMapLock);
-    auto * nodeID = device.nodeID;
-    MTRDevice * deviceToRemove = [self.nodeIDToDeviceMap objectForKey:nodeID];
-    if (deviceToRemove == device) {
-        [deviceToRemove invalidate];
-        [self.nodeIDToDeviceMap removeObjectForKey:nodeID];
-    } else {
-        MTR_LOG_ERROR("%@ Error: Cannot remove device %p with nodeID %llu", self, device, nodeID.unsignedLongLongValue);
-    }
-}
-
 #ifdef DEBUG
 - (NSDictionary<NSNumber *, NSNumber *> *)unitTestGetDeviceAttributeCounts
 {

src/darwin/Framework/CHIP/MTRDeviceController_Internal.h

@@ -40,13 +40,11 @@
 #import <Matter/MTRP256KeypairBridge.h>
 
 #import <Matter/MTRDefines.h>
-#import <Matter/MTRDeviceControllerStartupParams.h>
 #import <Matter/MTRDeviceControllerStorageDelegate.h>
 #import <Matter/MTRDiagnosticLogsType.h>
 #import <Matter/MTROTAProviderDelegate.h>
 
 @class MTRDeviceControllerParameters;
-@class MTRDeviceControllerStartupParamsInternal;
 @class MTRDeviceControllerFactory;
 @class MTRDevice;
 @class MTRAsyncWorkQueue;
@@ -79,18 +77,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - MTRDeviceControllerFactory methods
 
-/**
- * Start a new controller.  Returns whether startup succeeded.  If this fails,
- * it guarantees that it has called controllerShuttingDown on the
- * MTRDeviceControllerFactory.
- *
- * The return value will always match [controller isRunning] for this
- * controller.
- *
- * Only MTRDeviceControllerFactory should be calling this.
- */
-- (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams;
-
 /**
  * Will return chip::kUndefinedFabricIndex if we do not have a fabric index.
  */
@@ -135,22 +121,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic, retain, nullable) NSData * rootPublicKey;
 
-/**
- * Init a newly created controller.
- *
- * Only MTRDeviceControllerFactory should be calling this.
- */
-- (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
-                             queue:(dispatch_queue_t)queue
-                   storageDelegate:(id<MTRDeviceControllerStorageDelegate> _Nullable)storageDelegate
-              storageDelegateQueue:(dispatch_queue_t _Nullable)storageDelegateQueue
-               otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
-          otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
-                  uniqueIdentifier:(NSUUID *)uniqueIdentifier
-    concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
-      storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
-                    startSuspended:(BOOL)startSuspended;
-
 /**
  * Check whether this controller is running on the given fabric, as represented
  * by the provided FabricTable and fabric index.  The provided fabric table may
@@ -298,6 +268,10 @@ NS_ASSUME_NONNULL_BEGIN
 #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;
+/**
+ * _setupDeviceForNodeID is a hook expected to be implemented by subclasses to
+ * actually allocate a device object of the right type.
+ */
 - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(nullable NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)prefetchedClusterData;
 - (void)removeDevice:(MTRDevice *)device;
 
@@ -308,20 +282,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
 
-/**
- * Takes an assertion to keep the controller running. If `-[MTRDeviceController shutdown]` is called while an assertion
- * is held, the shutdown will be honored only after all assertions are released. Invoking this method multiple times increases
- * the number of assertions and needs to be matched with equal amount of '-[MTRDeviceController removeRunAssertion]` to release
- * the assertion.
- */
-- (void)addRunAssertion;
-
-/**
- * Removes an assertion to allow the controller to shutdown once all assertions have been released.
- * Invoking this method once all assertions have been released in a noop.
- */
-- (void)removeRunAssertion;
-
 /**
  * This method returns TRUE if this controller matches the fabric reference and node ID as listed in the parameters.
  */