Skip to content

Commit

Permalink
Address API review issues in MTRControllerFactory.
Browse files Browse the repository at this point in the history
* Rename to MTRDeviceControllerFactory.
* Change the startup params startServer to shouldStartServer.
* Change the startup params init signatures to be more aligned.
* Change isRunning to running.
* Rename startup to startControllerFactory and add NSError outparam.
* Rename shutdown to stopControllerFactory
* Rename startControllerOnExistingFabric to
  createControllerOnExistingFabric and add NSError outparam.
* Rename startControllerOnNewFabric to createControllerOnNewFabric and
  add NSError outparam.

Fixes #22594

Addresses part of #22420
  • Loading branch information
bzbarsky-apple committed Sep 14, 2022
1 parent 9c131da commit f4058e2
Show file tree
Hide file tree
Showing 19 changed files with 451 additions and 415 deletions.
31 changes: 14 additions & 17 deletions examples/darwin-framework-tool/commands/common/CHIPCommandBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -111,25 +111,26 @@

mOTADelegate = [[OTAProviderDelegate alloc] init];

auto factory = [MTRControllerFactory sharedInstance];
auto factory = [MTRDeviceControllerFactory sharedInstance];
if (factory == nil) {
ChipLogError(chipTool, "Controller factory is nil");
return CHIP_ERROR_INTERNAL;
}

auto params = [[MTRControllerFactoryParams alloc] initWithStorage:storage];
auto params = [[MTRDeviceControllerFactoryParams alloc] initWithStorage:storage];
params.port = @(kListenPort);
params.startServer = YES;
params.shouldStartServer = YES;
params.otaProviderDelegate = mOTADelegate;
NSArray<NSData *> * paaCertResults;
ReturnLogErrorOnFailure(GetPAACertsFromFolder(&paaCertResults));
if ([paaCertResults count] > 0) {
params.paaCerts = paaCertResults;
}

if ([factory startup:params] == NO) {
NSError * error;
if ([factory startControllerFactory:params error:&error] == NO) {
ChipLogError(chipTool, "Controller factory startup failed");
return CHIP_ERROR_INTERNAL;
return MTRErrorToCHIPErrorCode(error);
}

ReturnLogErrorOnFailure([gNocSigner createOrLoadKeys:storage]);
Expand All @@ -138,21 +139,19 @@

constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
for (size_t i = 0; i < ArraySize(identities); ++i) {
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:gNocSigner
fabricID:@(i + 1)
ipk:ipk];
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithIPK:ipk fabricID:@(i + 1) nocSigner:gNocSigner];

// We're not sure whether we're creating a new fabric or using an
// existing one, so just try both.
auto controller = [factory startControllerOnExistingFabric:controllerParams];
auto controller = [factory createControllerOnExistingFabric:controllerParams error:&error];
if (controller == nil) {
// Maybe we didn't have this fabric yet.
controllerParams.vendorID = @(chip::VendorId::TestVendor1);
controller = [factory startControllerOnNewFabric:controllerParams];
controller = [factory createControllerOnNewFabric:controllerParams error:&error];
}
if (controller == nil) {
ChipLogError(chipTool, "Controller startup failure.");
return CHIP_ERROR_INTERNAL;
return MTRErrorToCHIPErrorCode(error);
}

mControllers[identities[i]] = controller;
Expand Down Expand Up @@ -195,16 +194,14 @@
{
StopCommissioners();

auto factory = [MTRControllerFactory sharedInstance];
auto factory = [MTRDeviceControllerFactory sharedInstance];
NSData * ipk = [gNocSigner getIPK];

constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
for (size_t i = 0; i < ArraySize(identities); ++i) {
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:gNocSigner
fabricID:@(i + 1)
ipk:ipk];
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithIPK:ipk fabricID:@(i + 1) nocSigner:gNocSigner];

auto controller = [factory startControllerOnExistingFabric:controllerParams];
auto controller = [factory createControllerOnExistingFabric:controllerParams error:nil];
mControllers[identities[i]] = controller;
}
}
Expand All @@ -216,7 +213,7 @@
mControllers.clear();
mCurrentController = nil;

[[MTRControllerFactory sharedInstance] shutdown];
[[MTRDeviceControllerFactory sharedInstance] stopControllerFactory];
}

CHIP_ERROR CHIPCommandBridge::StartWaiting(chip::System::Clock::Timeout duration)
Expand Down
20 changes: 11 additions & 9 deletions examples/darwin-framework-tool/main.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@

