Followup project-chip#27318 - Fix review comments that came in after …

…landing
vivien-apple · Jul 4, 2023 · 6adfd1d · 6adfd1d
1 parent b3002c6
commit 6adfd1d
Show file tree

Hide file tree

Showing 10 changed files with 436 additions and 74 deletions.
diff --git a/.github/workflows/darwin.yaml b/.github/workflows/darwin.yaml
@@ -80,7 +80,10 @@ jobs:
               # Disable -Wunguarded-availability-new because we internally use
               # APIs we added after our deployment target version.  Maybe we
               # should change the deployment target version instead?
-              run: xcodebuild -target "Matter" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new'
+              #
+              # Disable BLE because the app does not have the permission to use
+              # it and that may crash the CI.
+              run: xcodebuild -target "Matter" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new' CHIP_IS_BLE=NO
               working-directory: src/darwin/Framework
             - name: Clean Build
               run: xcodebuild clean
@@ -121,7 +124,7 @@ jobs:
                   ../../../out/debug/chip-ota-requestor-app --interface-id -1 --secured-device-port 5543 --discriminator 1112 --KVS /tmp/chip-ota-requestor-kvs2 --otaDownloadPath /tmp/chip-ota-requestor-downloaded-image2 --autoApplyImage > >(tee /tmp/darwin/framework-tests/ota-requestor-app-2.log) 2> >(tee /tmp/darwin/framework-tests/ota-requestor-app-err-2.log >&2) &
                   # -enableUndefinedBehaviorSanitizer instruments the code in Matter.framework,
                   # but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_UBSAN=YES
-                  TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new' CHIP_IS_UBSAN=YES > >(tee /tmp/darwin/framework-tests/darwin-tests-asan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-err.log >&2)
+                  TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO > >(tee /tmp/darwin/framework-tests/darwin-tests-asan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-err.log >&2)
                   # -enableThreadSanitizer instruments the code in Matter.framework,
                   # but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_TSAN=YES
                   xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableThreadSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new' CHIP_IS_TSAN=YES > >(tee /tmp/darwin/framework-tests/darwin-tests-tsan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-tsan-err.log >&2)

diff --git a/examples/darwin-framework-tool/commands/discover/DiscoverCommissionablesCommand.mm b/examples/darwin-framework-tool/commands/discover/DiscoverCommissionablesCommand.mm
@@ -21,31 +21,32 @@
 auto gDispatchQueue = dispatch_queue_create("com.chip.discover", DISPATCH_QUEUE_SERIAL);
 
 @interface DeviceScannerDelegate : NSObject <MTRCommissionableBrowserDelegate>
-- (void)didDiscoverCommissionable:(MTRCommissionableBrowserResult *)device;
-- (void)commissionableUnavailable:(MTRCommissionableBrowserResult *)device;
+- (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice:(MTRCommissionableBrowserResult *)device;
+- (void)controller:(MTRDeviceController *)controller didRemoveCommissionableDevice:(MTRCommissionableBrowserResult *)device;
 @end
 
 @implementation DeviceScannerDelegate
-- (void)didDiscoverCommissionable:(MTRCommissionableBrowserResult *)device
+- (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice:(MTRCommissionableBrowserResult *)device
 {
-    auto serviceName = device.serviceName;
-    auto vendorId = device.vendorId;
-    auto productId = device.productId;
+    auto instanceName = device.instanceName;
+    auto vendorId = device.vendorID;
+    auto productId = device.productID;
     auto discriminator = device.discriminator;
     [gDiscoveredDevices addObject:device];
 
-    NSLog(@"Found Device (%@) with discriminator: %@ (vendor: %@, product: %@)", serviceName, discriminator, vendorId, productId);
+    NSLog(@"Found Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);
 }
 
-- (void)commissionableUnavailable:(MTRCommissionableBrowserResult *)device
+- (void)controller:(MTRDeviceController *)controller didRemoveCommissionableDevice:(MTRCommissionableBrowserResult *)device
 {
-    auto serviceName = device.serviceName;
-    auto vendorId = device.vendorId;
-    auto productId = device.productId;
+    auto instanceName = device.instanceName;
+    auto vendorId = device.vendorID;
+    auto productId = device.productID;
     auto discriminator = device.discriminator;
     [gDiscoveredDevices removeObjectIdenticalTo:device];
 
-    NSLog(@"Removed Device (%@) with discriminator: %@ (vendor: %@, product: %@)", serviceName, discriminator, vendorId, productId);
+    NSLog(
+        @"Removed Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);
 }
 @end
 
@@ -58,7 +59,7 @@ - (void)commissionableUnavailable:(MTRCommissionableBrowserResult *)device
     });
 
     auto delegate = [[DeviceScannerDelegate alloc] init];
-    auto success = [CurrentCommissioner() startScan:delegate queue:gDispatchQueue];
+    auto success = [CurrentCommissioner() startBrowseForCommissionables:delegate queue:gDispatchQueue];
     VerifyOrReturnError(success, CHIP_ERROR_INTERNAL);
 
     SetCommandExitStatus(CHIP_NO_ERROR);
@@ -69,7 +70,7 @@ - (void)commissionableUnavailable:(MTRCommissionableBrowserResult *)device
 {
     VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);
 
-    auto success = [CurrentCommissioner() stopScan];
+    auto success = [CurrentCommissioner() stopBrowseForCommissionables];
     VerifyOrReturnError(success, CHIP_ERROR_INTERNAL);
 
     SetCommandExitStatus(CHIP_NO_ERROR);
@@ -86,13 +87,13 @@ - (void)commissionableUnavailable:(MTRCommissionableBrowserResult *)device
 
         uint16_t index = 0;
         for (id device in gDiscoveredDevices) {
-            auto serviceName = [device serviceName];
-            auto vendorId = [device vendorId];
-            auto productId = [device productId];
+            auto instanceName = [device instanceName];
+            auto vendorId = [device vendorID];
+            auto productId = [device productID];
             auto discriminator = [device discriminator];
 
-            NSLog(
-                @"\t %u %@ - Discriminator: %@ - Vendor: %@ - Product: %@", index, serviceName, discriminator, vendorId, productId);
+            NSLog(@"\t %u %@ - Discriminator: %@ - Vendor: %@ - Product: %@", index, instanceName, discriminator, vendorId,
+                productId);
 
             index++;
         }

diff --git a/src/darwin/Framework/CHIP/MTRCommissionableBrowser.h b/src/darwin/Framework/CHIP/MTRCommissionableBrowser.h
@@ -21,13 +21,25 @@
 NS_ASSUME_NONNULL_BEGIN
 
 @protocol MTRCommissionableBrowserDelegate;
+@class MTRDeviceController;
 
 MTR_HIDDEN
 @interface MTRCommissionableBrowser : NSObject
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
-- (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate queue:(dispatch_queue_t)queue;
+- (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate
+                      controller:(MTRDeviceController *)controller
+                           queue:(dispatch_queue_t)queue;
+/**
+ * Start browsing the available networks (e.g IP, BLE) for commissionable nodes.
+ *
+ * If a browse is already ongoing this will not start a new browse and the return value will be NO.
+ */
 - (BOOL)start;
+
+/**
+ * Stop browsing the network for commissionable nodes.
+ */
 - (BOOL)stop;
 @end