Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RSDK-812-817] Add extra param to services #1545

Merged
merged 17 commits into from
Nov 7, 2022
Merged
2 changes: 1 addition & 1 deletion cli/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ func (c *AppClient) StartRobotPartShell(
return nil
}

input, output, err := shellSvc.Shell(c.c.Context)
input, output, err := shellSvc.Shell(c.c.Context, map[string]interface{}{})
if err != nil {
fmt.Fprintln(c.c.App.ErrWriter, err)
cli.OsExiter(1)
Expand Down
2 changes: 1 addition & 1 deletion components/camera/transformpipeline/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (ds *detectorSource) Read(ctx context.Context) (image.Image, func(), error)
if err != nil {
return nil, nil, fmt.Errorf("could not get next source image: %w", err)
}
dets, err := srv.Detections(ctx, img, ds.detectorName)
dets, err := srv.Detections(ctx, img, ds.detectorName, map[string]interface{}{})
if err != nil {
return nil, nil, fmt.Errorf("could not get detections: %w", err)
}
Expand Down
8 changes: 4 additions & 4 deletions components/camera/transformpipeline/detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func TestColorDetectionSource(t *testing.T) {
"segment_size_px": 15000,
},
}
err = srv.AddDetector(context.Background(), detConf)
err = srv.AddDetector(context.Background(), detConf, map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)

detector, err := camera.FromRobot(r, "color_detect")
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestTFLiteDetectionSource(t *testing.T) {
"num_threads": 1,
},
}
err = srv.AddDetector(context.Background(), detConf)
err = srv.AddDetector(context.Background(), detConf, map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)