int main(int argc, const char * argv[])
{
Commands commands;
registerCommandsPairing(commands);
registerCommandsInteractive(commands);
registerCommandsPayload(commands);
registerClusterOtaSoftwareUpdateProviderInteractive(commands);
registerCommandsStorage(commands);
registerCommandsTests(commands);
registerClusters(commands);
return commands.Run(argc, (char **) argv);
@autoreleasepool {
Commands commands;
registerCommandsPairing(commands);
registerCommandsInteractive(commands);
registerCommandsPayload(commands);
registerClusterOtaSoftwareUpdateProviderInteractive(commands);
registerCommandsStorage(commands);
registerCommandsTests(commands);
registerClusters(commands);
return commands.Run(argc, (char **) argv);
}
}
16 changes: 8 additions & 8 deletions src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ void MTRSetNextAvailableDeviceID(uint64_t id)
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
CHIPToolPersistentStorageDelegate * storage = [[CHIPToolPersistentStorageDelegate alloc] init];
__auto_type * factory = [MTRControllerFactory sharedInstance];
__auto_type * factoryParams = [[MTRControllerFactoryParams alloc] initWithStorage:storage];
if (![factory startup:factoryParams]) {
__auto_type * factory = [MTRDeviceControllerFactory sharedInstance];
__auto_type * factoryParams = [[MTRDeviceControllerFactoryParams alloc] initWithStorage:storage];
if (![factory startControllerFactory:factoryParams error:nil]) {
return;
}

Expand All @@ -87,14 +87,14 @@ void MTRSetNextAvailableDeviceID(uint64_t id)
return;
}

__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:keys fabricID:@(1) ipk:keys.ipk];
__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:keys.ipk fabricID:@(1) nocSigner:keys];
params.vendorID = @(kTestVendorId);

// We're not sure whether we have a fabric configured already; try as if
// we did, and if not fall back to creating a new one.
sController = [factory startControllerOnExistingFabric:params];
sController = [factory createControllerOnExistingFabric:params error:nil];
if (sController == nil) {
sController = [factory startControllerOnNewFabric:params];
sController = [factory createControllerOnNewFabric:params error:nil];
}
});

Expand All @@ -113,9 +113,9 @@ void MTRSetNextAvailableDeviceID(uint64_t id)
[controller shutdown];

NSLog(@"Starting up the stack");
__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:keys fabricID:@(1) ipk:keys.ipk];
__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:keys.ipk fabricID:@(1) nocSigner:keys];

sController = [[MTRControllerFactory sharedInstance] startControllerOnExistingFabric:params];
sController = [[MTRDeviceControllerFactory sharedInstance] createControllerOnExistingFabric:params error:nil];

return sController;
}
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, readonly) chip::NodeId nodeID;

/**
* Controllers are created via the MTRControllerFactory object.
* Controllers are created via the MTRDeviceControllerFactory object.
*/
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS

@interface MTRDeviceController : NSObject

@property (readonly, nonatomic) BOOL isRunning;
@property (readonly, nonatomic, getter=isRunning) BOOL running;

/**
* Return the Node ID assigned to the controller. Will return nil if the
Expand Down Expand Up @@ -96,7 +96,7 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
- (BOOL)getBaseDevice:(uint64_t)deviceID queue:(dispatch_queue_t)queue completion:(MTRDeviceConnectionCallback)completion;

/**
* Controllers are created via the MTRControllerFactory object.
* Controllers are created via the MTRDeviceControllerFactory object.
*/
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
Expand Down
8 changes: 4 additions & 4 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

#import "MTRBaseDevice_Internal.h"
#import "MTRCommissioningParameters.h"
#import "MTRControllerFactory_Internal.h"
#import "MTRDeviceControllerFactory_Internal.h"
#import "MTRDeviceControllerStartupParams.h"
#import "MTRDeviceControllerStartupParams_Internal.h"
#import "MTRDevicePairingDelegateBridge.h"
Expand Down Expand Up @@ -86,14 +86,14 @@ @interface MTRDeviceController ()
@property (readonly) MTRP256KeypairBridge signingKeypairBridge;
@property (readonly) MTRP256KeypairBridge operationalKeypairBridge;
@property (readonly) MTRDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge;
@property (readonly) MTRControllerFactory * factory;
@property (readonly) MTRDeviceControllerFactory * factory;
@property (readonly) NSMutableDictionary * nodeIDToDeviceMap;
@property (readonly) os_unfair_lock deviceMapLock; // protects nodeIDToDeviceMap
@end

