Skip to content

Commit

Permalink
[RSDK-9655] skip reflection for discovery service and add fake discov…
Browse files Browse the repository at this point in the history
…ery (#4697)
  • Loading branch information
JohnN193 authored Jan 10, 2025
1 parent d2b9c7e commit 27ae6f5
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 33 deletions.
59 changes: 59 additions & 0 deletions examples/customresources/models/mydiscovery/mydiscovery.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Package mydiscovery implements a discovery that returns some fake components.
package mydiscovery

import (
"context"

"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/components/movementsensor"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/discovery"
)

// Model is the full model definition.
var Model = resource.NewModel("acme", "demo", "mydiscovery")

func init() {
resource.RegisterService(
discovery.API,
Model,
resource.Registration[discovery.Service, resource.NoNativeConfig]{Constructor: func(
ctx context.Context,
deps resource.Dependencies,
conf resource.Config,
logger logging.Logger,
) (discovery.Service, error) {
return newDiscovery(conf.ResourceName(), logger), nil
}})
}

func newDiscovery(name resource.Name, logger logging.Logger) discovery.Service {
cfg1 := createFakeConfig("fake1", movementsensor.API)
cfg2 := createFakeConfig("fake2", camera.API)
return &Discovery{Named: name.AsNamed(), logger: logger, cfgs: []resource.Config{cfg1, cfg2}}
}

// DiscoverResources returns the discovered resources.
func (dis *Discovery) DiscoverResources(context.Context, map[string]any) ([]resource.Config, error) {
return dis.cfgs, nil
}

// Discovery is a fake Discovery service.
type Discovery struct {
resource.Named
resource.TriviallyReconfigurable
resource.TriviallyCloseable
logger logging.Logger
cfgs []resource.Config
}

// DoCommand echos input back to the caller.
func (dis *Discovery) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) {
return cmd, nil
}

// createFakeConfig creates a fake component with the defined name, api, and attributes.
func createFakeConfig(name string, api resource.API) resource.Config {
return resource.Config{Name: name, API: api, Model: resource.DefaultModelFamily.WithModel("fake")}
}
35 changes: 20 additions & 15 deletions module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"go.viam.com/rdk/protoutils"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/robot/client"
"go.viam.com/rdk/services/discovery"
rutils "go.viam.com/rdk/utils"
)