detector, err := camera.FromRobot(r, "tflite_detect")
Expand Down Expand Up @@ -206,7 +206,7 @@ func BenchmarkColorDetectionSource(b *testing.B) {
"segment_size_px": 15000,
},
}
err = srv.AddDetector(context.Background(), detConf)
err = srv.AddDetector(context.Background(), detConf, map[string]interface{}{})
test.That(b, err, test.ShouldBeNil)
detector, err := camera.FromRobot(r, "color_detect")
test.That(b, err, test.ShouldBeNil)
Expand Down Expand Up @@ -241,7 +241,7 @@ func BenchmarkTFLiteDetectionSource(b *testing.B) {
"num_threads": 1,
},
}
err = srv.AddDetector(context.Background(), detConf)
err = srv.AddDetector(context.Background(), detConf, map[string]interface{}{})
test.That(b, err, test.ShouldBeNil)
detector, err := camera.FromRobot(r, "tflite_detect")
test.That(b, err, test.ShouldBeNil)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ require (
go.uber.org/atomic v1.10.0
go.uber.org/multierr v1.8.0
go.uber.org/zap v1.23.0
go.viam.com/api v0.1.5-0.20221104220238-fa20e43bb186
go.viam.com/api v0.1.6-0.20221107194621-d3f3c811b448
go.viam.com/dynamixel v0.0.0-20210507131419-60a9033552cb
go.viam.com/test v1.1.1-0.20220909204145-f61b7c01c33e
go.viam.com/utils v0.1.3-0.20221107052430-1d6f4b5bf6eb
Expand Down
6 changes: 2 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1503,14 +1503,12 @@ go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
go.uber.org/zap v1.13.0/go.mod h1:zwrFLgMcdUuIBviXEYEH1YKNaOBnKXsx2IPda5bBwHM=
go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY=
go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY=
go.viam.com/api v0.1.5-0.20221104220238-fa20e43bb186 h1:lPyS6SrD+2zK71wEHMzZlVGfIe2OfmIUpZve+8dHK1M=
go.viam.com/api v0.1.5-0.20221104220238-fa20e43bb186/go.mod h1:GszZtg1xX/3/rrdjp8jvWAZIeFfCreXwN+LdpHNX+Ek=
go.viam.com/api v0.1.6-0.20221107194621-d3f3c811b448 h1:DCDf1tGNDUG8zU81QxxIO5Chp/K6blTZeFdNmoyBPGQ=
go.viam.com/api v0.1.6-0.20221107194621-d3f3c811b448/go.mod h1:GszZtg1xX/3/rrdjp8jvWAZIeFfCreXwN+LdpHNX+Ek=
go.viam.com/dynamixel v0.0.0-20210507131419-60a9033552cb h1:qXL9ae27upOyWn//rVGFJt90nDzT7Gz5WOaNe+9PJb8=
go.viam.com/dynamixel v0.0.0-20210507131419-60a9033552cb/go.mod h1:bVVm3GcP8koFgCYKWJkCU0r+lkOvTQLeLaFcw8AFghE=
go.viam.com/test v1.1.1-0.20220909204145-f61b7c01c33e h1:1tJIJv/zsobFV5feR2ZiG/+VGZkRPVHqJrSTkXbWfqQ=
go.viam.com/test v1.1.1-0.20220909204145-f61b7c01c33e/go.mod h1:XM0tej6riszsiNLT16uoyq1YjuYPWlRBweTPRDanIts=
go.viam.com/utils v0.1.3-0.20221107041059-332439321bcc h1:QHWU+fXBPnlzqrAyNzbms45MVTCsWpMkJ+OH06PjoGA=
go.viam.com/utils v0.1.3-0.20221107041059-332439321bcc/go.mod h1:U6PxWfe/SsEgBjgRtlkj5n8YIfiKOXtCszUqdiCyzVs=
go.viam.com/utils v0.1.3-0.20221107052430-1d6f4b5bf6eb h1:VoEKRB7CumnnYF8zXYfoF8Dy7cZbrUNFJkNZKvsXVHU=
go.viam.com/utils v0.1.3-0.20221107052430-1d6f4b5bf6eb/go.mod h1:U6PxWfe/SsEgBjgRtlkj5n8YIfiKOXtCszUqdiCyzVs=
go4.org v0.0.0-20180809161055-417644f6feb5/go.mod h1:MkTOUMDaeVYJUOUsaDXIhWPZYa1yOyC1qaOBpL57BhE=
Expand Down
6 changes: 3 additions & 3 deletions robot/impl/local_robot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -969,17 +969,17 @@ func TestSensorsService(t *testing.T) {
test.That(t, err, test.ShouldBeNil)

sensorNames := []resource.Name{movementsensor.Named("movement_sensor1"), movementsensor.Named("movement_sensor2")}
foundSensors, err := svc.Sensors(context.Background())
foundSensors, err := svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, rtestutils.NewResourceNameSet(foundSensors...), test.ShouldResemble, rtestutils.NewResourceNameSet(sensorNames...))

readings, err := svc.Readings(context.Background(), []resource.Name{movementsensor.Named("movement_sensor1")})
readings, err := svc.Readings(context.Background(), []resource.Name{movementsensor.Named("movement_sensor1")}, map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, len(readings), test.ShouldEqual, 1)
test.That(t, readings[0].Name, test.ShouldResemble, movementsensor.Named("movement_sensor1"))
test.That(t, len(readings[0].Readings), test.ShouldBeGreaterThan, 3)

readings, err = svc.Readings(context.Background(), sensorNames)
readings, err = svc.Readings(context.Background(), sensorNames, map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, len(readings), test.ShouldEqual, 2)

Expand Down
1 change: 1 addition & 0 deletions robot/impl/resource_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ func TestManagerAdd(t *testing.T) {
injectVisionService.GetObjectPointCloudsFunc = func(
ctx context.Context,
cameraName, segmenterName string,
extra map[string]interface{},
) ([]*viz.Object, error) {
return []*viz.Object{viz.NewEmptyObject()}, nil
}
Expand Down
12 changes: 6 additions & 6 deletions robot/impl/robot_reconfigure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2257,13 +2257,13 @@ func TestSensorsServiceUpdate(t *testing.T) {
svc, err := sensors.FromRobot(robot, resource.DefaultModelName)
test.That(t, err, test.ShouldBeNil)

foundSensors, err := svc.Sensors(context.Background())
foundSensors, err := svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, foundSensors, test.ShouldBeEmpty)

robot.Reconfigure(context.Background(), cfg)

foundSensors, err = svc.Sensors(context.Background())
foundSensors, err = svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, rdktestutils.NewResourceNameSet(foundSensors...), test.ShouldResemble, rdktestutils.NewResourceNameSet(sensorNames...))
})
Expand All @@ -2278,13 +2278,13 @@ func TestSensorsServiceUpdate(t *testing.T) {
svc, err := sensors.FromRobot(robot, resource.DefaultModelName)
test.That(t, err, test.ShouldBeNil)

foundSensors, err := svc.Sensors(context.Background())
foundSensors, err := svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, rdktestutils.NewResourceNameSet(foundSensors...), test.ShouldResemble, rdktestutils.NewResourceNameSet(sensorNames...))

robot.Reconfigure(context.Background(), emptyCfg)

foundSensors, err = svc.Sensors(context.Background())
foundSensors, err = svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, foundSensors, test.ShouldBeEmpty)
})
Expand All @@ -2299,13 +2299,13 @@ func TestSensorsServiceUpdate(t *testing.T) {
svc, err := sensors.FromRobot(robot, resource.DefaultModelName)
test.That(t, err, test.ShouldBeNil)

foundSensors, err := svc.Sensors(context.Background())
foundSensors, err := svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, rdktestutils.NewResourceNameSet(foundSensors...), test.ShouldResemble, rdktestutils.NewResourceNameSet(sensorNames...))