@implementation MTRDeviceController

- (instancetype)initWithFactory:(MTRControllerFactory *)factory queue:(dispatch_queue_t)queue
- (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory queue:(dispatch_queue_t)queue
{
if (self = [super init]) {
_chipWorkQueue = queue;
Expand Down Expand Up @@ -143,7 +143,7 @@ - (void)cleanupAfterStartup
}

// Part of cleanupAfterStartup that has to interact with the Matter work queue
// in a very specific way that only MTRControllerFactory knows about.
// in a very specific way that only MTRDeviceControllerFactory knows about.
- (void)shutDownCppController
{
if (_cppCommissioner) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ NS_ASSUME_NONNULL_BEGIN
@class MTRDeviceController;
@class MTRDeviceControllerStartupParams;

@interface MTRControllerFactoryParams : NSObject
@interface MTRDeviceControllerFactoryParams : NSObject
/*
* Storage delegate must be provided for correct functioning of Matter
* controllers. It is used to store persistent information for the fabrics the
Expand Down Expand Up @@ -66,44 +66,47 @@ NS_ASSUME_NONNULL_BEGIN
* Whether to run a server capable of accepting incoming CASE
* connections. Defaults to NO.
*/
@property (nonatomic, assign) BOOL startServer;
@property (nonatomic, assign) BOOL shouldStartServer;

- (instancetype)init NS_UNAVAILABLE;
- (instancetype)initWithStorage:(id<MTRPersistentStorageDelegate>)storageDelegate;
@end

@interface MTRControllerFactory : NSObject
@interface MTRDeviceControllerFactory : NSObject

@property (readonly, nonatomic) BOOL isRunning;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
/**
* If true, the factory is in a state where it can create controllers:
* startControllerFactory has been called, but stopControllerFactory has not been called
* since then.
*/
@property (readonly, nonatomic, getter=isRunning) BOOL running;

/**
* Return the single MTRControllerFactory we support existing. It starts off
* Return the single MTRDeviceControllerFactory we support existing. It starts off
* in a "not started" state.
*/
+ (instancetype)sharedInstance;

/**
* Start the controller factory. Repeated calls to startup without calls to
* shutdown in between are NO-OPs. Use the isRunning property to check whether
* the controller factory needs to be started up.
* Start the controller factory. Repeated calls to startControllerFactory
* without calls to stopControllerFactory in between are NO-OPs. Use the
* isRunning property to check whether the controller factory needs to be
* started up.
*
* @param[in] startupParams data needed to start up the controller factory.
*
* @return Whether startup succeded.
*/
- (BOOL)startup:(MTRControllerFactoryParams *)startupParams;
- (BOOL)startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParams error:(NSError * __autoreleasing *)error;

/**
* Shut down the controller factory. This will shut down any outstanding
* controllers as part of the factory shutdown.
* Stop the controller factory. This will shut down any outstanding
* controllers as part of the factory stopping.
*
* Repeated calls to shutdown without calls to startup in between are
* NO-OPs.
* Repeated calls to stopControllerFactory without calls to
* startControllerFactory in between are NO-OPs.
*/
- (void)shutdown;
- (void)stopControllerFactory;

/**
* Create a MTRDeviceController on an existing fabric. Returns nil on failure.
Expand All @@ -114,7 +117,8 @@ NS_ASSUME_NONNULL_BEGIN
* The fabric is identified by the root public key and fabric id in
* the startupParams.
*/
- (MTRDeviceController * _Nullable)startControllerOnExistingFabric:(MTRDeviceControllerStartupParams *)startupParams;
- (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceControllerStartupParams *)startupParams
error:(NSError * __autoreleasing *)error;

/**
* Create a MTRDeviceController on a new fabric. Returns nil on failure.
Expand All @@ -124,7 +128,11 @@ NS_ASSUME_NONNULL_BEGIN
* The fabric is identified by the root public key and fabric id in
* the startupParams.
*/
- (MTRDeviceController * _Nullable)startControllerOnNewFabric:(MTRDeviceControllerStartupParams *)startupParams;
- (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControllerStartupParams *)startupParams
error:(NSError * __autoreleasing *)error;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

@end

Expand Down
Loading

0 comments on commit f4058e2

Please sign in to comment.