Skip to content

Commit 693ef72

Browse files
bzbarsky-applebzbarsky-apple
authored
Refactor controller startup a bit to share more code. (project-chip#28385)
There was a lot of identical boilerplate between createControllerOnNewFabric and createControllerOnExistingFabric. checkIsRunning already sets error, which is why the setting of error if that returns false was removed.
1 parent 2a681e3 commit 693ef72

File tree

1 file changed

+97
-120
lines changed

1 file changed

+97
-120
lines changed

‎src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm

+97-120
Original file line numberDiff line numberDiff line change
@@ -553,16 +553,21 @@ - (void)stopControllerFactory
553553
_running = NO;
554554
}
555555

556-
- (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceControllerStartupParams *)startupParams
557-
error:(NSError * __autoreleasing *)error
556+
/**
557+
* Helper function to start a device controller with the given startup params.
558+
* The fabricChecker block will run on the Matter queue, and is expected to
559+
* return nil if pre-startup fabric table checks fail, and set fabricError to
560+
* the right error value in that situation.
561+
*/
562+
- (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceControllerStartupParams *)startupParams
563+
fabricChecker:(MTRDeviceControllerStartupParamsInternal * (^)(
564+
FabricTable * fabricTable, CHIP_ERROR & fabricError))fabricChecker
565+
error:(NSError * __autoreleasing *)error
558566
{
559567
[self _assertCurrentQueueIsNotMatterQueue];
560568

561569
if (![self checkIsRunning:error]) {
562570
MTR_LOG_ERROR("Trying to start controller while Matter controller factory is not running");
563-
if (error != nil) {
564-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE];
565-
}
566571
return nil;
567572
}
568573

@@ -578,53 +583,14 @@ - (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceCo
578583

579584
__block MTRDeviceControllerStartupParamsInternal * params = nil;
580585
__block CHIP_ERROR fabricError = CHIP_NO_ERROR;
586+
581587
// We want the block to end up with just a pointer to the fabric table,
582588
// since we know our on-stack instance will outlive the block.
583589
FabricTable fabricTableInstance;
584590
FabricTable * fabricTable = &fabricTableInstance;
585-
dispatch_sync(_chipWorkQueue, ^{
586-
const FabricInfo * fabric = nullptr;
587-
BOOL ok = [self findMatchingFabric:*fabricTable params:startupParams fabric:&fabric];
588-
if (!ok) {
589-
MTR_LOG_ERROR("Can't start on existing fabric: fabric matching failed");
590-
fabricError = CHIP_ERROR_INTERNAL;
591-
return;
592-
}
593-
594-
if (fabric == nullptr) {
595-
MTR_LOG_ERROR("Can't start on existing fabric: fabric not found");
596-
fabricError = CHIP_ERROR_NOT_FOUND;
597-
return;
598-
}
599-
600-
os_unfair_lock_lock(&_controllersLock);
601-
NSArray<MTRDeviceController *> * controllersCopy = [_controllers copy];
602-
os_unfair_lock_unlock(&_controllersLock);
603-
604-
for (MTRDeviceController * existing in controllersCopy) {
605-
BOOL isRunning = YES; // assume the worst
606-
if ([existing isRunningOnFabric:fabricTable fabricIndex:fabric->GetFabricIndex() isRunning:&isRunning]
607-
!= CHIP_NO_ERROR) {
608-
MTR_LOG_ERROR("Can't tell what fabric a controller is running on. Not safe to start.");
609-
fabricError = CHIP_ERROR_INTERNAL;
610-
return;
611-
}
612591

613-
if (isRunning) {
614-
MTR_LOG_ERROR("Can't start on existing fabric: another controller is running on it");
615-
fabricError = CHIP_ERROR_INCORRECT_STATE;
616-
return;
617-
}
618-
}
619-
620-
params = [[MTRDeviceControllerStartupParamsInternal alloc] initForExistingFabric:fabricTable
621-
fabricIndex:fabric->GetFabricIndex()
622-
keystore:_keystore
623-
advertiseOperational:self.advertiseOperational
624-
params:startupParams];
625-
if (params == nil) {
626-
fabricError = CHIP_ERROR_NO_MEMORY;
627-
}
592+
dispatch_sync(_chipWorkQueue, ^{
593+
params = fabricChecker(fabricTable, fabricError);
628594
});
629595

630596
if (params == nil) {
@@ -653,19 +619,68 @@ - (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceCo
653619
return controller;
654620
}
655621

622+
- (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceControllerStartupParams *)startupParams
623+
error:(NSError * __autoreleasing *)error
624+
{
625+
[self _assertCurrentQueueIsNotMatterQueue];
626+
627+
return [self
628+
_startDeviceController:startupParams
629+
fabricChecker:^MTRDeviceControllerStartupParamsInternal *(FabricTable * fabricTable, CHIP_ERROR & fabricError) {
630+
const FabricInfo * fabric = nullptr;
631+
BOOL ok = [self findMatchingFabric:*fabricTable params:startupParams fabric:&fabric];
632+
if (!ok) {
633+
MTR_LOG_ERROR("Can't start on existing fabric: fabric matching failed");
634+
fabricError = CHIP_ERROR_INTERNAL;
635+
return nil;
636+
}
637+
638+
if (fabric == nullptr) {
639+
MTR_LOG_ERROR("Can't start on existing fabric: fabric not found");
640+
fabricError = CHIP_ERROR_NOT_FOUND;
641+
return nil;
642+
}
643+
644+
os_unfair_lock_lock(&self->_controllersLock);
645+
NSArray<MTRDeviceController *> * controllersCopy = [self->_controllers copy];
646+
os_unfair_lock_unlock(&self->_controllersLock);
647+
648+
for (MTRDeviceController * existing in controllersCopy) {
649+
BOOL isRunning = YES; // assume the worst
650+
if ([existing isRunningOnFabric:fabricTable fabricIndex:fabric->GetFabricIndex() isRunning:&isRunning]
651+
!= CHIP_NO_ERROR) {
652+
MTR_LOG_ERROR("Can't tell what fabric a controller is running on. Not safe to start.");
653+
fabricError = CHIP_ERROR_INTERNAL;
654+
return nil;
655+
}
656+
657+
if (isRunning) {
658+
MTR_LOG_ERROR("Can't start on existing fabric: another controller is running on it");
659+
fabricError = CHIP_ERROR_INCORRECT_STATE;
660+
return nil;
661+
}
662+
}
663+
664+
auto * params =
665+
[[MTRDeviceControllerStartupParamsInternal alloc] initForExistingFabric:fabricTable
666+
fabricIndex:fabric->GetFabricIndex()
667+
keystore:self->_keystore
668+
advertiseOperational:self.advertiseOperational
669+
params:startupParams];
670+
if (params == nil) {
671+
fabricError = CHIP_ERROR_NO_MEMORY;
672+
}
673+
674+
return params;
675+
}
676+
error:error];
677+
}
678+
656679
- (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControllerStartupParams *)startupParams
657680
error:(NSError * __autoreleasing *)error
658681
{
659682
[self _assertCurrentQueueIsNotMatterQueue];
660683

661-
if (![self isRunning]) {
662-
MTR_LOG_ERROR("Trying to start controller while Matter controller factory is not running");
663-
if (error != nil) {
664-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE];
665-
}
666-
return nil;
667-
}
668-
669684
if (startupParams.vendorID == nil) {
670685
MTR_LOG_ERROR("Must provide vendor id when starting controller on new fabric");
671686
if (error != nil) {
@@ -682,71 +697,33 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl
682697
return nil;
683698
}
684699

685-
// Create the controller, so we start the event loop, since we plan to do
686-
// our fabric table operations there.
687-
auto * controller = [self createController];
688-
if (controller == nil) {
689-
if (error != nil) {
690-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY];
691-
}
692-
return nil;
693-
}
694-
695-
__block MTRDeviceControllerStartupParamsInternal * params = nil;
696-
__block CHIP_ERROR fabricError = CHIP_NO_ERROR;
697-
// We want the block to end up with just a pointer to the fabric table,
698-
// since we know our on-stack instance will outlive the block.
699-
FabricTable fabricTableInstance;
700-
FabricTable * fabricTable = &fabricTableInstance;
701-
dispatch_sync(_chipWorkQueue, ^{
702-
const FabricInfo * fabric = nullptr;
703-
BOOL ok = [self findMatchingFabric:*fabricTable params:startupParams fabric:&fabric];
704-
if (!ok) {
705-
MTR_LOG_ERROR("Can't start on new fabric: fabric matching failed");
706-
fabricError = CHIP_ERROR_INTERNAL;
707-
return;
708-
}
709-
710-
if (fabric != nullptr) {
711-
MTR_LOG_ERROR("Can't start on new fabric that matches existing fabric");
712-
fabricError = CHIP_ERROR_INCORRECT_STATE;
713-
return;
714-
}
715-
716-
params = [[MTRDeviceControllerStartupParamsInternal alloc] initForNewFabric:fabricTable
717-
keystore:_keystore
718-
advertiseOperational:self.advertiseOperational
719-
params:startupParams];
720-
if (params == nil) {
721-
fabricError = CHIP_ERROR_NO_MEMORY;
722-
}
723-
});
724-
725-
if (params == nil) {
726-
[self controllerShuttingDown:controller];
727-
if (error != nil) {
728-
*error = [MTRError errorForCHIPErrorCode:fabricError];
729-
}
730-
return nil;
731-
}
732-
733-
BOOL ok = [controller startup:params];
734-
if (ok == NO) {
735-
// TODO: get error from controller's startup.
736-
if (error != nil) {
737-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL];
738-
}
739-
return nil;
740-
}
741-
742-
// TODO: Need better error propagation.
743-
controller = [self maybeInitializeOTAProvider:controller];
744-
if (controller == nil) {
745-
if (error != nil) {
746-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL];
747-
}
748-
}
749-
return controller;
700+
return [self
701+
_startDeviceController:startupParams
702+
fabricChecker:^MTRDeviceControllerStartupParamsInternal *(FabricTable * fabricTable, CHIP_ERROR & fabricError) {
703+
const FabricInfo * fabric = nullptr;
704+
BOOL ok = [self findMatchingFabric:*fabricTable params:startupParams fabric:&fabric];
705+
if (!ok) {
706+
MTR_LOG_ERROR("Can't start on new fabric: fabric matching failed");
707+
fabricError = CHIP_ERROR_INTERNAL;
708+
return nil;
709+
}
710+
711+
if (fabric != nullptr) {
712+
MTR_LOG_ERROR("Can't start on new fabric that matches existing fabric");
713+
fabricError = CHIP_ERROR_INCORRECT_STATE;
714+
return nil;
715+
}
716+
717+
auto * params = [[MTRDeviceControllerStartupParamsInternal alloc] initForNewFabric:fabricTable
718+
keystore:self->_keystore
719+
advertiseOperational:self.advertiseOperational
720+
params:startupParams];
721+
if (params == nil) {
722+
fabricError = CHIP_ERROR_NO_MEMORY;
723+
}
724+
return params;
725+
}
726+
error:error];
750727
}
751728

752729
- (MTRDeviceController * _Nullable)createController

0 commit comments

Comments
 (0)