Skip to content

Commit e5940ec

Browse files
authored
[RSDK-11093] Make a vision.NewService() function that isn't deprecated (#5096)
Adwait and I pair programmed this; thanks for his help! The main changes are: - The `vizModel` struct no longer stores a `robot.Robot` within it. Instead, it stores a `logging.Logger` and a function that takes camera names and returns camera objects. - The old `NewService()` function has been renamed `DeprecatedNewService()`. Like before, it takes a `robot.Robot` as an argument, and can be used with `DeprecatedRobotConstructor` when registering a service. - There is a new function now named `NewService()`. It takes a `resource.Dependencies` as an object, and can be used with `Constructor` when registering a service. - The file `deprecated_vision_service_builder.go` (created in #5094) has been renamed to non-deprecated `vision_service_builder.go` This compiles and the tests pass, and the mlmodel vision service still works. However, nothing (yet) uses the new, non-deprecated `NewService()`. That can happen in future PRs. We have created a new function with a new type signature using an old name. However, we don't need to worry about someone outside of RDK using this function in their module and having the module break when they upgrade RDK versions: the reason RSDK-11093 was filed in the first place was that the old code was not usable in modules at all, because `DeprecatedNewService()` doesn't play well with modules!
1 parent 2c7cb76 commit e5940ec

File tree

16 files changed

+133
-33
lines changed

16 files changed

+133
-33
lines changed

services/vision/colordetector/color_detector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,5 @@ func registerColorDetector(
5959
return nil, errors.Errorf("could not find camera %q", conf.DefaultCamera)
6060
}
6161
}
62-
return vision.NewService(name, r, nil, nil, detector, nil, conf.DefaultCamera)
62+
return vision.DeprecatedNewService(name, r, nil, nil, detector, nil, conf.DefaultCamera)
6363
}

services/vision/colordetector/color_detector_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"go.viam.com/utils/artifact"
99

1010
"go.viam.com/rdk/components/camera"
11+
"go.viam.com/rdk/logging"
1112
"go.viam.com/rdk/resource"
1213
"go.viam.com/rdk/rimage"
1314
"go.viam.com/rdk/services/vision"
@@ -23,6 +24,7 @@ func TestColorDetector(t *testing.T) {
2324
}
2425
ctx := context.Background()
2526
r := &inject.Robot{}
27+
r.LoggerFunc = func() logging.Logger { return nil }
2628
name := vision.Named("test_cd")
2729
srv, err := registerColorDetector(ctx, name, &inp, r)
2830
test.That(t, err, test.ShouldBeNil)
@@ -68,6 +70,7 @@ func TestRegistrationWithDefaultCamera(t *testing.T) {
6870
}
6971

7072
r := &inject.Robot{}
73+
r.LoggerFunc = func() logging.Logger { return nil }
7174
r.ResourceByNameFunc = func(name resource.Name) (resource.Resource, error) {
7275
if name == cameraName {
7376
return inject.NewCamera(cameraName.Name), nil

services/vision/detectionstosegments/detections_to_3dsegments.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,5 +72,5 @@ func register3DSegmenterFromDetector(
7272
return nil, errors.Errorf("could not find camera %q", conf.DefaultCamera)
7373
}
7474
}
75-
return vision.NewService(name, r, nil, nil, detector, segmenter, conf.DefaultCamera)
75+
return vision.DeprecatedNewService(name, r, nil, nil, detector, segmenter, conf.DefaultCamera)
7676
}

services/vision/detectionstosegments/detections_to_3dsegments_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"go.viam.com/test"
1212

