Skip to content

Commit

Permalink
Followup project-chip#27318 - Fix review comments that came in after …
Browse files Browse the repository at this point in the history 
…landing
  • Loading branch information
@vivien-apple
vivien-apple committed Jun 27, 2023
1 parent 2d78169 commit 85a18dd
Show file tree
Hide file tree
Showing 9 changed files with 417 additions and 71 deletions.
39 changes: 20 additions & 19 deletions examples/darwin-framework-tool/commands/discover/DiscoverCommissionablesCommand.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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++;
}
Expand Down
14 changes: 13 additions & 1 deletion src/darwin/Framework/CHIP/MTRCommissionableBrowser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,24 @@
NS_ASSUME_NONNULL_BEGIN

@protocol MTRCommissionableBrowserDelegate;
@class MTRDeviceController;

@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

Expand Down
117 changes: 88 additions & 29 deletions src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import "MTRCommissionableBrowser.h"
#import "MTRCommissionableBrowserDelegate.h"
#import "MTRCommissionableBrowserResult_Internal.h"
#import "MTRDeviceController.h"
#import "MTRLogging_Internal.h"

#include <controller/CHIPDeviceController.h>
Expand All @@ -37,10 +38,11 @@ @implementation MTRCommissionableBrowserResultInterfaces
@end

@interface MTRCommissionableBrowserResult ()
@property (nonatomic) NSString * serviceName;
@property (nonatomic) NSNumber * vendorId;
@property (nonatomic) NSNumber * productId;
@property (nonatomic) NSString * instanceName;
@property (nonatomic) NSNumber * vendorID;
@property (nonatomic) NSNumber * productID;
@property (nonatomic) NSNumber * discriminator;
@property (nonatomic) BOOL commissioningMode;
@end

@implementation MTRCommissionableBrowserResult
Expand All @@ -54,13 +56,17 @@ @implementation MTRCommissionableBrowserResult
#endif // CONFIG_NETWORK_LAYER_BLE
{
public:
CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, dispatch_queue_t queue)
CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceController * controller, dispatch_queue_t queue)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mDelegate == nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mController == nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mDispatchQueue == nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mDiscoveredResults == nil, CHIP_ERROR_INCORRECT_STATE);

mDelegate = delegate;
mController = controller;
mDispatchQueue = queue;
mDiscoveredResults = [[NSMutableDictionary alloc] init];

Expand All @@ -80,12 +86,19 @@ CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, dispatch_queue_t

CHIP_ERROR Stop()
{
assertChipStackLockedByCurrentThread();

VerifyOrReturnError(mDelegate != nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mController != nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mDispatchQueue != nil, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mDiscoveredResults != nil, CHIP_ERROR_INCORRECT_STATE);

mDelegate = nil;
mController = nil;
mDispatchQueue = nil;

ClearBleDiscoveredDevices();
ClearDnssdDiscoveredDevices();
mDiscoveredResults = nil;

#if CONFIG_NETWORK_LAYER_BLE
Expand All @@ -95,9 +108,38 @@ CHIP_ERROR Stop()
return ChipDnssdStopBrowse(this);
}

void ClearBleDiscoveredDevices()
{
for (NSString * key in mDiscoveredResults) {
if ([mDiscoveredResults[key].instanceName isEqual:[NSString stringWithUTF8String:kBleKey]]) {
mDiscoveredResults[key] = nil;
}
}
}

void ClearDnssdDiscoveredDevices()
{
for (NSString * key in mDiscoveredResults) {
auto * interfaces = mDiscoveredResults[key].interfaces;
for (id interfaceKey in interfaces) {
// Check if the interface data has been resolved already, otherwise, just inform the
// back end that we may not need it anymore.
if (!interfaces[interfaceKey].resolutionData.HasValue()) {
ChipDnssdResolveNoLongerNeeded(key.UTF8String);
}
}

if (![mDiscoveredResults[key].instanceName isEqual:[NSString stringWithUTF8String:kBleKey]]) {
mDiscoveredResults[key] = nil;
}
}
}

/////////// CommissioningResolveDelegate Interface /////////
void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
{
assertChipStackLockedByCurrentThread();

auto & commissionData = nodeData.commissionData;
auto key = [NSString stringWithUTF8String:commissionData.instanceName];
if ([mDiscoveredResults objectForKey:key] == nil) {
Expand All @@ -106,13 +148,14 @@ void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
}

auto result = mDiscoveredResults[key];
result.serviceName = key;
result.vendorId = @(static_cast<chip::VendorId>(commissionData.vendorId));
result.productId = @(commissionData.productId);
result.instanceName = key;
result.vendorID = @(commissionData.vendorId);
result.productID = @(commissionData.productId);
result.discriminator = @(commissionData.longDiscriminator);
result.commissioningMode = commissionData.commissioningMode != 0;

auto & resolutionData = nodeData.resolutionData;
auto interfaces = result.interfaces;
auto * interfaces = result.interfaces;
interfaces[@(resolutionData.interfaceId.GetPlatformInterface())].resolutionData = chip::MakeOptional(resolutionData);

// Check if any interface for the advertised service has been resolved already. If so,
Expand All @@ -132,20 +175,26 @@ void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
}

dispatch_async(mDispatchQueue, ^{
[mDelegate didDiscoverCommissionable:result];
[mDelegate controller:mController didFindCommissionableDevice:result];
});
}