robot.Reconfigure(context.Background(), cfg)

foundSensors, err = svc.Sensors(context.Background())
foundSensors, err = svc.Sensors(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
test.That(t, rdktestutils.NewResourceNameSet(foundSensors...), test.ShouldResemble, rdktestutils.NewResourceNameSet(sensorNames...))
})
Expand Down
2 changes: 1 addition & 1 deletion services/datamanager/builtin/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func (svc *builtIn) initOrUpdateSyncer(_ context.Context, intervalMins float64,
}

// Sync performs a non-scheduled sync of the data in the capture directory.
func (svc *builtIn) Sync(_ context.Context) error {
func (svc *builtIn) Sync(_ context.Context, extra map[string]interface{}) error {
if svc.syncer == nil {
return errors.New("called Sync on data manager service with nil syncer")
}
Expand Down
6 changes: 3 additions & 3 deletions services/datamanager/builtin/builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ func TestManualSync(t *testing.T) {
test.That(t, err, test.ShouldBeNil)

// Run and upload files.
err = dmsvc.Sync(context.Background())
err = dmsvc.Sync(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
time.Sleep(syncWaitTime)

Expand All @@ -538,7 +538,7 @@ func TestManualSync(t *testing.T) {

// Sync again and verify it synced the second data capture file, but also validate that it didn't attempt to resync
// any files that were previously synced.
err = dmsvc.Sync(context.Background())
err = dmsvc.Sync(context.Background(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
time.Sleep(syncWaitTime)
_ = dmsvc.Close(context.TODO())
Expand Down Expand Up @@ -638,7 +638,7 @@ func TestManualAndScheduledSync(t *testing.T) {

// Perform a manual and scheduled syncDataCaptureFiles at approximately the same time, then close the svc.
time.Sleep(time.Millisecond * 250)
err = dmsvc.Sync(context.TODO())
err = dmsvc.Sync(context.TODO(), map[string]interface{}{})
test.That(t, err, test.ShouldBeNil)
time.Sleep(syncWaitTime)
_ = dmsvc.Close(context.TODO())
Expand Down
9 changes: 7 additions & 2 deletions services/datamanager/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/edaniels/golog"
pb "go.viam.com/api/service/datamanager/v1"
"go.viam.com/utils/protoutils"
"go.viam.com/utils/rpc"
)

Expand All @@ -29,8 +30,12 @@ func NewClientFromConn(ctx context.Context, conn rpc.ClientConn, name string, lo
return c
}

func (c *client) Sync(ctx context.Context) error {
_, err := c.client.Sync(ctx, &pb.SyncRequest{Name: c.name})
func (c *client) Sync(ctx context.Context, extra map[string]interface{}) error {
ext, err := protoutils.StructToStructPb(extra)
if err != nil {
return err
}
_, err = c.client.Sync(ctx, &pb.SyncRequest{Name: c.name, Extra: ext})
if err != nil {
return err
}
Expand Down
25 changes: 13 additions & 12 deletions services/datamanager/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ func TestClient(t *testing.T) {
rpcServer, err := rpc.NewServer(logger, rpc.WithUnauthenticated())
test.That(t, err, test.ShouldBeNil)

injectMS := &inject.DataManagerService{}
var extraOptions map[string]interface{}

injectDS := &inject.DataManagerService{}
resourceMap := map[resource.Name]interface{}{
datamanager.Named(testDataManagerServiceName): injectMS,
datamanager.Named(testDataManagerServiceName): injectDS,
}
svc, err := subtype.New(resourceMap)
test.That(t, err, test.ShouldBeNil)
Expand All @@ -59,13 +61,14 @@ func TestClient(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
client := datamanager.NewClientFromConn(context.Background(), conn, testDataManagerServiceName, logger)

injectMS.SyncFunc = func(
ctx context.Context,
) error {
injectDS.SyncFunc = func(ctx context.Context, extra map[string]interface{}) error {
extraOptions = extra
return nil
}
err = client.Sync(context.Background())
extra := map[string]interface{}{"foo": "Sync"}
err = client.Sync(context.Background(), extra)
test.That(t, err, test.ShouldBeNil)
test.That(t, extraOptions, test.ShouldResemble, extra)
test.That(t, utils.TryClose(context.Background(), client), test.ShouldBeNil)
test.That(t, conn.Close(), test.ShouldBeNil)
})
Expand All @@ -79,13 +82,11 @@ func TestClient(t *testing.T) {
test.That(t, ok, test.ShouldBeTrue)

passedErr := errors.New("fake sync error")
injectMS.SyncFunc = func(
ctx context.Context,
) error {
injectDS.SyncFunc = func(ctx context.Context, extra map[string]interface{}) error {
return passedErr
}

err = client2.Sync(context.Background())
err = client2.Sync(context.Background(), map[string]interface{}{})
test.That(t, err.Error(), test.ShouldContainSubstring, passedErr.Error())
test.That(t, utils.TryClose(context.Background(), client), test.ShouldBeNil)
test.That(t, conn.Close(), test.ShouldBeNil)
Expand All @@ -98,9 +99,9 @@ func TestClientDialerOption(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
gServer := grpc.NewServer()

injectMS := &inject.DataManagerService{}
injectDS := &inject.DataManagerService{}
resourceMap := map[resource.Name]interface{}{
datamanager.Named(testDataManagerServiceName): injectMS,
datamanager.Named(testDataManagerServiceName): injectDS,
}
server, err := newServer(resourceMap)
test.That(t, err, test.ShouldBeNil)
Expand Down
7 changes: 3 additions & 4 deletions services/datamanager/data_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func init() {

// Service defines what a Data Manager Service should expose to the users.
type Service interface {
Sync(ctx context.Context) error
Sync(ctx context.Context, extra map[string]interface{}) error
}

var (
Expand All @@ -65,7 +65,6 @@ var Subtype = resource.NewSubtype(
)

// Named is a helper for getting the named datamanager's typed resource name.
// RSDK-347 Implements datamanager's Named.
func Named(name string) resource.Name {
return resource.NameFromSubtype(Subtype, name)
}
Expand Down Expand Up @@ -102,10 +101,10 @@ type reconfigurableDataManager struct {
actual Service
}

func (svc *reconfigurableDataManager) Sync(ctx context.Context) error {
func (svc *reconfigurableDataManager) Sync(ctx context.Context, extra map[string]interface{}) error {
svc.mu.RLock()
defer svc.mu.RUnlock()
return svc.actual.Sync(ctx)
return svc.actual.Sync(ctx, extra)
}

func (svc *reconfigurableDataManager) Close(ctx context.Context) error {
Expand Down
21 changes: 19 additions & 2 deletions services/datamanager/data_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func TestReconfigurable(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
test.That(t, reconfSvc1, test.ShouldNotBeNil)

actualArm2 := &mock{name: testSvcName2}
reconfSvc2, err := datamanager.WrapWithReconfigurable(actualArm2)
actualSvc2 := &mock{name: testSvcName2}
reconfSvc2, err := datamanager.WrapWithReconfigurable(actualSvc2)
test.That(t, err, test.ShouldBeNil)
test.That(t, reconfSvc2, test.ShouldNotBeNil)
test.That(t, actualSvc1.reconfCount, test.ShouldEqual, 0)
Expand All @@ -58,10 +58,27 @@ func TestReconfigurable(t *testing.T) {
test.That(t, err, test.ShouldBeError, rutils.NewUnexpectedTypeError(reconfSvc1, nil))
}

func TestExtraOptions(t *testing.T) {
actualSvc1 := &mock{name: testSvcName1}
reconfSvc1, err := datamanager.WrapWithReconfigurable(actualSvc1)
test.That(t, err, test.ShouldBeNil)

test.That(t, actualSvc1.extra, test.ShouldEqual, nil)

reconfSvc1.(datamanager.Service).Sync(context.Background(), map[string]interface{}{"foo": "bar"})
test.That(t, actualSvc1.extra, test.ShouldResemble, map[string]interface{}{"foo": "bar"})
}

type mock struct {
datamanager.Service
name string
reconfCount int
extra map[string]interface{}
}

func (m *mock) Sync(_ context.Context, extra map[string]interface{}) error {
m.extra = extra
return nil
}

func (m *mock) Close(_ context.Context) error {
Expand Down
2 changes: 1 addition & 1 deletion services/datamanager/internal/data_manager_test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// updating processes in the data manager service. These functions are not exported to the user. This resolves
// a circular import caused by the inject package.
type DMService interface {
Sync(ctx context.Context) error
Sync(ctx context.Context, extra map[string]interface{}) error
Update(ctx context.Context, cfg *config.Config) error
Close(ctx context.Context) error
SetSyncerConstructor(fn datasync.ManagerConstructor)
Expand Down
5 changes: 1 addition & 4 deletions services/datamanager/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ func (server *subtypeServer) Sync(ctx context.Context, req *pb.SyncRequest) (*pb
if err != nil {
return nil, err
}
err = svc.Sync(
ctx,
)
if err != nil {
if err := svc.Sync(ctx, req.Extra.AsMap()); err != nil {
return nil, err
}
return &pb.SyncResponse{}, nil
Expand Down
Loading