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-447] Opt-in to implicit dependencies for all components #1488

Merged
merged 18 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions components/arm/wrapper/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/edaniels/golog"
commonpb "go.viam.com/api/common/v1"
pb "go.viam.com/api/component/arm/v1"
"go.viam.com/utils"

"go.viam.com/rdk/components/arm"
"go.viam.com/rdk/components/generic"
Expand All @@ -27,6 +28,16 @@ type AttrConfig struct {
ArmName string `json:"arm-name"`
}

// Validate ensures all parts of the config are valid.
func (cfg *AttrConfig) Validate(path string) ([]string, error) {
var deps []string
if cfg.ArmName == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "arm-name")
}
deps = append(deps, cfg.ArmName)
return deps, nil
}

func init() {
registry.RegisterComponent(arm.Subtype, ModelName, registry.Component{
RobotConstructor: func(ctx context.Context, r robot.Robot, config config.Component, logger golog.Logger) (interface{}, error) {
Expand Down
75 changes: 40 additions & 35 deletions components/base/wheeled/wheeled_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,46 @@ import (
rdkutils "go.viam.com/rdk/utils"
)

// Config is how you configure a wheeled base.
type Config struct {
WidthMM int `json:"width_mm"`
WheelCircumferenceMM int `json:"wheel_circumference_mm"`
SpinSlipFactor float64 `json:"spin_slip_factor,omitempty"`
Left []string `json:"left"`
Right []string `json:"right"`
}

// Validate ensures all parts of the config are valid.
func (config *Config) Validate(path string) ([]string, error) {
var deps []string

if config.WidthMM == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "width_mm")
}

if config.WheelCircumferenceMM == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "wheel_circumference_mm")
}
randhid marked this conversation as resolved.
Show resolved Hide resolved

if len(config.Left) == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "left")
}
if len(config.Right) == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "right")
}

if len(config.Left) != len(config.Right) {
return nil, utils.NewConfigValidationError(path,
fmt.Errorf("left and right need to have the same number of motors, not %d vs %d",
len(config.Left), len(config.Right)))
}

deps = append(deps, config.Left...)
deps = append(deps, config.Right...)

return deps, nil
}