1313
"go.viam.com/rdk/components/camera"
14+
"go.viam.com/rdk/logging"
1415
pc "go.viam.com/rdk/pointcloud"
1516
"go.viam.com/rdk/resource"
1617
"go.viam.com/rdk/rimage"
@@ -29,9 +30,10 @@ func (s *simpleDetector) Detect(context.Context, image.Image) ([]objectdetection
2930

3031
func Test3DSegmentsFromDetector(t *testing.T) {
3132
r := &inject.Robot{}
33+
r.LoggerFunc = func() logging.Logger { return nil }
3234
m := &simpleDetector{}
3335
name := vision.Named("testDetector")
34-
svc, err := vision.NewService(name, r, nil, nil, m.Detect, nil, "")
36+
svc, err := vision.DeprecatedNewService(name, r, nil, nil, m.Detect, nil, "")
3537
test.That(t, err, test.ShouldBeNil)
3638
cam := &inject.Camera{}
3739
cam.NextPointCloudFunc = func(ctx context.Context) (pc.PointCloud, error) {

services/vision/fake/vision.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,5 +71,5 @@ func registerFake(
7171
name resource.Name,
7272
r robot.Robot,
7373
) (vision.Service, error) {
74-
return vision.NewService(name, r, nil, fakeClassifier, fakeDetector, nil, fakeCameraName)
74+
return vision.DeprecatedNewService(name, r, nil, fakeClassifier, fakeDetector, nil, fakeCameraName)
7575
}

services/vision/fake/vision_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@ import (
77

88
"go.viam.com/test"
99

10+
"go.viam.com/rdk/logging"
1011
"go.viam.com/rdk/services/vision"
1112
"go.viam.com/rdk/testutils/inject"
1213
)
1314

1415
func TestFakeVision(t *testing.T) {
1516
ctx := context.Background()
1617
r := &inject.Robot{}
18+
r.LoggerFunc = func() logging.Logger {
19+
return nil
20+
}
1721
name := vision.Named("test_fake")
1822
srv, err := registerFake(name, r)
1923
test.That(t, err, test.ShouldBeNil)

services/vision/mlvision/ml_model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func registerMLModelVisionService(
221221
}
222222

223223
// Don't return a close function, because you don't want to close the underlying ML service
224-
return vision.NewService(name, r, nil, classifierFunc, detectorFunc, segmenter3DFunc, params.DefaultCamera)
224+
return vision.DeprecatedNewService(name, r, nil, classifierFunc, detectorFunc, segmenter3DFunc, params.DefaultCamera)
225225
}
226226

227227
func getLabelsFromFile(labelPath string) []string {

services/vision/mlvision/ml_model_test.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ func BenchmarkAddMLVisionModel(b *testing.B) {
2626

2727
b.ResetTimer()
2828
for i := 0; i < b.N; i++ {
29-
service, err := registerMLModelVisionService(ctx, name, &modelCfg, &inject.Robot{}, logging.NewLogger("benchmark"))
29+
r := inject.Robot{}
30+
r.LoggerFunc = func() logging.Logger {
31+
return nil
32+
}
33+
service, err := registerMLModelVisionService(ctx, name, &modelCfg, &r, logging.NewLogger("benchmark"))
3034
test.That(b, err, test.ShouldBeNil)
3135
test.That(b, service, test.ShouldNotBeNil)
3236
test.That(b, service.Name(), test.ShouldResemble, name)
@@ -42,7 +46,11 @@ func BenchmarkUseMLVisionModel(b *testing.B) {
4246
test.That(b, pic, test.ShouldNotBeNil)
4347
modelCfg := MLModelConfig{ModelName: name.Name}
4448

45-
service, err := registerMLModelVisionService(ctx, name, &modelCfg, &inject.Robot{}, logging.NewLogger("benchmark"))
49+
r := inject.Robot{}
50+
r.LoggerFunc = func() logging.Logger {
51+
return nil
52+
}
53+
service, err := registerMLModelVisionService(ctx, name, &modelCfg, &r, logging.NewLogger("benchmark"))
4654
test.That(b, err, test.ShouldBeNil)
4755
test.That(b, service, test.ShouldNotBeNil)
4856
test.That(b, service.Name(), test.ShouldResemble, name)
@@ -572,7 +580,10 @@ func TestRegistrationWithDefaultCamera(t *testing.T) {
572580
cameraName := camera.Named("test")
573581
modelCfg := MLModelConfig{ModelName: modelName.Name}
574582

575-
r := &inject.Robot{}
583+
r := inject.Robot{}
584+
r.LoggerFunc = func() logging.Logger {
585+
return nil
586+
}
576587
r.ResourceByNameFunc = func(name resource.Name) (resource.Resource, error) {
577588
switch name {
578589
case modelName:
@@ -584,23 +595,23 @@ func TestRegistrationWithDefaultCamera(t *testing.T) {
584595
}
585596
}
586597

587-
service, err := registerMLModelVisionService(ctx, modelName, &modelCfg, r, logging.NewLogger("benchmark"))
598+
service, err := registerMLModelVisionService(ctx, modelName, &modelCfg, &r, logging.NewLogger("benchmark"))
588599
test.That(t, err, test.ShouldBeNil)
589600
test.That(t, service, test.ShouldNotBeNil)
590601

591602
modelCfg.DefaultCamera = cameraName.Name
592-
service, err = registerMLModelVisionService(ctx, modelName, &modelCfg, r, logging.NewLogger("benchmark"))
603+
service, err = registerMLModelVisionService(ctx, modelName, &modelCfg, &r, logging.NewLogger("benchmark"))
593604
test.That(t, err, test.ShouldBeNil)
594605
test.That(t, service, test.ShouldNotBeNil)
595606

596607
modelCfg.DefaultCamera = "not-camera"
597-
_, err = registerMLModelVisionService(ctx, modelName, &modelCfg, r, logging.NewLogger("benchmark"))
608+
_, err = registerMLModelVisionService(ctx, modelName, &modelCfg, &r, logging.NewLogger("benchmark"))
598609
test.That(t, err, test.ShouldNotBeNil)
599610
test.That(t, err.Error(), test.ShouldContainSubstring, "could not find camera \"not-camera\"")
600611

601612
// Test that *FromCamera errors when camera is not found
602613
modelCfg.DefaultCamera = ""
603-
service, err = registerMLModelVisionService(ctx, modelName, &modelCfg, r, logging.NewLogger("benchmark"))
614+
service, err = registerMLModelVisionService(ctx, modelName, &modelCfg, &r, logging.NewLogger("benchmark"))
604615
test.That(t, err, test.ShouldBeNil)
605616
test.That(t, service, test.ShouldNotBeNil)
606617
_, err = service.DetectionsFromCamera(ctx, "", nil)

services/vision/obstaclesdepth/obstacles_depth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func registerObstaclesDepth(
101101
}
102102

103103
segmenter := myObsDep.buildObsDepth(logger) // does the thing
104-
return svision.NewService(name, r, nil, nil, nil, segmenter, conf.DefaultCamera)
104+
return svision.DeprecatedNewService(name, r, nil, nil, nil, segmenter, conf.DefaultCamera)
105105
}
106106

107107
// BuildObsDepth will check for intrinsics and determine how to build based on that.

services/vision/obstaclesdistance/obstacles_distance.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func registerObstacleDistanceDetector(
120120
return nil, errors.Errorf("could not find camera %q", conf.DefaultCamera)
121121
}
122122
}
123-
return svision.NewService(name, r, nil, nil, nil, segmenter, conf.DefaultCamera)
123+
return svision.DeprecatedNewService(name, r, nil, nil, nil, segmenter, conf.DefaultCamera)
124124
}
125125

126126
func medianFromPointClouds(ctx context.Context, clouds []pointcloud.PointCloud) (r3.Vector, error) {

0 commit comments

Comments
 (0)