[RSDK-9655] skip reflection for discovery service and add fake discovery

examples/customresources/models/mydiscovery/mydiscovery.go

@@ -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")}
+}

module/module.go

@@ -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"
 )
 
@@ -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)

module/module_test.go

@@ -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"
@@ -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)
 
@@ -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) {

services/discovery/fake/fake.go

@@ -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}
+}

services/discovery/fake/fake_test.go

@@ -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)
+	}
+}

services/discovery/register/register.go

@@ -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"
+)

services/register/all.go

@@ -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"