Skip to content

Commit

Permalink
Address review comment.
Browse files Browse the repository at this point in the history
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed Sep 5, 2023
1 parent f17493f commit c77c209
Showing 1 changed file with 28 additions and 58 deletions.
86 changes: 28 additions & 58 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,11 +545,10 @@ - (void)stopControllerFactory
* return nil if pre-startup fabric table checks fail, and set fabricError to
* the right error value in that situation.
*
* existingController can be provided if the controller object has already been
* allocated but not yet initialized. If not provided, a new controller object
* will be created.
* The provided controller is expected to have just been allocated and to not be
* initialized yet.
*/
- (MTRDeviceController * _Nullable)_startDeviceController:(nullable MTRDeviceController *)existingController
- (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *)controller
startupParams:(id)startupParams
fabricChecker:(MTRDeviceControllerStartupParamsInternal * (^)(FabricTable * fabricTable,
MTRDeviceController * controller,
Expand Down Expand Up @@ -610,21 +609,35 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(nullable MTRDeviceCon
otaProviderDelegateQueue = self.otaProviderDelegateQueue;
}

// Create the controller, so we start the event loop, since we plan to do
// our fabric table operations there.
auto * controller = [self _createController:existingController
storageDelegate:storageDelegate
storageDelegateQueue:storageDelegateQueue
otaProviderDelegate:otaProviderDelegate
otaProviderDelegateQueue:otaProviderDelegateQueue
uniqueIdentifier:uniqueIdentifier];
controller = [controller initWithFactory:self
queue:_chipWorkQueue
storageDelegate:storageDelegate
storageDelegateQueue:storageDelegateQueue
otaProviderDelegate:otaProviderDelegate
otaProviderDelegateQueue:otaProviderDelegateQueue
uniqueIdentifier:uniqueIdentifier];
if (controller == nil) {
if (error != nil) {
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY];
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
}
return nil;
}

if ([_controllers count] == 0) {
// Bringing up the first controller. Start the event loop now. If we
// fail to bring it up, its cleanup will stop the event loop again.
chip::DeviceLayer::PlatformMgrImpl().StartEventLoopTask();
dispatch_sync(_chipWorkQueue, ^{
self->_operationalBrowser = new MTROperationalBrowser(self, self->_chipWorkQueue);
});
}

// Add the controller to _controllers now, so if we fail partway through its
// startup we will still do the right cleanups.
os_unfair_lock_lock(&_controllersLock);
[_controllers addObject:controller];
os_unfair_lock_unlock(&_controllersLock);

__block MTRDeviceControllerStartupParamsInternal * params = nil;
__block CHIP_ERROR fabricError = CHIP_NO_ERROR;

Expand Down Expand Up @@ -719,7 +732,7 @@ - (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceCo
return nil;
}

return [self _startDeviceController:nil
return [self _startDeviceController:[MTRDeviceController alloc]
startupParams:startupParams
fabricChecker:^MTRDeviceControllerStartupParamsInternal *(
FabricTable * fabricTable, MTRDeviceController * controller, CHIP_ERROR & fabricError) {
Expand Down Expand Up @@ -796,7 +809,7 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl
return nil;
}

return [self _startDeviceController:nil
return [self _startDeviceController:[MTRDeviceController alloc]
startupParams:startupParams
fabricChecker:^MTRDeviceControllerStartupParamsInternal *(
FabricTable * fabricTable, MTRDeviceController * controller, CHIP_ERROR & fabricError) {
Expand Down Expand Up @@ -830,49 +843,6 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl
error:error];
}

- (MTRDeviceController * _Nullable)_createController:(nullable MTRDeviceController *)existingController
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
{
[self _assertCurrentQueueIsNotMatterQueue];

MTRDeviceController * controller = existingController;
if (controller == nil) {
controller = [MTRDeviceController alloc];
}
controller = [controller initWithFactory:self
queue:_chipWorkQueue
storageDelegate:storageDelegate
storageDelegateQueue:storageDelegateQueue
otaProviderDelegate:otaProviderDelegate
otaProviderDelegateQueue:otaProviderDelegateQueue
uniqueIdentifier:uniqueIdentifier];
if (controller == nil) {
MTR_LOG_ERROR("Failed to init controller");
return nil;
}

if ([_controllers count] == 0) {
// Bringing up the first controller. Start the event loop now. If we
// fail to bring it up, its cleanup will stop the event loop again.
chip::DeviceLayer::PlatformMgrImpl().StartEventLoopTask();
dispatch_sync(_chipWorkQueue, ^{
self->_operationalBrowser = new MTROperationalBrowser(self, self->_chipWorkQueue);
});
}

// Add the controller to _controllers now, so if we fail partway through its
// startup we will still do the right cleanups.
os_unfair_lock_lock(&_controllersLock);
[_controllers addObject:controller];
os_unfair_lock_unlock(&_controllersLock);

return controller;
}

// Finds a fabric that matches the given params, if one exists.
//
// Returns NO on failure, YES on success. If YES is returned, the
Expand Down

0 comments on commit c77c209

Please sign in to comment.