From e9cf683a8f950de4d8930eaa5fb8e1223f86b966 Mon Sep 17 00:00:00 2001 From: John Date: Fri, 3 Jan 2025 17:10:05 -0500 Subject: [PATCH 01/12] add GetModelsFromModules to robot interface --- go.mod | 2 ++ go.sum | 4 ++-- module/modmanager/manager.go | 10 ++++++++++ resource/discovery.go | 7 +++++++ robot/impl/local_robot.go | 18 ++++++++++++++++++ robot/robot.go | 4 ++++ 6 files changed, 43 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index ae0d6f3459a..2449499cb82 100644 --- a/go.mod +++ b/go.mod @@ -431,3 +431,5 @@ require ( github.com/ziutek/mymysql v1.5.4 // indirect golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e ) + +replace go.viam.com/api => github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 diff --git a/go.sum b/go.sum index ff9673c7a94..d7f1ab3c889 100644 --- a/go.sum +++ b/go.sum @@ -788,6 +788,8 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks= +github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 h1:V2fQZbYvvjDK/oz7LxluzuKXdFGljuvbpJhs7cqT2jo= +github.com/johnn193/api v0.0.0-20241231164642-99f059defc82/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/jonboulle/clockwork v0.3.0 h1:9BSCMi8C+0qdApAp4auwX0RkLGUjs956h0EkuQymUhg= github.com/jonboulle/clockwork v0.3.0/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8= @@ -1513,8 +1515,6 @@ go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= -go.viam.com/api v0.1.372 h1:Al9P7yojBDdNVAF7nrr5BAbzCvb+vrSp8N7BitbV0mQ= -go.viam.com/api v0.1.372/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls= go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug= go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI= go.viam.com/utils v0.1.118 h1:Kp6ebrCBiYReeSC1XnWPTjtBJoTUsQ6YWAomQkQF/mE= diff --git a/module/modmanager/manager.go b/module/modmanager/manager.go index 849fd63f60d..f30f40eb802 100644 --- a/module/modmanager/manager.go +++ b/module/modmanager/manager.go @@ -587,6 +587,16 @@ func (mgr *Manager) Configs() []config.Module { return configs } +// // AllModels returns a slice of config.Module representing the currently available models from the currently managed modules +// func (mgr *Manager) AllModels() []config.Module { +// var configs []config.Module +// mgr.modules.Range(func(_ string, mod *module) bool { +// configs = append(configs, mod.cfg) +// return true +// }) +// return configs +// } + // Provides returns true if a component/service config WOULD be handled by a module. func (mgr *Manager) Provides(conf resource.Config) bool { _, ok := mgr.getModule(conf) diff --git a/resource/discovery.go b/resource/discovery.go index 9b2a11f6cba..fa6477b2315 100644 --- a/resource/discovery.go +++ b/resource/discovery.go @@ -33,6 +33,13 @@ type ( Query DiscoveryQuery Cause error } + + ModuleModelDiscovery struct { + ModuleName string + API API + Model Model + FromLocalModule bool + } ) func (e *DiscoverError) Error() string { diff --git a/robot/impl/local_robot.go b/robot/impl/local_robot.go index 0822cfcdef7..41ce11dbefe 100644 --- a/robot/impl/local_robot.go +++ b/robot/impl/local_robot.go @@ -1065,6 +1065,24 @@ func (r *localRobot) discoverRobotInternals(query resource.DiscoveryQuery) (inte } } +func (r *localRobot) GetModelsFromModules(ctx context.Context) ([]*resource.ModuleModelDiscovery, error) { + moduleTypes := map[string]config.ModuleType{} + models := []*resource.ModuleModelDiscovery{} + for _, moduleConfig := range r.manager.moduleManager.Configs() { + moduleName := moduleConfig.Name + moduleTypes[moduleName] = moduleConfig.Type + } + for moduleName, handleMap := range r.manager.moduleManager.Handles() { + for api, handle := range handleMap { + for _, model := range handle { + modelModel := resource.ModuleModelDiscovery{ModuleName: moduleName, Model: model, FromLocalModule: moduleTypes[moduleName] == config.ModuleTypeLocal, API: api.API} + models = append(models, &modelModel) + } + } + } + return models, nil +} + func dialRobotClient( ctx context.Context, config config.Remote, diff --git a/robot/robot.go b/robot/robot.go index f57330d0852..64482b63af6 100644 --- a/robot/robot.go +++ b/robot/robot.go @@ -88,6 +88,10 @@ type Robot interface { // Only implemented for webcam cameras in builtin components. DiscoverComponents(ctx context.Context, qs []resource.DiscoveryQuery) ([]resource.Discovery, error) + // GetModelsFromModules returns a list of models supported by the configured modules, + // and specifies whether the models are from a local or registry module. + GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) + // RemoteByName returns a remote robot by name. RemoteByName(name string) (Robot, bool) From fc99a702a750d94d10520e267f131338967eb85a Mon Sep 17 00:00:00 2001 From: John Date: Tue, 7 Jan 2025 15:22:43 -0500 Subject: [PATCH 02/12] bump api --- go.mod | 4 +--- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index f1c9edc29f3..83e5680b9af 100644 --- a/go.mod +++ b/go.mod @@ -75,7 +75,7 @@ require ( go.uber.org/atomic v1.11.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 - go.viam.com/api v0.1.378 + go.viam.com/api v0.1.380 go.viam.com/test v1.2.4 go.viam.com/utils v0.1.118 goji.io v2.0.2+incompatible @@ -431,5 +431,3 @@ require ( github.com/ziutek/mymysql v1.5.4 // indirect golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e ) - -replace go.viam.com/api => github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 diff --git a/go.sum b/go.sum index d7f1ab3c889..55b2f7196c5 100644 --- a/go.sum +++ b/go.sum @@ -788,8 +788,6 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks= -github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 h1:V2fQZbYvvjDK/oz7LxluzuKXdFGljuvbpJhs7cqT2jo= -github.com/johnn193/api v0.0.0-20241231164642-99f059defc82/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/jonboulle/clockwork v0.3.0 h1:9BSCMi8C+0qdApAp4auwX0RkLGUjs956h0EkuQymUhg= github.com/jonboulle/clockwork v0.3.0/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8= @@ -1515,6 +1513,8 @@ go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= +go.viam.com/api v0.1.380 h1:VgRHDlPBku+kjIp4omxmPNmRVZezytFUUOFJ2xpRFR8= +go.viam.com/api v0.1.380/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls= go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug= go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI= go.viam.com/utils v0.1.118 h1:Kp6ebrCBiYReeSC1XnWPTjtBJoTUsQ6YWAomQkQF/mE= From 28d46db20e0bd79dbc0e04ed1574d804abc1b626 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 7 Jan 2025 18:22:10 -0500 Subject: [PATCH 03/12] move to mod manager and add client code --- module/modmanager/manager.go | 27 ++++++++++++++++++--------- module/modmaninterface/interface.go | 1 + robot/client/client.go | 22 ++++++++++++++++++++++ robot/impl/local_robot.go | 18 ++---------------- robot/impl/resource_manager_test.go | 9 +++++++++ 5 files changed, 52 insertions(+), 25 deletions(-) diff --git a/module/modmanager/manager.go b/module/modmanager/manager.go index ddadee1831d..511101802ff 100644 --- a/module/modmanager/manager.go +++ b/module/modmanager/manager.go @@ -596,15 +596,24 @@ func (mgr *Manager) Configs() []config.Module { return configs } -// // AllModels returns a slice of config.Module representing the currently available models from the currently managed modules -// func (mgr *Manager) AllModels() []config.Module { -// var configs []config.Module -// mgr.modules.Range(func(_ string, mod *module) bool { -// configs = append(configs, mod.cfg) -// return true -// }) -// return configs -// } +// AllModels returns a slice of resource.ModuleModelDiscovery representing the currently available models from the currently managed modules +func (mgr *Manager) AllModels() []resource.ModuleModelDiscovery { + moduleTypes := map[string]config.ModuleType{} + models := []resource.ModuleModelDiscovery{} + for _, moduleConfig := range mgr.Configs() { + moduleName := moduleConfig.Name + moduleTypes[moduleName] = moduleConfig.Type + } + for moduleName, handleMap := range mgr.Handles() { + for api, handle := range handleMap { + for _, model := range handle { + modelModel := resource.ModuleModelDiscovery{ModuleName: moduleName, Model: model, FromLocalModule: moduleTypes[moduleName] == config.ModuleTypeLocal, API: api.API} + models = append(models, modelModel) + } + } + } + return models +} // Provides returns true if a component/service config WOULD be handled by a module. func (mgr *Manager) Provides(conf resource.Config) bool { diff --git a/module/modmaninterface/interface.go b/module/modmaninterface/interface.go index 22f589ccee4..411880657da 100644 --- a/module/modmaninterface/interface.go +++ b/module/modmaninterface/interface.go @@ -24,6 +24,7 @@ type ModuleManager interface { CleanModuleDataDirectory() error Configs() []config.Module + AllModels() []resource.ModuleModelDiscovery Provides(cfg resource.Config) bool Handles() map[string]module.HandlerMap diff --git a/robot/client/client.go b/robot/client/client.go index 32a641052a2..59c3648f337 100644 --- a/robot/client/client.go +++ b/robot/client/client.go @@ -876,6 +876,28 @@ func (rc *RobotClient) DiscoverComponents(ctx context.Context, qs []resource.Dis return discoveries, nil } +func (rc *RobotClient) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { + resp, err := rc.client.GetModelsFromModules(ctx, &pb.GetModelsFromModulesRequest{}) + if err != nil { + return nil, err + } + protoModels := resp.GetModels() + models := []resource.ModuleModelDiscovery{} + for _, protoModel := range protoModels { + modelTriplet, err := resource.NewModelFromString(protoModel.Model) + if err != nil { + return nil, err + } + api, err := resource.NewAPIFromString(protoModel.Api) + if err != nil { + return nil, err + } + model := resource.ModuleModelDiscovery{ModuleName: protoModel.ModuleName, Model: modelTriplet, API: api, FromLocalModule: protoModel.FromLocalModule} + models = append(models, model) + } + return models, nil +} + // FrameSystemConfig returns the configuration of the frame system of a given machine. // // frameSystem, err := machine.FrameSystemConfig(context.Background(), nil) diff --git a/robot/impl/local_robot.go b/robot/impl/local_robot.go index 9c3cd9e5cfa..577636e78bb 100644 --- a/robot/impl/local_robot.go +++ b/robot/impl/local_robot.go @@ -1079,22 +1079,8 @@ func (r *localRobot) discoverRobotInternals(query resource.DiscoveryQuery) (inte } } -func (r *localRobot) GetModelsFromModules(ctx context.Context) ([]*resource.ModuleModelDiscovery, error) { - moduleTypes := map[string]config.ModuleType{} - models := []*resource.ModuleModelDiscovery{} - for _, moduleConfig := range r.manager.moduleManager.Configs() { - moduleName := moduleConfig.Name - moduleTypes[moduleName] = moduleConfig.Type - } - for moduleName, handleMap := range r.manager.moduleManager.Handles() { - for api, handle := range handleMap { - for _, model := range handle { - modelModel := resource.ModuleModelDiscovery{ModuleName: moduleName, Model: model, FromLocalModule: moduleTypes[moduleName] == config.ModuleTypeLocal, API: api.API} - models = append(models, &modelModel) - } - } - } - return models, nil +func (r *localRobot) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { + return r.manager.moduleManager.AllModels(), nil } func dialRobotClient( diff --git a/robot/impl/resource_manager_test.go b/robot/impl/resource_manager_test.go index 4a2e9c39c7a..2b721bea120 100644 --- a/robot/impl/resource_manager_test.go +++ b/robot/impl/resource_manager_test.go @@ -1894,6 +1894,15 @@ func (rr *dummyRobot) DiscoverComponents(ctx context.Context, qs []resource.Disc return rr.robot.DiscoverComponents(ctx, qs) } +func (rr *dummyRobot) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { + rr.mu.Lock() + defer rr.mu.Unlock() + if rr.offline { + return nil, errors.New("offline") + } + return rr.robot.GetModelsFromModules(ctx) +} + func (rr *dummyRobot) RemoteNames() []string { return nil } From d79a1c79a3b56d28509883c63319cab1ec840ef4 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 7 Jan 2025 18:22:55 -0500 Subject: [PATCH 04/12] add comment --- robot/client/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/robot/client/client.go b/robot/client/client.go index 59c3648f337..124ee478ab7 100644 --- a/robot/client/client.go +++ b/robot/client/client.go @@ -876,6 +876,9 @@ func (rc *RobotClient) DiscoverComponents(ctx context.Context, qs []resource.Dis return discoveries, nil } +// GetModelsFromModules returns the available models from the configured modules on a given machine. +// +// models, err := machine.GetModelsFromModules(context.Background()) func (rc *RobotClient) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { resp, err := rc.client.GetModelsFromModules(ctx, &pb.GetModelsFromModulesRequest{}) if err != nil { From 67906bb068afe01daf6d45862ebd62e6378d2914 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 8 Jan 2025 12:04:29 -0500 Subject: [PATCH 05/12] add server code and lint --- module/modmanager/manager.go | 8 ++++++-- resource/discovery.go | 11 +++++++++++ robot/client/client.go | 5 ++++- robot/server/server.go | 13 +++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/module/modmanager/manager.go b/module/modmanager/manager.go index 511101802ff..0ae6de23387 100644 --- a/module/modmanager/manager.go +++ b/module/modmanager/manager.go @@ -596,7 +596,8 @@ func (mgr *Manager) Configs() []config.Module { return configs } -// AllModels returns a slice of resource.ModuleModelDiscovery representing the currently available models from the currently managed modules +// AllModels returns a slice of resource.ModuleModelDiscovery representing the available models +// from the currently managed modules. func (mgr *Manager) AllModels() []resource.ModuleModelDiscovery { moduleTypes := map[string]config.ModuleType{} models := []resource.ModuleModelDiscovery{} @@ -607,7 +608,10 @@ func (mgr *Manager) AllModels() []resource.ModuleModelDiscovery { for moduleName, handleMap := range mgr.Handles() { for api, handle := range handleMap { for _, model := range handle { - modelModel := resource.ModuleModelDiscovery{ModuleName: moduleName, Model: model, FromLocalModule: moduleTypes[moduleName] == config.ModuleTypeLocal, API: api.API} + modelModel := resource.ModuleModelDiscovery{ + ModuleName: moduleName, Model: model, API: api.API, + FromLocalModule: moduleTypes[moduleName] == config.ModuleTypeLocal, + } models = append(models, modelModel) } } diff --git a/resource/discovery.go b/resource/discovery.go index fa6477b2315..40decc4ea01 100644 --- a/resource/discovery.go +++ b/resource/discovery.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + pb "go.viam.com/api/robot/v1" + "go.viam.com/rdk/logging" ) @@ -34,6 +36,7 @@ type ( Cause error } + // ModuleModelDiscovery holds the API and Model information of models within a module. ModuleModelDiscovery struct { ModuleName string API API @@ -42,6 +45,14 @@ type ( } ) +// ToProto converts a ModuleModelDiscovery into the equivalent proto message +func (mm *ModuleModelDiscovery) ToProto() *pb.ModuleModel { + return &pb.ModuleModel{ + Model: mm.Model.String(), Api: mm.API.String(), ModuleName: mm.ModuleName, + FromLocalModule: mm.FromLocalModule, + } +} + func (e *DiscoverError) Error() string { return fmt.Sprintf("failed to get discovery for api %q and model %q error: %v", e.Query.API, e.Query.Model, e.Cause) } diff --git a/robot/client/client.go b/robot/client/client.go index 124ee478ab7..189f87b2a2a 100644 --- a/robot/client/client.go +++ b/robot/client/client.go @@ -895,7 +895,10 @@ func (rc *RobotClient) GetModelsFromModules(ctx context.Context) ([]resource.Mod if err != nil { return nil, err } - model := resource.ModuleModelDiscovery{ModuleName: protoModel.ModuleName, Model: modelTriplet, API: api, FromLocalModule: protoModel.FromLocalModule} + model := resource.ModuleModelDiscovery{ + ModuleName: protoModel.ModuleName, Model: modelTriplet, API: api, + FromLocalModule: protoModel.FromLocalModule, + } models = append(models, model) } return models, nil diff --git a/robot/server/server.go b/robot/server/server.go index 86c3df4a8ee..c23e250bc40 100644 --- a/robot/server/server.go +++ b/robot/server/server.go @@ -223,6 +223,19 @@ func (s *Server) DiscoverComponents(ctx context.Context, req *pb.DiscoverCompone return &pb.DiscoverComponentsResponse{Discovery: pbDiscoveries}, nil } +// GetModelsFromModules returns all models from the currently managed modules. +func (s *Server) GetModelsFromModules(ctx context.Context, req *pb.GetModelsFromModulesRequest) (*pb.GetModelsFromModulesResponse, error) { + models, err := s.robot.GetModelsFromModules(ctx) + if err != nil { + return nil, err + } + resp := pb.GetModelsFromModulesResponse{} + for _, mm := range models { + resp.Models = append(resp.Models, mm.ToProto()) + } + return &resp, nil +} + // FrameSystemConfig returns the info of each individual part that makes up the frame system. func (s *Server) FrameSystemConfig(ctx context.Context, req *pb.FrameSystemConfigRequest) (*pb.FrameSystemConfigResponse, error) { fsCfg, err := s.robot.FrameSystemConfig(ctx) From b1a617baa7d584b23e3cc67a3152a56d41d18cd5 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 8 Jan 2025 13:05:07 -0500 Subject: [PATCH 06/12] lint --- resource/discovery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resource/discovery.go b/resource/discovery.go index 40decc4ea01..f4b6c8e686b 100644 --- a/resource/discovery.go +++ b/resource/discovery.go @@ -45,7 +45,7 @@ type ( } ) -// ToProto converts a ModuleModelDiscovery into the equivalent proto message +// ToProto converts a ModuleModelDiscovery into the equivalent proto message. func (mm *ModuleModelDiscovery) ToProto() *pb.ModuleModel { return &pb.ModuleModel{ Model: mm.Model.String(), Api: mm.API.String(), ModuleName: mm.ModuleName, From 45fdac8803440952bc81fc724410b7de8867f42c Mon Sep 17 00:00:00 2001 From: John Date: Wed, 15 Jan 2025 14:56:06 -0500 Subject: [PATCH 07/12] add modmanager test --- module/modmanager/manager_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/module/modmanager/manager_test.go b/module/modmanager/manager_test.go index 5ab749732f6..71b32b213bc 100644 --- a/module/modmanager/manager_test.go +++ b/module/modmanager/manager_test.go @@ -206,6 +206,24 @@ func TestModManagerFunctions(t *testing.T) { test.That(t, ok, test.ShouldBeTrue) test.That(t, reg.Constructor, test.ShouldNotBeNil) + t.Log("test AllModels") + modCfg2 := config.Module{ + Name: "simple-module2", + ExePath: modPath, + Type: config.ModuleTypeLocal, + } + err = mgr.Add(ctx, modCfg2) + test.That(t, err, test.ShouldBeNil) + expectedModels := []resource.ModuleModelDiscovery{{ModuleName: "simple-module", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: false}, {ModuleName: "simple-module2", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: true}} + models := mgr.AllModels() + test.That(t, models, test.ShouldResemble, expectedModels) + t.Log("test Provides") ok = mgr.Provides(cfgCounter1) test.That(t, ok, test.ShouldBeTrue) From c7bc457812c55d011d4b725093abda75b8fe221b Mon Sep 17 00:00:00 2001 From: John Date: Wed, 15 Jan 2025 15:40:57 -0500 Subject: [PATCH 08/12] add server client tests --- module/modmanager/manager_test.go | 21 ++++++++---- robot/client/client_test.go | 54 +++++++++++++++++++++++++++++++ robot/server/server_test.go | 43 ++++++++++++++++++++++++ testutils/inject/robot.go | 39 ++++++++++++++-------- 4 files changed, 136 insertions(+), 21 deletions(-) diff --git a/module/modmanager/manager_test.go b/module/modmanager/manager_test.go index 71b32b213bc..ae94b144348 100644 --- a/module/modmanager/manager_test.go +++ b/module/modmanager/manager_test.go @@ -214,13 +214,20 @@ func TestModManagerFunctions(t *testing.T) { } err = mgr.Add(ctx, modCfg2) test.That(t, err, test.ShouldBeNil) - expectedModels := []resource.ModuleModelDiscovery{{ModuleName: "simple-module", - API: resource.NewAPI("rdk", "component", "generic"), - Model: resource.NewModel("acme", "demo", "mycounter"), - FromLocalModule: false}, {ModuleName: "simple-module2", - API: resource.NewAPI("rdk", "component", "generic"), - Model: resource.NewModel("acme", "demo", "mycounter"), - FromLocalModule: true}} + expectedModels := []resource.ModuleModelDiscovery{ + { + ModuleName: "simple-module", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: false, + }, + { + ModuleName: "simple-module2", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: true, + }, + } models := mgr.AllModels() test.That(t, models, test.ShouldResemble, expectedModels) diff --git a/robot/client/client_test.go b/robot/client/client_test.go index 5258dcb2e7c..f20d03d1690 100644 --- a/robot/client/client_test.go +++ b/robot/client/client_test.go @@ -1354,6 +1354,60 @@ func TestClientDiscovery(t *testing.T) { test.That(t, err, test.ShouldBeNil) } +func TestClientGetModelsFromModules(t *testing.T) { + injectRobot := &inject.Robot{} + injectRobot.ResourceRPCAPIsFunc = func() []resource.RPCAPI { return nil } + injectRobot.ResourceNamesFunc = func() []resource.Name { + return finalResources + } + injectRobot.MachineStatusFunc = func(_ context.Context) (robot.MachineStatus, error) { + return robot.MachineStatus{State: robot.StateRunning}, nil + } + expectedModels := []resource.ModuleModelDiscovery{ + { + ModuleName: "simple-module", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: false, + }, + { + ModuleName: "simple-module2", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: true, + }, + } + injectRobot.GetModelsFromModulesFunc = func(context.Context) ([]resource.ModuleModelDiscovery, error) { + return expectedModels, nil + } + + gServer := grpc.NewServer() + pb.RegisterRobotServiceServer(gServer, server.New(injectRobot)) + listener, err := net.Listen("tcp", "localhost:0") + test.That(t, err, test.ShouldBeNil) + logger := logging.NewTestLogger(t) + + go gServer.Serve(listener) + defer gServer.Stop() + + client, err := New(context.Background(), listener.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + + resp, err := client.GetModelsFromModules(context.Background()) + test.That(t, err, test.ShouldBeNil) + test.That(t, len(resp), test.ShouldEqual, 2) + test.That(t, resp, test.ShouldResemble, expectedModels) + for index, model := range resp { + test.That(t, model.ModuleName, test.ShouldEqual, expectedModels[index].ModuleName) + test.That(t, model.Model, test.ShouldResemble, expectedModels[index].Model) + test.That(t, model.API, test.ShouldResemble, expectedModels[index].API) + test.That(t, model.FromLocalModule, test.ShouldEqual, expectedModels[index].FromLocalModule) + } + + err = client.Close(context.Background()) + test.That(t, err, test.ShouldBeNil) +} + func ensurePartsAreEqual(part, otherPart *referenceframe.FrameSystemPart) error { if part.FrameConfig.Name() != otherPart.FrameConfig.Name() { return fmt.Errorf("part had name %s while other part had name %s", part.FrameConfig.Name(), otherPart.FrameConfig.Name()) diff --git a/robot/server/server_test.go b/robot/server/server_test.go index 40425a8976b..c3a112ba7f5 100644 --- a/robot/server/server_test.go +++ b/robot/server/server_test.go @@ -446,6 +446,49 @@ func TestServer(t *testing.T) { }) }) + t.Run("GetModelsFromModules", func(t *testing.T) { + injectRobot := &inject.Robot{} + injectRobot.ResourceRPCAPIsFunc = func() []resource.RPCAPI { return nil } + injectRobot.ResourceNamesFunc = func() []resource.Name { return []resource.Name{} } + server := server.New(injectRobot) + + expectedModels := []resource.ModuleModelDiscovery{ + { + ModuleName: "simple-module", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: false, + }, + { + ModuleName: "simple-module2", + API: resource.NewAPI("rdk", "component", "generic"), + Model: resource.NewModel("acme", "demo", "mycounter"), + FromLocalModule: true, + }, + } + injectRobot.GetModelsFromModulesFunc = func(context.Context) ([]resource.ModuleModelDiscovery, error) { + return expectedModels, nil + } + expectedProto := []*pb.ModuleModel{expectedModels[0].ToProto(), expectedModels[1].ToProto()} + + req := &pb.GetModelsFromModulesRequest{} + resp, err := server.GetModelsFromModules(context.Background(), req) + test.That(t, err, test.ShouldBeNil) + protoModels := resp.GetModels() + test.That(t, len(protoModels), test.ShouldEqual, 2) + test.That(t, protoModels, test.ShouldResemble, expectedProto) + for index, protoModel := range protoModels { + test.That(t, protoModel.ModuleName, test.ShouldEqual, expectedProto[index].ModuleName) + test.That(t, protoModel.ModuleName, test.ShouldEqual, expectedModels[index].ModuleName) + test.That(t, protoModel.Model, test.ShouldEqual, expectedProto[index].Model) + test.That(t, protoModel.Model, test.ShouldEqual, expectedModels[index].Model.String()) + test.That(t, protoModel.Api, test.ShouldEqual, expectedProto[index].Api) + test.That(t, protoModel.Api, test.ShouldEqual, expectedModels[index].API.String()) + test.That(t, protoModel.FromLocalModule, test.ShouldEqual, expectedProto[index].FromLocalModule) + test.That(t, protoModel.FromLocalModule, test.ShouldEqual, expectedModels[index].FromLocalModule) + } + }) + t.Run("ResourceRPCSubtypes", func(t *testing.T) { injectRobot := &inject.Robot{} injectRobot.ResourceRPCAPIsFunc = func() []resource.RPCAPI { return nil } diff --git a/testutils/inject/robot.go b/testutils/inject/robot.go index 570cc8d1c2b..8b42d50c45b 100644 --- a/testutils/inject/robot.go +++ b/testutils/inject/robot.go @@ -26,20 +26,21 @@ import ( // Robot is an injected robot. type Robot struct { robot.LocalRobot - Mu sync.RWMutex // Ugly, has to be manually locked if a test means to swap funcs on an in-use robot. - DiscoverComponentsFunc func(ctx context.Context, keys []resource.DiscoveryQuery) ([]resource.Discovery, error) - RemoteByNameFunc func(name string) (robot.Robot, bool) - ResourceByNameFunc func(name resource.Name) (resource.Resource, error) - RemoteNamesFunc func() []string - ResourceNamesFunc func() []resource.Name - ResourceRPCAPIsFunc func() []resource.RPCAPI - ProcessManagerFunc func() pexec.ProcessManager - ConfigFunc func() *config.Config - LoggerFunc func() logging.Logger - CloseFunc func(ctx context.Context) error - StopAllFunc func(ctx context.Context, extra map[resource.Name]map[string]interface{}) error - FrameSystemConfigFunc func(ctx context.Context) (*framesystem.Config, error) - TransformPoseFunc func( + Mu sync.RWMutex // Ugly, has to be manually locked if a test means to swap funcs on an in-use robot. + DiscoverComponentsFunc func(ctx context.Context, keys []resource.DiscoveryQuery) ([]resource.Discovery, error) + GetModelsFromModulesFunc func(ctx context.Context) ([]resource.ModuleModelDiscovery, error) + RemoteByNameFunc func(name string) (robot.Robot, bool) + ResourceByNameFunc func(name resource.Name) (resource.Resource, error) + RemoteNamesFunc func() []string + ResourceNamesFunc func() []resource.Name + ResourceRPCAPIsFunc func() []resource.RPCAPI + ProcessManagerFunc func() pexec.ProcessManager + ConfigFunc func() *config.Config + LoggerFunc func() logging.Logger + CloseFunc func(ctx context.Context) error + StopAllFunc func(ctx context.Context, extra map[resource.Name]map[string]interface{}) error + FrameSystemConfigFunc func(ctx context.Context) (*framesystem.Config, error) + TransformPoseFunc func( ctx context.Context, pose *referenceframe.PoseInFrame, dst string, @@ -228,6 +229,16 @@ func (r *Robot) DiscoverComponents(ctx context.Context, keys []resource.Discover return r.DiscoverComponentsFunc(ctx, keys) } +// GetModelsFromModules calls the injected GetModelsFromModules or the real one. +func (r *Robot) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { + r.Mu.RLock() + defer r.Mu.RUnlock() + if r.GetModelsFromModulesFunc == nil { + return r.LocalRobot.GetModelsFromModules(ctx) + } + return r.GetModelsFromModulesFunc(ctx) +} + // FrameSystemConfig calls the injected FrameSystemConfig or the real version. func (r *Robot) FrameSystemConfig(ctx context.Context) (*framesystem.Config, error) { r.Mu.RLock() From 96489cb1ea8d5913ab6e260bb71e7c9a36fdffe2 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 15 Jan 2025 16:07:25 -0500 Subject: [PATCH 09/12] add robot impl tests --- robot/impl/discovery_test.go | 72 ++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/robot/impl/discovery_test.go b/robot/impl/discovery_test.go index 31c68be3044..f959b7104db 100644 --- a/robot/impl/discovery_test.go +++ b/robot/impl/discovery_test.go @@ -8,10 +8,19 @@ import ( modulepb "go.viam.com/api/module/v1" "go.viam.com/test" + "go.viam.com/rdk/components/base" + "go.viam.com/rdk/components/generic" "go.viam.com/rdk/config" + "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/mygizmo" + "go.viam.com/rdk/examples/customresources/models/mynavigation" + "go.viam.com/rdk/examples/customresources/models/mysum" "go.viam.com/rdk/logging" "go.viam.com/rdk/resource" "go.viam.com/rdk/robot" + "go.viam.com/rdk/services/navigation" rtestutils "go.viam.com/rdk/testutils" ) @@ -168,3 +177,66 @@ func TestDiscovery(t *testing.T) { test.That(t, len(complexHandles.Handlers), test.ShouldBeGreaterThan, 1) }) } + +func TestGetModelsFromModules(t *testing.T) { + t.Run("no modules configured", func(t *testing.T) { + r := setupLocalRobotWithFakeConfig(t) + models, err := r.GetModelsFromModules(context.Background()) + test.That(t, err, test.ShouldBeNil) + test.That(t, models, test.ShouldBeEmpty) + }) + t.Run("local and registry modules are configured", func(t *testing.T) { + r := setupLocalRobotWithFakeConfig(t) + ctx := context.Background() + + // add modules + complexPath := rtestutils.BuildTempModule(t, "examples/customresources/demos/complexmodule") + simplePath := rtestutils.BuildTempModule(t, "examples/customresources/demos/simplemodule") + cfg := &config.Config{ + Modules: []config.Module{ + { + Name: "simple", + ExePath: simplePath, + Type: config.ModuleTypeRegistry, + }, + { + Name: "complex", + ExePath: complexPath, + Type: config.ModuleTypeLocal, + }, + }, + } + r.Reconfigure(ctx, cfg) + models, err := r.GetModelsFromModules(context.Background()) + test.That(t, err, test.ShouldBeNil) + test.That(t, models, test.ShouldHaveLength, 5) + + for _, model := range models { + switch model.Model { + case mygizmo.Model: + test.That(t, model.FromLocalModule, test.ShouldEqual, true) + test.That(t, model.ModuleName, test.ShouldEqual, "complex") + test.That(t, model.API, test.ShouldResemble, gizmoapi.API) + case mysum.Model: + test.That(t, model.FromLocalModule, test.ShouldEqual, true) + test.That(t, model.ModuleName, test.ShouldEqual, "complex") + test.That(t, model.API, test.ShouldResemble, summationapi.API) + case mybase.Model: + test.That(t, model.FromLocalModule, test.ShouldEqual, true) + test.That(t, model.ModuleName, test.ShouldEqual, "complex") + test.That(t, model.API, test.ShouldResemble, base.API) + case mynavigation.Model: + test.That(t, model.FromLocalModule, test.ShouldEqual, true) + test.That(t, model.ModuleName, test.ShouldEqual, "complex") + test.That(t, model.API, test.ShouldResemble, navigation.API) + case resource.NewModel("acme", "demo", "mycounter"): + test.That(t, model.FromLocalModule, test.ShouldEqual, false) + test.That(t, model.ModuleName, test.ShouldEqual, "simple") + test.That(t, model.API, test.ShouldResemble, generic.API) + default: + t.Fail() + t.Logf("test GetModelsFromModules failure: unrecoginzed model %v", model.Model) + } + } + }) +} From 93eb088575de9f1f21578086f599f757934fad68 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 15 Jan 2025 16:49:08 -0500 Subject: [PATCH 10/12] tweak modmanager test --- module/modmanager/manager_test.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/module/modmanager/manager_test.go b/module/modmanager/manager_test.go index ae94b144348..fa1d5cea979 100644 --- a/module/modmanager/manager_test.go +++ b/module/modmanager/manager_test.go @@ -214,22 +214,23 @@ func TestModManagerFunctions(t *testing.T) { } err = mgr.Add(ctx, modCfg2) test.That(t, err, test.ShouldBeNil) - expectedModels := []resource.ModuleModelDiscovery{ - { - ModuleName: "simple-module", - API: resource.NewAPI("rdk", "component", "generic"), - Model: resource.NewModel("acme", "demo", "mycounter"), - FromLocalModule: false, - }, - { - ModuleName: "simple-module2", - API: resource.NewAPI("rdk", "component", "generic"), - Model: resource.NewModel("acme", "demo", "mycounter"), - FromLocalModule: true, - }, - } models := mgr.AllModels() - test.That(t, models, test.ShouldResemble, expectedModels) + for _, model := range models { + test.That(t, model.Model, test.ShouldEqual, resource.NewModel("acme", "demo", "mycounter")) + test.That(t, model.API, test.ShouldResemble, resource.NewAPI("rdk", "component", "generic")) + switch model.ModuleName { + case "simple-module": + test.That(t, model.FromLocalModule, test.ShouldEqual, false) + case "simple-module2": + test.That(t, model.FromLocalModule, test.ShouldEqual, true) + default: + t.Fail() + t.Logf("test AllModels failure: unrecoginzed moduleName %v", model.ModuleName) + } + } + names, err := mgr.Remove(modCfg2.Name) + test.That(t, names, test.ShouldBeEmpty) + test.That(t, err, test.ShouldBeNil) t.Log("test Provides") ok = mgr.Provides(cfgCounter1) From 30387e8f5d8e7a6a4156bc4f892727bad9b75c61 Mon Sep 17 00:00:00 2001 From: John Date: Fri, 24 Jan 2025 12:09:19 -0500 Subject: [PATCH 11/12] fix test --- module/modmanager/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/modmanager/manager_test.go b/module/modmanager/manager_test.go index fa1d5cea979..73029ea1846 100644 --- a/module/modmanager/manager_test.go +++ b/module/modmanager/manager_test.go @@ -216,7 +216,7 @@ func TestModManagerFunctions(t *testing.T) { test.That(t, err, test.ShouldBeNil) models := mgr.AllModels() for _, model := range models { - test.That(t, model.Model, test.ShouldEqual, resource.NewModel("acme", "demo", "mycounter")) + test.That(t, model.Model, test.ShouldResemble, resource.NewModel("acme", "demo", "mycounter")) test.That(t, model.API, test.ShouldResemble, resource.NewAPI("rdk", "component", "generic")) switch model.ModuleName { case "simple-module": From ebb9473a1b46ad214b2592d9a6ac226b2015bd03 Mon Sep 17 00:00:00 2001 From: JohnN193 <92045055+JohnN193@users.noreply.github.com> Date: Mon, 27 Jan 2025 13:12:34 -0500 Subject: [PATCH 12/12] Update robot/client/client.go Co-authored-by: Zack Porter <121693134+zaporter-work@users.noreply.github.com> --- robot/client/client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/robot/client/client.go b/robot/client/client.go index ddb5f646d30..43d74247771 100644 --- a/robot/client/client.go +++ b/robot/client/client.go @@ -918,8 +918,6 @@ func (rc *RobotClient) DiscoverComponents(ctx context.Context, qs []resource.Dis } // GetModelsFromModules returns the available models from the configured modules on a given machine. -// -// models, err := machine.GetModelsFromModules(context.Background()) func (rc *RobotClient) GetModelsFromModules(ctx context.Context) ([]resource.ModuleModelDiscovery, error) { resp, err := rc.client.GetModelsFromModules(ctx, &pb.GetModelsFromModulesRequest{}) if err != nil {