func init() {
wheeledBaseComp := registry.Component{
Constructor: func(
Expand Down Expand Up @@ -277,41 +317,6 @@ func (base *wheeledBase) Width(ctx context.Context) (int, error) {
return base.widthMm, nil
}

// Config is how you configure a wheeled base.
type Config struct {
WidthMM int `json:"width_mm"`
WheelCircumferenceMM int `json:"wheel_circumference_mm"`
SpinSlipFactor float64 `json:"spin_slip_factor,omitempty"`
Left []string `json:"left"`
Right []string `json:"right"`
}

// Validate ensures all parts of the config are valid.
func (config *Config) Validate(path string) ([]string, error) {
var deps []string

if config.WidthMM == 0 {
return nil, errors.New("need a width_mm for a wheeled base")
}

if config.WheelCircumferenceMM == 0 {
return nil, errors.New("need a wheel_circumference_mm for a wheeled base")
}

if len(config.Left) == 0 || len(config.Right) == 0 {
return nil, errors.New("need left and right motors")
}

if len(config.Left) != len(config.Right) {
return nil, fmt.Errorf("left and right need to have the same number of motors, not %d vs %d", len(config.Left), len(config.Right))
}

deps = append(deps, config.Left...)
deps = append(deps, config.Right...)

return deps, nil
}

// CreateWheeledBase returns a new wheeled base defined by the given config.
func CreateWheeledBase(
ctx context.Context,
Expand Down
8 changes: 4 additions & 4 deletions components/base/wheeled/wheeled_base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,22 @@ func TestValidate(t *testing.T) {
cfg := &Config{}
deps, err := cfg.Validate("path")
test.That(t, deps, test.ShouldBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, "need a width_mm for a wheeled base")
test.That(t, err.Error(), test.ShouldContainSubstring, "\"width_mm\" is required")

cfg.WidthMM = 100
deps, err = cfg.Validate("path")
test.That(t, deps, test.ShouldBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, "need a wheel_circumference_mm for a wheeled base")
test.That(t, err.Error(), test.ShouldContainSubstring, "\"wheel_circumference_mm\" is required")

cfg.WheelCircumferenceMM = 1000
deps, err = cfg.Validate("path")
test.That(t, deps, test.ShouldBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, "need left and right motors")
test.That(t, err.Error(), test.ShouldContainSubstring, "\"left\" is required")

cfg.Left = []string{"fl-m", "bl-m"}
deps, err = cfg.Validate("path")
test.That(t, deps, test.ShouldBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, "need left and right motors")
test.That(t, err.Error(), test.ShouldContainSubstring, "\"right\" is required")

cfg.Right = []string{"fr-m"}
deps, err = cfg.Validate("path")
Expand Down
11 changes: 11 additions & 0 deletions components/board/arduino/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/edaniels/golog"
"github.com/pkg/errors"
rdkutils "go.viam.com/utils"

"go.viam.com/rdk/components/board"
"go.viam.com/rdk/components/encoder"
Expand Down Expand Up @@ -90,6 +91,16 @@ type EncoderConfig struct {
MotorName string `json:"motor_name"`
}

// Validate ensures all parts of the config are valid.
func (cfg *EncoderConfig) Validate(path string) ([]string, error) {
var deps []string
if cfg.BoardName == "" {
return nil, rdkutils.NewConfigValidationFieldRequiredError(path, "board")
}
deps = append(deps, cfg.BoardName)
return deps, nil
}

// TicksCount returns number of ticks since last zeroing.
func (e *Encoder) TicksCount(ctx context.Context, extra map[string]interface{}) (int64, error) {
res, err := e.board.runCommand("motor-position " + e.name)
Expand Down
50 changes: 26 additions & 24 deletions components/board/hat/pca9685/pca9685.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,32 @@ var (
_ = board.GPIOPin(&gpioPin{})
)

// Config describes a PCA9685 board attached to some other board via I2C.
type Config struct {
BoardName string `json:"board_name"`
I2CName string `json:"i2c_name"`
I2CAddress *int `json:"i2c_address"`
}

// Validate ensures all parts of the config are valid.
func (config *Config) Validate(path string) ([]string, error) {
var deps []string
if config.BoardName == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "board_name")
}
if config.I2CName == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "i2c_name")
}
if config.I2CAddress == nil {
config.I2CAddress = &defaultAddr
}
if *config.I2CAddress < 0 || *config.I2CAddress > 255 {
return nil, utils.NewConfigValidationError(path, errors.New("i2c_address must be an unsigned byte"))
}
deps = append(deps, config.BoardName)
return deps, nil
}

func init() {
registry.RegisterComponent(
board.Subtype,
Expand Down Expand Up @@ -68,30 +94,6 @@ func init() {
&Config{})
}

// Config describes a PCA9685 board attached to some other board via I2C.
type Config struct {
BoardName string `json:"board_name"`
I2CName string `json:"i2c_name"`
I2CAddress *int `json:"i2c_address"`
}

// Validate ensures all parts of the config are valid.
func (config *Config) Validate(path string) error {
if config.BoardName == "" {
return utils.NewConfigValidationFieldRequiredError(path, "board_name")
}
if config.I2CName == "" {
return utils.NewConfigValidationFieldRequiredError(path, "i2c_name")
}
if config.I2CAddress == nil {
config.I2CAddress = &defaultAddr
}
if *config.I2CAddress < 0 || *config.I2CAddress > 255 {
return utils.NewConfigValidationError(path, errors.New("i2c_address must be an unsigned byte"))
}
return nil
}

// PCA9685 is a general purpose 16-channel 12-bit PWM controller.
type PCA9685 struct {
generic.Unimplemented
Expand Down
14 changes: 14 additions & 0 deletions components/camera/videosource/align.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/pkg/errors"
"go.opencensus.io/trace"
"go.uber.org/multierr"
"go.viam.com/utils"

"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/config"
Expand Down Expand Up @@ -148,6 +149,19 @@ type alignAttrs struct {
Warp interface{} `json:"warp,omitempty"`
}

func (cfg *alignAttrs) Validate(path string) ([]string, error) {
var deps []string
if cfg.Color == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "color_camera_name")
}
deps = append(deps, cfg.Color)
if cfg.Depth == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "depth_camera_name")
}
deps = append(deps, cfg.Depth)
return deps, nil
}

// alignColorDepth takes a color and depth image source and aligns them together.
type alignColorDepth struct {
color, depth gostream.VideoStream
Expand Down
11 changes: 11 additions & 0 deletions components/camera/videosource/join_pointcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/golang/geo/r3"
"github.com/pkg/errors"
"go.opencensus.io/trace"
"go.viam.com/utils"

"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/components/generic"
Expand Down Expand Up @@ -70,6 +71,16 @@ type JoinAttrs struct {
Closeness float64 `json:"closeness_mm"`
}

// Validate ensures all parts of the config are valid.
func (cfg *JoinAttrs) Validate(path string) ([]string, error) {
var deps []string
if len(cfg.SourceCameras) == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "source_cameras")
}
deps = append(deps, cfg.SourceCameras...)
return deps, nil
}

