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.

Sign up for GitHub

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-9677] add GetModelsFromModules to robot interface #4676

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+100 −0
Open

[RSDK-9677] add GetModelsFromModules to robot interface #4676

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e9cf683
add GetModelsFromModules to robot interface
JohnN193 Jan 3, 2025
51e5f87
Merge branch 'main' of github.com:viamrobotics/rdk into getModels
JohnN193 Jan 7, 2025
fc99a70
bump api
JohnN193 Jan 7, 2025
1b2ac32
Merge branch 'main' of github.com:viamrobotics/rdk into getModels
JohnN193 Jan 7, 2025
28d46db
move to mod manager and add client code
JohnN193 Jan 7, 2025
d79a1c7
add comment
JohnN193 Jan 7, 2025
d00abde
Merge branch 'main' of github.com:viamrobotics/rdk into getModels
JohnN193 Jan 8, 2025
67906bb
add server code and lint
JohnN193 Jan 8, 2025
b1a617b
lint
JohnN193 Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
10 changes: 10 additions & 0 deletions module/modmanager/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions resource/discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ type (
Query DiscoveryQuery
Cause error
}

ModuleModelDiscovery struct {
ModuleName string
API API
Model Model
FromLocalModule bool
}
)

func (e *DiscoverError) Error() string {
Expand Down
18 changes: 18 additions & 0 deletions robot/impl/local_robot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"go.viam.com/rdk/utils"
)

var _ = robot.LocalRobot(&localRobot{})

Check failure on line 46 in robot/impl/local_robot.go

View workflow job for this annotation

GitHub Actions / macos / build 

cannot convert &localRobot{} (value of type *localRobot) to type robot.LocalRobot: *localRobot does not implement robot.LocalRobot (wrong type for method GetModelsFromModules)

// localRobot satisfies robot.LocalRobot and defers most
// logic to its manager.
Expand Down Expand Up @@ -395,7 +395,7 @@
} else {
heartbeatWindow = cfg.Network.Sessions.HeartbeatWindow
}
r.sessionManager = robot.NewSessionManager(r, heartbeatWindow)

Check failure on line 398 in robot/impl/local_robot.go

View workflow job for this annotation

GitHub Actions / macos / build 

cannot use r (variable of type *localRobot) as robot.Robot value in argument to robot.NewSessionManager: *localRobot does not implement robot.Robot (wrong type for method GetModelsFromModules)

var successful bool
defer func() {
Expand Down Expand Up @@ -435,7 +435,7 @@

// we assume these never appear in our configs and as such will not be removed from the
// resource graph
r.webSvc = web.New(r, logger, rOpts.webOptions...)

Check failure on line 438 in robot/impl/local_robot.go

View workflow job for this annotation

GitHub Actions / macos / build 

cannot use r (variable of type *localRobot) as robot.Robot value in argument to web.New: *localRobot does not implement robot.Robot (wrong type for method GetModelsFromModules)
r.frameSvc, err = framesystem.New(ctx, resource.Dependencies{}, logger)
if err != nil {
return nil, err
Expand Down Expand Up @@ -511,7 +511,7 @@
}

successful = true
return r, nil

Check failure on line 514 in robot/impl/local_robot.go

View workflow job for this annotation

GitHub Actions / macos / build 

cannot use r (variable of type *localRobot) as robot.LocalRobot value in return statement: *localRobot does not implement robot.LocalRobot (wrong type for method GetModelsFromModules)
}

// New returns a new robot with parts sourced from the given config.
Expand Down Expand Up @@ -1065,6 +1065,24 @@
}
}

func (r *localRobot) GetModelsFromModules(ctx context.Context) ([]*resource.ModuleModelDiscovery, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cheukt @zaporter-work wasn't sure whether everything should live at the robot level or if I should add something to the module manager to better surface these messages.

Also not sure who I should be having to review this stuff, I assumed it would be cheuk but if theres a more appropriate person lmk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having more of the logic in the modmanager would make a bit more sense and make it easier to test. I can review but I didn't implement the original code nor am I the end consumer, would be good to have @zaporter-work as a reviewer too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk moved to module manager. Just need to add tests for the server & manager and it should be ready

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,
Expand Down Expand Up @@ -1466,7 +1484,7 @@
logger.Warnf("sensor_name %s in maintenance config is not in a supported format", cfg.MaintenanceConfig.SensorName)
return true
}
sensorComponent, err := robot.ResourceFromRobot[sensor.Sensor](r, name)

Check failure on line 1487 in robot/impl/local_robot.go

View workflow job for this annotation

GitHub Actions / macos / build 

cannot use r (variable of type *localRobot) as robot.Robot value in argument to robot.ResourceFromRobot[sensor.Sensor]: *localRobot does not implement robot.Robot (wrong type for method GetModelsFromModules)
if err != nil {
logger.Warnf("%s, Starting reconfiguration", err.Error())
return true
Expand Down
4 changes: 4 additions & 0 deletions robot/robot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading