Darwin: Move preWarmCommissioningSession to MTRDeviceControllerFactory (

#33184) * Darwin: Move preWarmCommissioningSession to MTRDeviceControllerFactory This makes it possible to start pre-warming before a MTRDeviceController has been created, since doing that requires the client to have obtained various configuration details. * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> * Tweaks from review * restyle --------- Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Apr 29, 2024 · 34cc8b8 · 34cc8b8
1 parent f9ac954
commit 34cc8b8
Show file tree

Hide file tree

Showing 11 changed files with 148 additions and 90 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h
@@ -178,16 +178,8 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
                                                         error:(NSError * __autoreleasing *)error
     MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
 
-/**
- * Optionally pre-warm the controller for setting up a commissioning session.
- * This may be called before setupCommissioningSessionWithPayload if it's known
- * that a commissioning attempt will soon take place but the commissioning
- * payload is not known yet.
- *
- * For example this may do a BLE scan in advance so results are ready earlier
- * once the discriminator is known.
- */
-- (void)preWarmCommissioningSession MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+- (void)preWarmCommissioningSession MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
+    MTR_NEWLY_DEPRECATED("-[MTRDeviceControllerFactory preWarmCommissioningSession]");
 
 /**
  * Set the Delegate for the device controller  as well as the Queue on which the Delegate callbacks will be triggered

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -86,7 +86,6 @@
 static NSString * const kErrorPartialDacVerifierInit = @"Init failure while creating a partial DAC verifier";
 static NSString * const kErrorPairDevice = @"Failure while pairing the device";
 static NSString * const kErrorStopPairing = @"Failure while trying to stop the pairing process";
-static NSString * const kErrorPreWarmCommissioning = @"Failure while trying to pre-warm the commissioning process";
 static NSString * const kErrorOpenPairingWindow = @"Open Pairing Window failed";
 static NSString * const kErrorNotRunning = @"Controller is not running. Call startup first.";
 static NSString * const kErrorSetupCodeGen = @"Generating Manual Pairing Code failed";
@@ -896,15 +895,7 @@ - (BOOL)stopBrowseForCommissionables
 
 - (void)preWarmCommissioningSession
 {
-    auto block = ^{
-        auto errorCode = chip::DeviceLayer::PlatformMgrImpl().PrepareCommissioning();
-        MATTER_LOG_METRIC(kMetricPreWarmCommissioning, errorCode);
-
-        // The checkForError is just so it logs
-        [MTRDeviceController checkForError:errorCode logMsg:kErrorPreWarmCommissioning error:nil];
-    };
-
-    [self syncRunOnWorkQueue:block error:nil];
+    [_factory preWarmCommissioningSession];
 }
 
 - (MTRBaseDevice *)deviceBeingCommissionedWithNodeID:(NSNumber *)nodeID error:(NSError * __autoreleasing *)error

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.h b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.h
@@ -177,6 +177,18 @@ MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
 - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControllerStartupParams *)startupParams
                                                          error:(NSError * __autoreleasing *)error;
 
+/**
+ * If possible, pre-warm the Matter stack for setting up a commissioning session.
+ *
+ * This may be called before -[MTRDeviceController setupCommissioningSessionWithPayload:]
+ * if it is known that a commissioning attempt will soon take place, but the commissioning
+ * payload is not known yet.
+ *
+ * The controller factory must be running for pre-warming to take place.  Pre-warming can take place
+ * before any controllers are started.
+ */
+- (void)preWarmCommissioningSession MTR_NEWLY_AVAILABLE;
+
 @end
 
 /**

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
@@ -34,6 +34,7 @@
 #import "MTRFabricInfo_Internal.h"
 #import "MTRFramework.h"
 #import "MTRLogging_Internal.h"
+#import "MTRMetricKeys.h"
 #import "MTRMetricsCollector.h"
 #import "MTROTAProviderDelegateBridge.h"
 #import "MTROperationalBrowser.h"
@@ -66,6 +67,7 @@
 
 using namespace chip;
 using namespace chip::Controller;
+using namespace chip::Tracing::DarwinFramework;
 
 static bool sExitHandlerRegistered = false;
 static void ShutdownOnExit()
@@ -82,6 +84,11 @@ @interface MTRDeviceControllerFactoryParams ()
 
 @end
 
+MTR_DIRECT_MEMBERS
+@interface MTRDeviceControllerFactory ()
+- (void)preWarmCommissioningSessionDone;
+@end
+
 MTR_DIRECT_MEMBERS
 @implementation MTRDeviceControllerFactory {
     dispatch_queue_t _chipWorkQueue;
@@ -163,6 +170,13 @@ @implementation MTRDeviceControllerFactory {
     // in an atomic way that endpoint IDs are unique.
     NSMutableArray<MTRServerEndpoint *> * _serverEndpoints;
     os_unfair_lock _serverEndpointsLock; // Protects access to _serverEndpoints.
+
+    class final : public DeviceLayer::BleScannerDelegate {
+        void OnBleScanStopped() override
+        {
+            [MTRDeviceControllerFactory.sharedInstance preWarmCommissioningSessionDone];
+        }
+    } _preWarmingDelegate;
 }
 
 + (void)initialize
@@ -742,6 +756,32 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl
                                   error:error];
 }
 
+- (void)preWarmCommissioningSession
+{
+    dispatch_async(_chipWorkQueue, ^{
+        CHIP_ERROR err = CHIP_ERROR_INCORRECT_STATE;
+        if (!self->_running) {
+            MTR_LOG_ERROR("Can't pre-warm, Matter controller factory is not running");
+        } else {
+            MTR_LOG_DEFAULT("Pre-warming commissioning session");
+            self->_controllerFactory->EnsureAndRetainSystemState();
+            err = DeviceLayer::PlatformMgrImpl().StartBleScan(&self->_preWarmingDelegate, DeviceLayer::BleScanMode::kPreWarm);
+            if (err != CHIP_NO_ERROR) {
+                MTR_LOG_ERROR("Pre-warming failed: %" CHIP_ERROR_FORMAT, err.Format());
+                self->_controllerFactory->ReleaseSystemState();
+            }
+        }
+        MATTER_LOG_METRIC(kMetricPreWarmCommissioning, err);
+    });
+}
+
+- (void)preWarmCommissioningSessionDone
+{
+    assertChipStackLockedByCurrentThread();
+    MTR_LOG_DEFAULT("Pre-warming done");
+    self->_controllerFactory->ReleaseSystemState();
+}
+
 // Finds a fabric that matches the given params, if one exists.
 //
 // Returns NO on failure, YES on success.  If YES is returned, the

diff --git a/src/platform/Darwin/BLEManagerImpl.cpp b/src/platform/Darwin/BLEManagerImpl.cpp
@@ -87,24 +87,18 @@ void BLEManagerImpl::_Shutdown()
     }
 }
 
-CHIP_ERROR BLEManagerImpl::StartScan(BleScannerDelegate * delegate)
+CHIP_ERROR BLEManagerImpl::StartScan(BleScannerDelegate * delegate, BleScanMode mode)
 {
-    if (mConnectionDelegate)
-    {
-        static_cast<BleConnectionDelegateImpl *>(mConnectionDelegate)->StartScan(delegate);
-        return CHIP_NO_ERROR;
-    }
-    return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(mConnectionDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    static_cast<BleConnectionDelegateImpl *>(mConnectionDelegate)->StartScan(delegate, mode);
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR BLEManagerImpl::StopScan()
 {
-    if (mConnectionDelegate)
-    {
-        static_cast<BleConnectionDelegateImpl *>(mConnectionDelegate)->StopScan();
-        return CHIP_NO_ERROR;
-    }
-    return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(mConnectionDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    static_cast<BleConnectionDelegateImpl *>(mConnectionDelegate)->StopScan();
+    return CHIP_NO_ERROR;
 }
 
 bool BLEManagerImpl::_IsAdvertisingEnabled()

diff --git a/src/platform/Darwin/BLEManagerImpl.h b/src/platform/Darwin/BLEManagerImpl.h
@@ -25,14 +25,12 @@
 
 #include <lib/core/Global.h>
 #include <lib/support/CodeUtils.h>
+#include <platform/Darwin/BleScannerDelegate.h>
 
 #if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
 
 namespace chip {
 namespace DeviceLayer {
-
-class BleScannerDelegate;
-
 namespace Internal {
 
 using namespace chip::Ble;
@@ -48,7 +46,7 @@ class BLEManagerImpl final : public BLEManager, private BleLayer
 
 public:
     CHIP_ERROR ConfigureBle(uint32_t aNodeId, bool aIsCentral) { return CHIP_NO_ERROR; }
-    CHIP_ERROR StartScan(BleScannerDelegate * delegate = nullptr);
+    CHIP_ERROR StartScan(BleScannerDelegate * delegate, BleScanMode mode = BleScanMode::kDefault);
     CHIP_ERROR StopScan();
 
 private:

diff --git a/src/platform/Darwin/BleConnectionDelegate.h b/src/platform/Darwin/BleConnectionDelegate.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include <ble/Ble.h>
+#include <platform/Darwin/BleScannerDelegate.h>
 
 namespace chip {
 namespace DeviceLayer {
@@ -26,11 +27,12 @@ namespace Internal {
 class BleConnectionDelegateImpl : public Ble::BleConnectionDelegate
 {
 public:
-    void StartScan(BleScannerDelegate * delegate = nullptr);
+    void StartScan(BleScannerDelegate * delegate, BleScanMode mode = BleScanMode::kDefault);
     void StopScan();
-    virtual void NewConnection(Ble::BleLayer * bleLayer, void * appState, const SetupDiscriminator & connDiscriminator);
-    virtual void NewConnection(Ble::BleLayer * bleLayer, void * appState, BLE_CONNECTION_OBJECT connObj);
-    virtual CHIP_ERROR CancelConnection();
+
+    void NewConnection(Ble::BleLayer * bleLayer, void * appState, const SetupDiscriminator & connDiscriminator) override;
+    void NewConnection(Ble::BleLayer * bleLayer, void * appState, BLE_CONNECTION_OBJECT connObj) override;
+    CHIP_ERROR CancelConnection() override;
 
 private:
     CHIP_ERROR DoCancel();