From 2e59239f8336d80d7b7afd3713db8f37273a7863 Mon Sep 17 00:00:00 2001 From: Ray Bjorkman Date: Thu, 10 Apr 2025 14:24:15 -0400 Subject: [PATCH 1/3] move inject/motion_service.go to its own package --- protoutils/messages.go | 2 +- robot/impl/resource_manager_test.go | 8 ------ services/motion/client_test.go | 4 +-- services/motion/motion_helpers_test.go | 3 ++- services/motion/server_test.go | 16 ++++++------ services/navigation/builtin/builtin_test.go | 25 ++++++++++--------- .../inject/{ => motion}/motion_service.go | 1 + 7 files changed, 27 insertions(+), 32 deletions(-) rename testutils/inject/{ => motion}/motion_service.go (97%) diff --git a/protoutils/messages.go b/protoutils/messages.go index 9376031d273..64d9a339cb5 100644 --- a/protoutils/messages.go +++ b/protoutils/messages.go @@ -6,7 +6,7 @@ import ( "strconv" "github.com/golang/geo/r3" - protov1 "github.com/golang/protobuf/proto" //nolint:staticcheck + protov1 "github.com/golang/protobuf/proto" commonpb "go.viam.com/api/common/v1" robotpb "go.viam.com/api/robot/v1" "go.viam.com/utils/protoutils" diff --git a/robot/impl/resource_manager_test.go b/robot/impl/resource_manager_test.go index 95cdf4326bd..42bef8847d3 100644 --- a/robot/impl/resource_manager_test.go +++ b/robot/impl/resource_manager_test.go @@ -59,7 +59,6 @@ import ( "go.viam.com/rdk/robot/framesystem" "go.viam.com/rdk/robot/packages" weboptions "go.viam.com/rdk/robot/web/options" - "go.viam.com/rdk/services/motion" "go.viam.com/rdk/services/shell" "go.viam.com/rdk/services/vision" "go.viam.com/rdk/session" @@ -462,13 +461,6 @@ func TestManagerAdd(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, resource1, test.ShouldEqual, injectBoard) - injectMotionService := &inject.MotionService{} - objectMResName := motion.Named("motion1") - manager.resources.AddNode(objectMResName, resource.NewConfiguredGraphNode(resource.Config{}, injectMotionService, unknownModel)) - motionService, err := manager.ResourceByName(objectMResName) - test.That(t, err, test.ShouldBeNil) - test.That(t, motionService, test.ShouldEqual, injectMotionService) - injectVisionService := &inject.VisionService{} injectVisionService.GetObjectPointCloudsFunc = func( ctx context.Context, diff --git a/services/motion/client_test.go b/services/motion/client_test.go index abf06b9afca..f7bcae7aeef 100644 --- a/services/motion/client_test.go +++ b/services/motion/client_test.go @@ -27,7 +27,7 @@ import ( "go.viam.com/rdk/services/slam" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils" - "go.viam.com/rdk/testutils/inject" + inject "go.viam.com/rdk/testutils/inject/motion" ) var ( @@ -43,7 +43,7 @@ func TestClient(t *testing.T) { rpcServer, err := rpc.NewServer(logger, rpc.WithUnauthenticated()) test.That(t, err, test.ShouldBeNil) - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } diff --git a/services/motion/motion_helpers_test.go b/services/motion/motion_helpers_test.go index f1274b64ddd..db7526201ca 100644 --- a/services/motion/motion_helpers_test.go +++ b/services/motion/motion_helpers_test.go @@ -18,11 +18,12 @@ import ( "go.viam.com/rdk/services/motion" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils/inject" + injectmotion "go.viam.com/rdk/testutils/inject/motion" ) func TestPollHistoryUntilSuccessOrError(t *testing.T) { ctx := context.Background() - ms := inject.NewMotionService("my motion") + ms := injectmotion.NewMotionService("my motion") t.Run("returns error if context is cancelled", func(t *testing.T) { cancelledCtx, cancelFn := context.WithCancel(context.Background()) cancelFn() diff --git a/services/motion/server_test.go b/services/motion/server_test.go index fe95b0c5bbc..bb54e5477b7 100644 --- a/services/motion/server_test.go +++ b/services/motion/server_test.go @@ -28,7 +28,7 @@ import ( "go.viam.com/rdk/services/vision" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils" - "go.viam.com/rdk/testutils/inject" + inject "go.viam.com/rdk/testutils/inject/motion" ) func newServer(resources map[resource.Name]motion.Service) (pb.MotionServiceServer, error) { @@ -53,7 +53,7 @@ func TestServerMove(t *testing.T) { test.That(t, err, test.ShouldBeError, errors.New("resource \"rdk:service:motion/motion1\" not found")) // error - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources = map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -76,7 +76,7 @@ func TestServerMove(t *testing.T) { test.That(t, resp.GetSuccess(), test.ShouldBeTrue) // Multiple Servies names Valid - injectMS = &inject.MotionService{} + injectMS = inject.NewMotionService("test") resources = map[resource.Name]motion.Service{ testMotionServiceName: injectMS, testMotionServiceName2: injectMS, @@ -97,7 +97,7 @@ func TestServerMove(t *testing.T) { } func TestServerMoveOnGlobe(t *testing.T) { - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -279,7 +279,7 @@ func TestServerMoveOnGlobe(t *testing.T) { } func TestServerMoveOnMap(t *testing.T) { - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -450,7 +450,7 @@ func TestServerMoveOnMap(t *testing.T) { } func TestServerStopPlan(t *testing.T) { - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -515,7 +515,7 @@ func TestServerStopPlan(t *testing.T) { } func TestServerListPlanStatuses(t *testing.T) { - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -621,7 +621,7 @@ func TestServerListPlanStatuses(t *testing.T) { } func TestServerGetPlan(t *testing.T) { - injectMS := &inject.MotionService{} + injectMS := inject.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } diff --git a/services/navigation/builtin/builtin_test.go b/services/navigation/builtin/builtin_test.go index 66bfe173347..6a89427f9c4 100644 --- a/services/navigation/builtin/builtin_test.go +++ b/services/navigation/builtin/builtin_test.go @@ -37,12 +37,13 @@ import ( _ "go.viam.com/rdk/services/vision/colordetector" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils/inject" + injectmotion "go.viam.com/rdk/testutils/inject/motion" viz "go.viam.com/rdk/vision" ) type startWaypointState struct { ns navigation.Service - injectMS *inject.MotionService + injectMS *injectmotion.MotionService base base.Base movementSensor *inject.MovementSensor closeFunc func() @@ -230,7 +231,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): inject.NewBase("new_base"), - resource.NewName(motion.API, "builtin"): inject.NewMotionService("new_motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("new_motion"), resource.NewName(movementsensor.API, "movement_sensor"): inject.NewMovementSensor("movement_sensor"), } @@ -252,7 +253,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): inject.NewBase("new_base"), - resource.NewName(motion.API, "builtin"): inject.NewMotionService("new_motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("new_motion"), resource.NewName(movementsensor.API, "movement_sensor"): inject.NewMovementSensor("movement_sensor"), } @@ -285,7 +286,7 @@ func TestNew(t *testing.T) { deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, resource.NewName(camera.API, "camera"): inject.NewCamera("camera"), - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), resource.NewName(vision.API, "vision"): inject.NewVisionService("vision"), } @@ -319,7 +320,7 @@ func TestNew(t *testing.T) { deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, resource.NewName(camera.API, "camera"): inject.NewCamera("camera"), - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), resource.NewName(vision.API, "vision"): inject.NewVisionService("vision"), } @@ -361,7 +362,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), } err := svc.Reconfigure(ctx, deps, resource.Config{ConvertedAttributes: cfg}) @@ -381,7 +382,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), resource.NewName(camera.API, "camera"): inject.NewCamera("camera"), resource.NewName(movementsensor.API, "movement_sensor"): inject.NewMovementSensor("movement_sensor"), } @@ -403,7 +404,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), resource.NewName(vision.API, "vision"): inject.NewVisionService("vision"), resource.NewName(movementsensor.API, "movement_sensor"): inject.NewMovementSensor("movement_sensor"), } @@ -425,7 +426,7 @@ func TestNew(t *testing.T) { } deps := resource.Dependencies{ resource.NewName(base.API, "base"): &inject.Base{}, - resource.NewName(motion.API, "builtin"): inject.NewMotionService("motion"), + resource.NewName(motion.API, "builtin"): injectmotion.NewMotionService("motion"), resource.NewName(vision.API, "vision"): inject.NewVisionService("vision"), resource.NewName(camera.API, "camera"): inject.NewCamera("camera"), resource.NewName(movementsensor.API, "movement_sensor"): inject.NewMovementSensor("movement_sensor"), @@ -633,7 +634,7 @@ func setupStartWaypoint(ctx context.Context, t *testing.T, logger logging.Logger }, }, } - injectMS := inject.NewMotionService("test_motion") + injectMS := injectmotion.NewMotionService("test_motion") deps := resource.Dependencies{ injectMS.Name(): injectMS, fakeBase.Name(): fakeBase, @@ -684,7 +685,7 @@ func setupStartWaypointExplore(ctx context.Context, t *testing.T, logger logging }, }, } - injectMS := inject.NewMotionService("test_motion") + injectMS := injectmotion.NewMotionService("test_motion") deps := resource.Dependencies{ injectMS.Name(): injectMS, fakeBase.Name(): fakeBase, @@ -1722,7 +1723,7 @@ func TestGetObstacles(t *testing.T) { ) test.That(t, err, test.ShouldBeNil) - injectMS := inject.NewMotionService("test_motion") + injectMS := injectmotion.NewMotionService("test_motion") injectedVis := inject.NewVisionService("test_vision") injectMovementSensor := inject.NewMovementSensor("test_movement") injectedCam := inject.NewCamera("test_camera") diff --git a/testutils/inject/motion_service.go b/testutils/inject/motion/motion_service.go similarity index 97% rename from testutils/inject/motion_service.go rename to testutils/inject/motion/motion_service.go index 66a8cc2f7e0..4252d9f19a0 100644 --- a/testutils/inject/motion_service.go +++ b/testutils/inject/motion/motion_service.go @@ -1,3 +1,4 @@ +// Package inject separates the injected motion service from the rest of the injected packages to isolate an NLopt dependency. package inject import ( From bd6470c8fba1b36e7a78839a278d7f4fe4b22537 Mon Sep 17 00:00:00 2001 From: Ray Bjorkman Date: Thu, 10 Apr 2025 14:32:16 -0400 Subject: [PATCH 2/3] conformity in import name --- services/motion/client_test.go | 4 ++-- services/motion/server_test.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/services/motion/client_test.go b/services/motion/client_test.go index f7bcae7aeef..6d7dfd5cd65 100644 --- a/services/motion/client_test.go +++ b/services/motion/client_test.go @@ -27,7 +27,7 @@ import ( "go.viam.com/rdk/services/slam" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils" - inject "go.viam.com/rdk/testutils/inject/motion" + injectmotion "go.viam.com/rdk/testutils/inject/motion" ) var ( @@ -43,7 +43,7 @@ func TestClient(t *testing.T) { rpcServer, err := rpc.NewServer(logger, rpc.WithUnauthenticated()) test.That(t, err, test.ShouldBeNil) - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } diff --git a/services/motion/server_test.go b/services/motion/server_test.go index bb54e5477b7..9d1726faeee 100644 --- a/services/motion/server_test.go +++ b/services/motion/server_test.go @@ -28,7 +28,7 @@ import ( "go.viam.com/rdk/services/vision" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/testutils" - inject "go.viam.com/rdk/testutils/inject/motion" + injectmotion "go.viam.com/rdk/testutils/inject/motion" ) func newServer(resources map[resource.Name]motion.Service) (pb.MotionServiceServer, error) { @@ -53,7 +53,7 @@ func TestServerMove(t *testing.T) { test.That(t, err, test.ShouldBeError, errors.New("resource \"rdk:service:motion/motion1\" not found")) // error - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources = map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -76,7 +76,7 @@ func TestServerMove(t *testing.T) { test.That(t, resp.GetSuccess(), test.ShouldBeTrue) // Multiple Servies names Valid - injectMS = inject.NewMotionService("test") + injectMS = injectmotion.NewMotionService("test") resources = map[resource.Name]motion.Service{ testMotionServiceName: injectMS, testMotionServiceName2: injectMS, @@ -97,7 +97,7 @@ func TestServerMove(t *testing.T) { } func TestServerMoveOnGlobe(t *testing.T) { - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -279,7 +279,7 @@ func TestServerMoveOnGlobe(t *testing.T) { } func TestServerMoveOnMap(t *testing.T) { - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -450,7 +450,7 @@ func TestServerMoveOnMap(t *testing.T) { } func TestServerStopPlan(t *testing.T) { - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -515,7 +515,7 @@ func TestServerStopPlan(t *testing.T) { } func TestServerListPlanStatuses(t *testing.T) { - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -621,7 +621,7 @@ func TestServerListPlanStatuses(t *testing.T) { } func TestServerGetPlan(t *testing.T) { - injectMS := inject.NewMotionService("test") + injectMS := injectmotion.NewMotionService("test") resources := map[resource.Name]motion.Service{ testMotionServiceName: injectMS, } @@ -751,7 +751,7 @@ func TestServerGetPlan(t *testing.T) { func TestServerDoCommand(t *testing.T) { resourceMap := map[resource.Name]motion.Service{ - testMotionServiceName: &inject.MotionService{ + testMotionServiceName: &injectmotion.MotionService{ DoCommandFunc: testutils.EchoFunc, }, } From 8ff05dfb05c7b3b1c767342776bdfcdbb488d56f Mon Sep 17 00:00:00 2001 From: Ray Bjorkman Date: Mon, 14 Apr 2025 13:23:14 -0400 Subject: [PATCH 3/3] lint --- protoutils/messages.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protoutils/messages.go b/protoutils/messages.go index 64d9a339cb5..9376031d273 100644 --- a/protoutils/messages.go +++ b/protoutils/messages.go @@ -6,7 +6,7 @@ import ( "strconv" "github.com/golang/geo/r3" - protov1 "github.com/golang/protobuf/proto" + protov1 "github.com/golang/protobuf/proto" //nolint:staticcheck commonpb "go.viam.com/api/common/v1" robotpb "go.viam.com/api/robot/v1" "go.viam.com/utils/protoutils"