/////////// DnssdBrowseDelegate Interface /////////
void OnBrowseAdd(DnssdService service) override
{
assertChipStackLockedByCurrentThread();

VerifyOrReturn(mDelegate != nil);
VerifyOrReturn(mController != nil);
VerifyOrReturn(mDispatchQueue != nil);

auto key = [NSString stringWithUTF8String:service.mName];
if ([mDiscoveredResults objectForKey:key] == nil) {
mDiscoveredResults[key] = [[MTRCommissionableBrowserResult alloc] init];
mDiscoveredResults[key].interfaces = [[NSMutableDictionary alloc] init];
}

auto interfaces = mDiscoveredResults[key].interfaces;
auto * interfaces = mDiscoveredResults[key].interfaces;
auto interfaceKey = @(service.mInterface.GetPlatformInterface());
interfaces[interfaceKey] = [[MTRCommissionableBrowserResultInterfaces alloc] init];

Expand All @@ -154,14 +203,20 @@ void OnBrowseAdd(DnssdService service) override

void OnBrowseRemove(DnssdService service) override
{
assertChipStackLockedByCurrentThread();

VerifyOrReturn(mDelegate != nil);
VerifyOrReturn(mController != nil);
VerifyOrReturn(mDispatchQueue != nil);

auto key = [NSString stringWithUTF8String:service.mName];
if ([mDiscoveredResults objectForKey:key] == nil) {
// It should not happens.
return;
}

auto result = mDiscoveredResults[key];
auto interfaces = result.interfaces;
auto * interfaces = result.interfaces;
auto interfaceKey = @(service.mInterface.GetPlatformInterface());

// Check if the interface data has been resolved already, otherwise, just inform the
Expand All @@ -177,7 +232,7 @@ void OnBrowseRemove(DnssdService service) override
// too and informs the delegate that it is gone.
if ([interfaces count] == 0) {
dispatch_async(mDispatchQueue, ^{
[mDelegate commissionableUnavailable:result];
[mDelegate controller:mController didRemoveCommissionableDevice:result];
});

mDiscoveredResults[key] = nil;
Expand All @@ -186,39 +241,37 @@ void OnBrowseRemove(DnssdService service) override

void OnBrowseStop(CHIP_ERROR error) override
{
for (id key in mDiscoveredResults) {
auto interfaces = mDiscoveredResults[key].interfaces;
for (id interfaceKey in interfaces) {
// Check if the interface data has been resolved already, otherwise, just inform the
// back end that we may not need it anymore.
if (!interfaces[interfaceKey].resolutionData.HasValue()) {
ChipDnssdResolveNoLongerNeeded([key UTF8String]);
}
}
}
assertChipStackLockedByCurrentThread();

ClearDnssdDiscoveredDevices();
}

#if CONFIG_NETWORK_LAYER_BLE
/////////// BleScannerDelegate Interface /////////
void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificationInfo & info) override
{
assertChipStackLockedByCurrentThread();

auto result = [[MTRCommissionableBrowserResult alloc] init];
result.serviceName = [NSString stringWithUTF8String:kBleKey];
result.vendorId = @(static_cast<chip::VendorId>(info.GetVendorId()));
result.productId = @(info.GetProductId());
result.instanceName = [NSString stringWithUTF8String:kBleKey];
result.vendorID = @(info.GetVendorId());
result.productID = @(info.GetProductId());
result.discriminator = @(info.GetDeviceDiscriminator());
result.commissioningMode = YES;
result.params = chip::MakeOptional(chip::Controller::SetUpCodePairerParameters(connObj, false /* connected */));

auto key = [NSString stringWithFormat:@"%@", connObj];
mDiscoveredResults[key] = result;

dispatch_async(mDispatchQueue, ^{
[mDelegate didDiscoverCommissionable:result];
[mDelegate controller:mController didFindCommissionableDevice:result];
});
}

void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
{
assertChipStackLockedByCurrentThread();

auto key = [NSString stringWithFormat:@"%@", connObj];
if ([mDiscoveredResults objectForKey:key] == nil) {
// It should not happens.
Expand All @@ -229,43 +282,49 @@ void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
mDiscoveredResults[key] = nil;

dispatch_async(mDispatchQueue, ^{
[mDelegate commissionableUnavailable:result];
[mDelegate controller:mController didFindCommissionableDevice:result];
});
}
#endif // CONFIG_NETWORK_LAYER_BLE

private:
dispatch_queue_t mDispatchQueue;
id<MTRCommissionableBrowserDelegate> mDelegate;
MTRDeviceController * mController;
NSMutableDictionary<NSString *, MTRCommissionableBrowserResult *> * mDiscoveredResults;
};

@interface MTRCommissionableBrowser ()
@property (strong, nonatomic) dispatch_queue_t queue;
@property (nonatomic, readonly) id<MTRCommissionableBrowserDelegate> delegate;
@property (nonatomic, readonly) MTRDeviceController * controller;
@property (unsafe_unretained, nonatomic) CommissionableBrowserInternal browser;
@end

@implementation MTRCommissionableBrowser
- (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate queue:(dispatch_queue_t)queue
- (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate
controller:(MTRDeviceController *)controller
queue:(dispatch_queue_t)queue
{
if (self = [super init]) {
_delegate = delegate;
_controller = controller;
_queue = queue;
}
return self;
}

- (BOOL)start
{
VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Start(_delegate, _queue), NO);
VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Start(_delegate, _controller, _queue), NO);
return YES;
}

- (BOOL)stop
{
VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Stop(), NO);
_delegate = nil;
_controller = nil;
_queue = nil;
return YES;
}
Expand Down
Loading

0 comments on commit 85a18dd

Please sign in to comment.