type (
// MergeMethodType Defines which strategy is used for merging.
MergeMethodType string
Expand Down
17 changes: 10 additions & 7 deletions components/gripper/softrobotics/gripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,27 @@ type AttrConfig struct {
}

// Validate ensures all parts of the config are valid.
func (cfg *AttrConfig) Validate(path string) error {
func (cfg *AttrConfig) Validate(path string) ([]string, error) {
var deps []string
if cfg.Board == "" {
return utils.NewConfigValidationFieldRequiredError(path, "board")
return nil, utils.NewConfigValidationFieldRequiredError(path, "board")
}
if cfg.Open == "" {
return utils.NewConfigValidationFieldRequiredError(path, "open")
return nil, utils.NewConfigValidationFieldRequiredError(path, "open")
}
if cfg.Close == "" {
return utils.NewConfigValidationFieldRequiredError(path, "close")
return nil, utils.NewConfigValidationFieldRequiredError(path, "close")
}
if cfg.Power == "" {
return utils.NewConfigValidationFieldRequiredError(path, "power")
return nil, utils.NewConfigValidationFieldRequiredError(path, "power")
}

if cfg.AnalogReader != "psi" {
Copy link
Member

Choose a reason for hiding this comment

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

(unrelated to this PR) That's a very strange requirement. If this doesn't make sense to you either, can you please open a ticket to remove this requirement?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not familiar with the softrobotic gripper, looks like its supposed to be a pin so make sense its required

Copy link
Member

Choose a reason for hiding this comment

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

This is because of the ADC magic we do to make this gripper work with a pi. We can remove all these requirements and return warnings if we ever want to break this code.

return utils.NewConfigValidationError(path, errors.Errorf("analog_reader %s on board must be created and called 'psi'", cfg.AnalogReader))
return nil, utils.NewConfigValidationError(path,
errors.Errorf("analog_reader %s on board must be created and called 'psi'", cfg.AnalogReader))
}
return nil
deps = append(deps, cfg.Board)
return deps, nil
}

func init() {
Expand Down
10 changes: 6 additions & 4 deletions components/gripper/vgripper/v1/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,17 @@ type AttrConfig struct {
}

// Validate ensures all parts of the config are valid.
func (config *AttrConfig) Validate(path string) error {
func (config *AttrConfig) Validate(path string) ([]string, error) {
var deps []string
if config.Board == "" {
return utils.NewConfigValidationFieldRequiredError(path, "board")
return nil, utils.NewConfigValidationFieldRequiredError(path, "board")
}

if config.PressureLimit == 0 {
return utils.NewConfigValidationFieldRequiredError(path, "pressure_limit")
return nil, utils.NewConfigValidationFieldRequiredError(path, "pressure_limit")
}
return nil
deps = append(deps, config.Board)
return deps, nil
}

func init() {
Expand Down
8 changes: 5 additions & 3 deletions components/gripper/yahboom/dofbot.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ type AttrConfig struct {
}

// Validate ensures all parts of the config are valid.
func (config *AttrConfig) Validate(path string) error {
func (config *AttrConfig) Validate(path string) ([]string, error) {
var deps []string
if config.Arm == "" {
return gutils.NewConfigValidationFieldRequiredError(path, "arm")
return nil, gutils.NewConfigValidationFieldRequiredError(path, "arm")
}
return nil
deps = append(deps, config.Arm)
return deps, nil
}

func init() {
Expand Down
27 changes: 20 additions & 7 deletions components/input/gpio/gpio.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ import (

const modelName = "gpio"

// Config is the overall config.
type Config struct {
Board string `json:"board"`
Buttons map[string]ButtonConfig `json:"buttons"`
Axes map[string]AxisConfig `json:"axes"`
}

// Validate ensures all parts of the config are valid.
func (config *Config) Validate(path string) ([]string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why no buttons and axes validation? Should they be marked optional?

Copy link
Member

Choose a reason for hiding this comment

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

Honestly would have to ask @Otterverse. I don't think anyone in nyc has used a gpio input controller.

Copy link
Member

@randhid randhid Oct 19, 2022

Choose a reason for hiding this comment

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

Since he's on PTO, I looked at the file, I suggest omitting them both as empty by default, and only returning an error or warning if both Buttons and Axes are empty. There can be a use case where you use the joystick or the buttons only. @npmenard

var deps []string
if config.Board == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "board")
}
if len(config.Axes) == 0 && len(config.Buttons) == 0 {
return nil, utils.NewConfigValidationError(path, errors.New("buttons and axes cannot be both empty"))
}
deps = append(deps, config.Board)
return deps, nil
}

func init() {
registry.RegisterComponent(input.Subtype, modelName, registry.Component{Constructor: NewGPIOController})

Expand Down Expand Up @@ -72,13 +92,6 @@ func NewGPIOController(ctx context.Context, deps registry.Dependencies, config c
return &c, nil
}

// Config is the overall config.
type Config struct {
Board string `json:"board"`
Buttons map[string]ButtonConfig `json:"buttons"`
Axes map[string]AxisConfig `json:"axes"`
}

// AxisConfig is a subconfig for axes.
type AxisConfig struct {
Control input.Control `json:"control"`
Expand Down
Loading