RSDK-6925 Prefactors (viamrobotics#3961)

maximpertsov · May 16, 2024 · f0f6852 · f0f6852
1 parent ba1b7b7
commit f0f6852
Showing 1 changed file with 68 additions and 64 deletions.
diff --git a/robot/impl/resource_manager.go b/robot/impl/resource_manager.go
@@ -518,7 +518,7 @@ func (manager *resourceManager) Close(ctx context.Context) error {
 // or reconfigure resources that are wrapped in a placeholderResource.
 func (manager *resourceManager) completeConfig(
 	ctx context.Context,
-	robot *localRobot,
+	lr *localRobot,
 ) {
 	manager.configLock.Lock()
 	defer func() {
@@ -529,65 +529,7 @@ func (manager *resourceManager) completeConfig(
 	}()
 
 	// first handle remotes since they may reveal unresolved dependencies
-	for _, resName := range manager.resources.FindNodesByAPI(client.RemoteAPI) {
-		gNode, ok := manager.resources.Node(resName)
-		if !ok || !gNode.NeedsReconfigure() {
-			continue
-		}
-		var verb string
-		if gNode.IsUninitialized() {
-			verb = "configuring"
-		} else {
-			verb = "reconfiguring"
-		}
-		manager.logger.CInfow(ctx, fmt.Sprintf("Now %s a remote", verb), "resource", resName)
-		switch resName.API {
-		case client.RemoteAPI:
-			remConf, err := resource.NativeConfig[*config.Remote](gNode.Config())
-			if err != nil {
-				manager.logger.CErrorw(ctx,
-					"remote config error",
-					"error",
-					err,
-				)
-				continue
-			}
-			if gNode.IsUninitialized() {
-				gNode.InitializeLogger(
-					manager.logger, fromRemoteNameToRemoteNodeName(remConf.Name).String(), manager.logger.GetLevel(),
-				)
-			}
-			// this is done in config validation but partial start rules require us to check again
-			if _, err := remConf.Validate(""); err != nil {
-				gNode.LogAndSetLastError(
-					fmt.Errorf("remote config validation error: %w", err), "remote", remConf.Name)
-				continue
-			}
-			rr, err := manager.processRemote(ctx, *remConf, gNode)
-			if err != nil {
-				gNode.LogAndSetLastError(
-					fmt.Errorf("error connecting to remote: %w", err), "remote", remConf.Name)
-				continue
-			}
-			manager.addRemote(ctx, rr, gNode, *remConf)
-			rr.SetParentNotifier(func() {
-				if robot.closeContext.Err() != nil {
-					return
-				}
-
-				// Trigger completeConfig goroutine execution when a change in remote
-				// is detected.
-				select {
-				case <-robot.closeContext.Done():
-					return
-				case robot.triggerConfig <- struct{}{}:
-				}
-			})
-		default:
-			err := errors.New("config is not a remote config")
-			manager.logger.CErrorw(ctx, err.Error(), "resource", resName)
-		}
-	}
+	manager.completeConfigForRemotes(ctx, lr)
 
 	// now resolve prior to sorting in case there's anything newly discovered
 	if err := manager.resources.ResolveDependencies(manager.logger); err != nil {
@@ -612,12 +554,12 @@ func (manager *resourceManager) completeConfig(
 		cleanup := rutils.SlowStartupLogger(
 			ctx, "Waiting for resource to complete (re)configuration", "resource", resName.String(), manager.logger)
 
-		robot.reconfigureWorkers.Add(1)
+		lr.reconfigureWorkers.Add(1)
 		goutils.PanicCapturingGo(func() {
 			defer func() {
 				cleanup()
 				resChan <- struct{}{}
-				robot.reconfigureWorkers.Done()
+				lr.reconfigureWorkers.Done()
 			}()
 			gNode, ok := manager.resources.Node(resName)
 			if !ok || !gNode.NeedsReconfigure() {
@@ -659,7 +601,7 @@ func (manager *resourceManager) completeConfig(
 
 			switch {
 			case resName.API.IsComponent(), resName.API.IsService():
-				newRes, newlyBuilt, err := manager.processResource(ctxWithTimeout, conf, gNode, robot)
+				newRes, newlyBuilt, err := manager.processResource(ctxWithTimeout, conf, gNode, lr)
 				if newlyBuilt || err != nil {
 					if err := manager.markChildrenForUpdate(resName); err != nil {
 						manager.logger.CErrorw(ctx,
@@ -698,14 +640,76 @@ func (manager *resourceManager) completeConfig(
 		case <-resChan:
 		case <-ctxWithTimeout.Done():
 			if errors.Is(ctxWithTimeout.Err(), context.DeadlineExceeded) {
-				robot.logger.CWarn(ctx, rutils.NewBuildTimeoutError(resName.String()))
+				lr.logger.CWarn(ctx, rutils.NewBuildTimeoutError(resName.String()))
 			}
 		case <-ctx.Done():
 			return
 		}
 	} // for-each resource name
 }
 
+func (manager *resourceManager) completeConfigForRemotes(ctx context.Context, lr *localRobot) {
+	for _, resName := range manager.resources.FindNodesByAPI(client.RemoteAPI) {
+		gNode, ok := manager.resources.Node(resName)
+		if !ok || !gNode.NeedsReconfigure() {
+			continue
+		}
+		var verb string
+		if gNode.IsUninitialized() {
+			verb = "configuring"
+		} else {
+			verb = "reconfiguring"
+		}
+		manager.logger.CInfow(ctx, fmt.Sprintf("Now %s a remote", verb), "resource", resName)
+		switch resName.API {
+		case client.RemoteAPI:
+			remConf, err := resource.NativeConfig[*config.Remote](gNode.Config())
+			if err != nil {
+				manager.logger.CErrorw(ctx,
+					"remote config error",
+					"error",
+					err,
+				)
+				continue
+			}
+			if gNode.IsUninitialized() {
+				gNode.InitializeLogger(
+					manager.logger, fromRemoteNameToRemoteNodeName(remConf.Name).String(), manager.logger.GetLevel(),
+				)
+			}
+			// this is done in config validation but partial start rules require us to check again
+			if _, err := remConf.Validate(""); err != nil {
+				gNode.LogAndSetLastError(
+					fmt.Errorf("remote config validation error: %w", err), "remote", remConf.Name)
+				continue
+			}
+			rr, err := manager.processRemote(ctx, *remConf, gNode)
+			if err != nil {
+				gNode.LogAndSetLastError(
+					fmt.Errorf("error connecting to remote: %w", err), "remote", remConf.Name)
+				continue
+			}
+			manager.addRemote(ctx, rr, gNode, *remConf)
+			rr.SetParentNotifier(func() {
+				if lr.closeContext.Err() != nil {
+					return
+				}
+
+				// Trigger completeConfig goroutine execution when a change in remote
+				// is detected.
+				select {
+				case <-lr.closeContext.Done():
+					return
+				case lr.triggerConfig <- struct{}{}:
+				}
+			})
+		default:
+			err := errors.New("config is not a remote config")
+			manager.logger.CErrorw(ctx, err.Error(), "resource", resName)
+		}
+	}
+}
+
 // cleanAppImageEnv attempts to revert environment variable changes so
 // normal, non-AppImage processes can be executed correctly.
 func cleanAppImageEnv() error {