Expand Down Expand Up @@ -134,22 +135,26 @@ func NewHandlerMapFromProto(ctx context.Context, pMap *pb.HandlerMap, conn rpc.C
var errs error
for _, h := range pMap.GetHandlers() {
api := protoutils.ResourceNameFromProto(h.Subtype.Subtype).API

symDesc, err := reflSource.FindSymbol(h.Subtype.ProtoService)
if err != nil {
errs = multierr.Combine(errs, err)
if errors.Is(err, grpcurl.ErrReflectionNotSupported) {
return nil, errs
}
continue
}
svcDesc, ok := symDesc.(*desc.ServiceDescriptor)
if !ok {
return nil, errors.Errorf("expected descriptor to be service descriptor but got %T", symDesc)
}
rpcAPI := &resource.RPCAPI{
API: api,
Desc: svcDesc,
API: api,
}
// due to how tagger is setup in the api we cannot use reflection on the discovery service currently
// for now we will skip the reflection step for discovery until the issue is resolved.
// TODO(RSDK-9718) - remove the skip.
if api != discovery.API {
symDesc, err := reflSource.FindSymbol(h.Subtype.ProtoService)
if err != nil {
errs = multierr.Combine(errs, err)
if errors.Is(err, grpcurl.ErrReflectionNotSupported) {
return nil, errs
}
continue
}
svcDesc, ok := symDesc.(*desc.ServiceDescriptor)
if !ok {
return nil, errors.Errorf("expected descriptor to be service descriptor but got %T", symDesc)
}
rpcAPI.Desc = svcDesc
}
for _, m := range h.Models {
model, err := resource.NewModelFromString(m)
Expand Down
68 changes: 50 additions & 18 deletions module/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ import (
"go.viam.com/rdk/examples/customresources/apis/gizmoapi"
"go.viam.com/rdk/examples/customresources/apis/summationapi"
"go.viam.com/rdk/examples/customresources/models/mybase"
"go.viam.com/rdk/examples/customresources/models/mydiscovery"
"go.viam.com/rdk/examples/customresources/models/mygizmo"
"go.viam.com/rdk/examples/customresources/models/mysum"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/module"
"go.viam.com/rdk/resource"
robotimpl "go.viam.com/rdk/robot/impl"
"go.viam.com/rdk/services/datamanager"
"go.viam.com/rdk/services/discovery"
"go.viam.com/rdk/services/shell"
"go.viam.com/rdk/testutils/inject"
rutils "go.viam.com/rdk/utils"
Expand Down Expand Up @@ -176,6 +178,7 @@ func TestModuleFunctions(t *testing.T) {

test.That(t, m.AddModelFromRegistry(ctx, gizmoapi.API, mygizmo.Model), test.ShouldBeNil)
test.That(t, m.AddModelFromRegistry(ctx, base.API, mybase.Model), test.ShouldBeNil)
test.That(t, m.AddModelFromRegistry(ctx, discovery.API, mydiscovery.Model), test.ShouldBeNil)

test.That(t, m.Start(ctx), test.ShouldBeNil)

Expand Down Expand Up @@ -204,37 +207,66 @@ func TestModuleFunctions(t *testing.T) {
t.Run("HandlerMap", func(t *testing.T) {
// test the raw return
handlers := resp.GetHandlermap().GetHandlers()
test.That(t, "acme", test.ShouldBeIn, handlers[0].Subtype.Subtype.Namespace, handlers[1].Subtype.Subtype.Namespace)
test.That(t, "rdk", test.ShouldBeIn, handlers[0].Subtype.Subtype.Namespace, handlers[1].Subtype.Subtype.Namespace)
test.That(t, "component", test.ShouldBeIn, handlers[0].Subtype.Subtype.Type, handlers[1].Subtype.Subtype.Type)
test.That(t, "gizmo", test.ShouldBeIn, handlers[0].Subtype.Subtype.Subtype, handlers[1].Subtype.Subtype.Subtype)
test.That(t, "base", test.ShouldBeIn, handlers[0].Subtype.Subtype.Subtype, handlers[1].Subtype.Subtype.Subtype)
test.That(t, "acme:demo:mygizmo", test.ShouldBeIn, handlers[0].GetModels()[0], handlers[1].GetModels()[0])
test.That(t, "acme:demo:mybase", test.ShouldBeIn, handlers[0].GetModels()[0], handlers[1].GetModels()[0])
test.That(t, "acme", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Namespace, handlers[1].Subtype.Subtype.Namespace, handlers[2].Subtype.Subtype.Namespace)
test.That(t, "rdk", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Namespace, handlers[1].Subtype.Subtype.Namespace, handlers[2].Subtype.Subtype.Namespace)
test.That(t, "component", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Type, handlers[1].Subtype.Subtype.Type, handlers[2].Subtype.Subtype.Type)
test.That(t, "service", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Type, handlers[1].Subtype.Subtype.Type, handlers[2].Subtype.Subtype.Type)
test.That(t, "gizmo", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Subtype, handlers[1].Subtype.Subtype.Subtype, handlers[2].Subtype.Subtype.Subtype)
test.That(t, "base", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Subtype, handlers[1].Subtype.Subtype.Subtype, handlers[2].Subtype.Subtype.Subtype)
test.That(t, "discovery", test.ShouldBeIn,
handlers[0].Subtype.Subtype.Subtype, handlers[1].Subtype.Subtype.Subtype, handlers[2].Subtype.Subtype.Subtype)
test.That(t, "acme:demo:mygizmo", test.ShouldBeIn,
handlers[0].GetModels()[0], handlers[1].GetModels()[0], handlers[2].GetModels()[0])
test.That(t, "acme:demo:mybase", test.ShouldBeIn,
handlers[0].GetModels()[0], handlers[1].GetModels()[0], handlers[2].GetModels()[0])
test.That(t, "acme:demo:mydiscovery", test.ShouldBeIn,
handlers[0].GetModels()[0], handlers[1].GetModels()[0], handlers[2].GetModels()[0])

// convert from proto
hmap, err := module.NewHandlerMapFromProto(ctx, resp.GetHandlermap(), rpc.GrpcOverHTTPClientConn{ClientConn: conn})
test.That(t, err, test.ShouldBeNil)
test.That(t, len(hmap), test.ShouldEqual, 2)
test.That(t, len(hmap), test.ShouldEqual, 3)

for k, v := range hmap {
test.That(t, k.API, test.ShouldBeIn, gizmoapi.API, base.API)
if k.API == gizmoapi.API {
test.That(t, k.API, test.ShouldBeIn, gizmoapi.API, base.API, discovery.API)
switch k.API {
case gizmoapi.API:
test.That(t, mygizmo.Model, test.ShouldResemble, v[0])
} else {
case discovery.API:
test.That(t, mydiscovery.Model, test.ShouldResemble, v[0])
default:
test.That(t, mybase.Model, test.ShouldResemble, v[0])
}
}

// convert back to proto
handlers2 := hmap.ToProto().GetHandlers()
test.That(t, "acme", test.ShouldBeIn, handlers2[0].Subtype.Subtype.Namespace, handlers2[1].Subtype.Subtype.Namespace)
test.That(t, "rdk", test.ShouldBeIn, handlers2[0].Subtype.Subtype.Namespace, handlers2[1].Subtype.Subtype.Namespace)
test.That(t, "component", test.ShouldBeIn, handlers2[0].Subtype.Subtype.Type, handlers2[1].Subtype.Subtype.Type)
test.That(t, "gizmo", test.ShouldBeIn, handlers2[0].Subtype.Subtype.Subtype, handlers2[1].Subtype.Subtype.Subtype)
test.That(t, "base", test.ShouldBeIn, handlers2[0].Subtype.Subtype.Subtype, handlers2[1].Subtype.Subtype.Subtype)
test.That(t, "acme:demo:mygizmo", test.ShouldBeIn, handlers2[0].GetModels()[0], handlers2[1].GetModels()[0])
test.That(t, "acme:demo:mybase", test.ShouldBeIn, handlers2[0].GetModels()[0], handlers2[1].GetModels()[0])
test.That(t, "acme", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Namespace, handlers2[1].Subtype.Subtype.Namespace, handlers2[2].Subtype.Subtype.Namespace)
test.That(t, "rdk", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Namespace, handlers2[1].Subtype.Subtype.Namespace, handlers2[2].Subtype.Subtype.Namespace)
test.That(t, "component", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Type, handlers2[1].Subtype.Subtype.Type, handlers2[2].Subtype.Subtype.Type)
test.That(t, "service", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Type, handlers2[1].Subtype.Subtype.Type, handlers2[2].Subtype.Subtype.Type)
test.That(t, "gizmo", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Subtype, handlers2[1].Subtype.Subtype.Subtype, handlers2[2].Subtype.Subtype.Subtype)
test.That(t, "base", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Subtype, handlers2[1].Subtype.Subtype.Subtype, handlers2[2].Subtype.Subtype.Subtype)
test.That(t, "discovery", test.ShouldBeIn,
handlers2[0].Subtype.Subtype.Subtype, handlers2[1].Subtype.Subtype.Subtype, handlers2[2].Subtype.Subtype.Subtype)
test.That(t, "acme:demo:mygizmo", test.ShouldBeIn,
handlers2[0].GetModels()[0], handlers2[1].GetModels()[0], handlers2[2].GetModels()[0])
test.That(t, "acme:demo:mybase", test.ShouldBeIn,
handlers2[0].GetModels()[0], handlers2[1].GetModels()[0], handlers2[2].GetModels()[0])
test.That(t, "acme:demo:mydiscovery", test.ShouldBeIn,
handlers2[0].GetModels()[0], handlers2[1].GetModels()[0], handlers2[2].GetModels()[0])
})

t.Run("GetParentResource", func(t *testing.T) {
Expand Down
75 changes: 75 additions & 0 deletions services/discovery/fake/fake.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Package fake implements a fake discovery service.
package fake

import (
"context"

"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/components/movementsensor"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/discovery"
"go.viam.com/rdk/utils"
)

func init() {
resource.RegisterService(
discovery.API,
resource.DefaultModelFamily.WithModel("fake"),
resource.Registration[discovery.Service, resource.NoNativeConfig]{Constructor: func(
ctx context.Context,
deps resource.Dependencies,
conf resource.Config,
logger logging.Logger,
) (discovery.Service, error) {
return newDiscovery(conf.ResourceName(), logger), nil
}})
}

func newDiscovery(name resource.Name, logger logging.Logger) discovery.Service {
cfg1 := createFakeConfig("fake1", movementsensor.API, nil)
cfg2 := createFakeConfig("fake2", camera.API, nil)
return &Discovery{Named: name.AsNamed(), logger: logger, cfgs: []resource.Config{cfg1, cfg2}}
}

// DiscoverResources returns the discovered resources.
func (dis *Discovery) DiscoverResources(context.Context, map[string]any) ([]resource.Config, error) {
return dis.cfgs, nil
}

// Discovery is a fake Discovery service that returns.
type Discovery struct {
resource.Named
resource.TriviallyReconfigurable
resource.TriviallyCloseable
logger logging.Logger
cfgs []resource.Config
}

// DoCommand echos input back to the caller.
func (dis *Discovery) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) {
return cmd, nil
}

// createFakeConfig creates a fake component with the defined name, api, and attributes.
// additionally the commented code is an example of how to take a model's Config and convert it into Attributes for the api.
//
//nolint:unparam
func createFakeConfig(name string, api resource.API, attributes utils.AttributeMap) resource.Config {
// // using the camera's Config struct in case a breaking change occurs
// attributes := viamrtsp.Config{Address: address}
// var result map[string]interface{}

// // marshal to bytes
// jsonBytes, err := json.Marshal(attributes)
// if err != nil {
// return resource.Config{}, err
// }

// // convert to map to be used as attributes in resource.Config
// err = json.Unmarshal(jsonBytes, &result)
// if err != nil {
// return resource.Config{}, err
// }
return resource.Config{Name: name, API: api, Model: resource.DefaultModelFamily.WithModel("fake"), Attributes: attributes}
}
34 changes: 34 additions & 0 deletions services/discovery/fake/fake_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package fake

import (
"context"
"testing"

"go.viam.com/test"

"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/components/movementsensor"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/discovery"
)

func TestDiscoverResources(t *testing.T) {
ctx := context.Background()
logger := logging.NewTestLogger(t)

dis := newDiscovery(discovery.Named("foo"), logger)

expectedCfgs := []resource.Config{
createFakeConfig("fake1", movementsensor.API, nil),
createFakeConfig("fake2", camera.API, nil),
}
cfgs, err := dis.DiscoverResources(ctx, nil)
test.That(t, err, test.ShouldBeNil)
test.That(t, len(cfgs), test.ShouldEqual, len(expectedCfgs))
for index, cfg := range cfgs {
test.That(t, cfg.Name, test.ShouldEqual, expectedCfgs[index].Name)
test.That(t, cfg.API, test.ShouldResemble, expectedCfgs[index].API)
test.That(t, cfg.Model, test.ShouldResemble, expectedCfgs[index].Model)
}
}
7 changes: 7 additions & 0 deletions services/discovery/register/register.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Package register registers all relevant discovery models and also API specific functions
package register

import (
// for discovery models.
_ "go.viam.com/rdk/services/discovery/fake"
)
1 change: 1 addition & 0 deletions services/register/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
// register services.
_ "go.viam.com/rdk/services/baseremotecontrol/register"
_ "go.viam.com/rdk/services/datamanager/register"
_ "go.viam.com/rdk/services/discovery/register"
_ "go.viam.com/rdk/services/generic/register"
_ "go.viam.com/rdk/services/shell/register"
_ "go.viam.com/rdk/services/slam/register"
Expand Down

0 comments on commit 27ae6f5

Please sign in to comment.