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-833: add motor names to motor errors #1682

Merged
merged 18 commits into from
Dec 22, 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
19 changes: 12 additions & 7 deletions components/motor/dimensionengineering/sabertooth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package dimensionengineering

import (
"context"
"errors"
"fmt"
"io"
"math"
Expand All @@ -14,6 +13,7 @@ import (
"github.com/edaniels/golog"
"github.com/jacobsa/go-serial/serial"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
utils "go.viam.com/utils"

"go.viam.com/rdk/components/motor"
Expand Down Expand Up @@ -67,6 +67,9 @@ type Motor struct {

// A manager to ensure only a single operation is happening at any given time since commands could overlap on the serial port
opMgr operation.SingleOperationManager

// Name of the motor
motorName string
}

// Config adds DimensionEngineering-specific config options.
Expand Down Expand Up @@ -127,7 +130,7 @@ func init() {
if !ok {
return nil, rdkutils.NewUnexpectedTypeError(conf, config.ConvertedAttributes)
}
return NewMotor(ctx, conf, logger)
return NewMotor(ctx, conf, config.Name, logger)
},
}
registry.RegisterComponent(motor.Subtype, modelName, _motor)
Expand Down Expand Up @@ -216,7 +219,7 @@ func (cfg *Config) validateValues() error {
}

// NewMotor returns a Sabertooth driven motor.
func NewMotor(ctx context.Context, c *Config, logger golog.Logger) (motor.LocalMotor, error) {
func NewMotor(ctx context.Context, c *Config, name string, logger golog.Logger) (motor.LocalMotor, error) {
globalMu.Lock()
defer globalMu.Unlock()

Expand Down Expand Up @@ -257,6 +260,7 @@ func NewMotor(ctx context.Context, c *Config, logger golog.Logger) (motor.LocalM
dirFlip: c.DirectionFlip,
minPowerPct: c.MinPowerPct,
maxPowerPct: c.MaxPowerPct,
motorName: name,
}

if err := m.configure(c); err != nil {
Expand Down Expand Up @@ -384,7 +388,7 @@ func (m *Motor) SetPower(ctx context.Context, powerPct float64, extra map[string
}
c, err := newCommand(m.c.address, cmd, m.Channel, byte(int(rawSpeed)))
if err != nil {
return err
return errors.Wrapf(err, "error in SetPower from motor (%s)", m.motorName)
}
err = m.c.sendCmd(c)
return err
Expand All @@ -401,7 +405,7 @@ func (m *Motor) GoFor(ctx context.Context, rpm, revolutions float64, extra map[s
powerPct, waitDur := goForMath(m.maxRPM, rpm, revolutions)
err := m.SetPower(ctx, powerPct, extra)
if err != nil {
return err
return errors.Wrapf(err, "error in GoFor from motor (%s)", m.motorName)
}

if revolutions == 0 {
Expand All @@ -418,7 +422,7 @@ func (m *Motor) GoFor(ctx context.Context, rpm, revolutions float64, extra map[s
// at a specific speed. Regardless of the directionality of the RPM this function will move the motor
// towards the specified target/position.
func (m *Motor) GoTo(ctx context.Context, rpm, position float64, extra map[string]interface{}) error {
return errors.New("not supported")
return motor.NewGoToUnsupportedError(fmt.Sprintf("Channel %d on Sabertooth %d", m.Channel, m.c.address))
}

// GoTillStop moves a motor until stopped by the controller (due to switch or function) or stopFunc.
Expand All @@ -428,7 +432,8 @@ func (m *Motor) GoTillStop(ctx context.Context, rpm float64, stopFunc func(ctx c

// ResetZeroPosition defines the current position to be zero (+/- offset).
func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map[string]interface{}) error {
return errors.New("not supported")
return motor.NewResetZeroPositionUnsupportedError(fmt.Sprintf("Channel %d on Sabertooth %d",
m.Channel, m.c.address))
}

// Position reports the position in revolutions.
Expand Down
18 changes: 13 additions & 5 deletions components/motor/dmc4000/dmc.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package dmc4000
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"math"
Expand All @@ -16,6 +15,7 @@ import (
"github.com/edaniels/golog"
"github.com/jacobsa/go-serial/serial"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"go.viam.com/utils"
"go.viam.com/utils/usb"

Expand Down Expand Up @@ -62,6 +62,7 @@ type Motor struct {
jogging bool
opMgr operation.SingleOperationManager
powerPct float64
motorName string
}

// Config adds DMC-specific config options.
Expand Down Expand Up @@ -90,7 +91,7 @@ func init() {

_motor := registry.Component{
Constructor: func(ctx context.Context, _ registry.Dependencies, config config.Component, logger golog.Logger) (interface{}, error) {
return NewMotor(ctx, config.ConvertedAttributes.(*Config), logger)
return NewMotor(ctx, config.ConvertedAttributes.(*Config), config.Name, logger)
},
}
registry.RegisterComponent(motor.Subtype, modelName, _motor)
Expand All @@ -114,7 +115,7 @@ func init() {
}

// NewMotor returns a DMC4000 driven motor.
func NewMotor(ctx context.Context, c *Config, logger golog.Logger) (motor.LocalMotor, error) {
func NewMotor(ctx context.Context, c *Config, name string, logger golog.Logger) (motor.LocalMotor, error) {
if c.SerialDevice == "" {
devs := usb.Search(usbFilter, func(vendorID, productID int) bool {
if vendorID == 0x403 && productID == 0x6001 {
Expand Down Expand Up @@ -167,6 +168,7 @@ func NewMotor(ctx context.Context, c *Config, logger golog.Logger) (motor.LocalM
MaxAcceleration: c.MaxAcceleration,
HomeRPM: c.HomeRPM,
powerPct: 0.0,
motorName: name,
}

if m.MaxRPM <= 0 {
Expand Down Expand Up @@ -542,7 +544,7 @@ func (m *Motor) GoTo(ctx context.Context, rpm, position float64, extra map[strin
defer m.c.mu.Unlock()

if err := m.doGoTo(rpm, position); err != nil {
return err
return motor.NewGoToUnsupportedError(m.motorName)
}

return m.opMgr.WaitForSuccess(
Expand Down Expand Up @@ -601,6 +603,9 @@ func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map
m.c.mu.Lock()
defer m.c.mu.Unlock()
_, err := m.c.sendCmd(fmt.Sprintf("DP%s=%d", m.Axis, int(offset*float64(m.TicksPerRotation))))
if err != nil {
return errors.Wrapf(err, "error in ResetZeroPosition from motor (%s)", m.motorName)
}
return err
}

Expand All @@ -622,7 +627,7 @@ func (m *Motor) Stop(ctx context.Context, extra map[string]interface{}) error {
m.jogging = false
_, err := m.c.sendCmd(fmt.Sprintf("ST%s", m.Axis))
if err != nil {
return err
return errors.Wrapf(err, "error in Stop function from motor (%s)", m.motorName)
}

return m.opMgr.WaitForSuccess(
Expand All @@ -646,6 +651,9 @@ func (m *Motor) IsMoving(ctx context.Context) (bool, error) {
// IsPowered returns whether or not the motor is currently moving.
func (m *Motor) IsPowered(ctx context.Context, extra map[string]interface{}) (bool, float64, error) {
on, err := m.IsMoving(ctx)
if err != nil {
return on, m.powerPct, errors.Wrapf(err, "error in IsPowered from motor (%s)", m.motorName)
}
return on, m.powerPct, err
}

Expand Down
5 changes: 5 additions & 0 deletions components/motor/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,8 @@ func NewFeatureUnsupportedError(feature Feature, motorName string) error {
func NewZeroRPMError() error {
return errors.New("Cannot move motor at 0 RPM")
}

// NewGoToUnsupportedError returns error when a motor is required to support GoTo feature.
func NewGoToUnsupportedError(motorName string) error {
return errors.Errorf("motor with name %s does not support GoTo", motorName)
}
6 changes: 3 additions & 3 deletions components/motor/fake/motor.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (m *Motor) GoTo(ctx context.Context, rpm, pos float64, extra map[string]int

// GoTillStop always returns an error.
func (m *Motor) GoTillStop(ctx context.Context, rpm float64, stopFunc func(ctx context.Context) bool) error {
return errors.New("not supported")
return motor.NewGoTillStopUnsupportedError(m.Name)
}

// ResetZeroPosition resets the zero position.
Expand All @@ -336,7 +336,7 @@ func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map

err := m.Encoder.Reset(ctx, int64(offset*float64(m.TicksPerRotation)), extra)
if err != nil {
return err
return errors.Wrapf(err, "error in ResetZeroPosition from motor (%s)", m.Name)
}

return nil
Expand All @@ -352,7 +352,7 @@ func (m *Motor) Stop(ctx context.Context, extra map[string]interface{}) error {
if m.Encoder != nil {
err := m.Encoder.SetSpeed(ctx, 0.0)
if err != nil {
return err
return errors.Wrapf(err, "error in Stop from motor (%s)", m.Name)
}
}
return nil
Expand Down
12 changes: 7 additions & 5 deletions components/motor/gpio/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// NewMotor constructs a new GPIO based motor on the given board using the
// given configuration.
func NewMotor(b board.Board, mc Config, logger golog.Logger) (motor.Motor, error) {
func NewMotor(b board.Board, mc Config, name string, logger golog.Logger) (motor.Motor, error) {
if mc.MaxPowerPct == 0 {
mc.MaxPowerPct = 1.0
}
Expand All @@ -40,6 +40,7 @@ func NewMotor(b board.Board, mc Config, logger golog.Logger) (motor.Motor, error
maxRPM: mc.MaxRPM,
dirFlip: mc.DirectionFlip,
logger: logger,
motorName: name,
}

if mc.Pins.A != "" {
Expand Down Expand Up @@ -103,6 +104,7 @@ type Motor struct {
powerPct float64
maxRPM float64
dirFlip bool
motorName string

opMgr operation.SingleOperationManager
logger golog.Logger
Expand Down Expand Up @@ -263,7 +265,7 @@ func (m *Motor) GoFor(ctx context.Context, rpm, revolutions float64, extra map[s
powerPct, waitDur := goForMath(m.maxRPM, rpm, revolutions)
err := m.SetPower(ctx, powerPct, extra)
if err != nil {
return err
return errors.Wrapf(err, "error in GoFor from motor (%s)", m.motorName)
}

if revolutions == 0 {
Expand Down Expand Up @@ -295,15 +297,15 @@ func (m *Motor) IsMoving(ctx context.Context) (bool, error) {

// GoTo is not supported.
func (m *Motor) GoTo(ctx context.Context, rpm, positionRevolutions float64, extra map[string]interface{}) error {
return errors.New("not supported")
return motor.NewGoToUnsupportedError(m.motorName)
susmitaSanyal marked this conversation as resolved.
Show resolved Hide resolved
}

// ResetZeroPosition is not supported.
func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map[string]interface{}) error {
return errors.New("not supported")
return motor.NewResetZeroPositionUnsupportedError(m.motorName)
}

// GoTillStop is not supported.
func (m *Motor) GoTillStop(ctx context.Context, rpm float64, stopFunc func(ctx context.Context) bool) error {
return motor.NewGoTillStopUnsupportedError("(name unavailable)")
return motor.NewGoTillStopUnsupportedError(m.motorName)
susmitaSanyal marked this conversation as resolved.
Show resolved Hide resolved
}
39 changes: 32 additions & 7 deletions components/motor/gpio/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"go.viam.com/rdk/components/board"
fakeboard "go.viam.com/rdk/components/board/fake"
"go.viam.com/rdk/components/motor"
"go.viam.com/rdk/config"
)

const maxRPM = 100
Expand All @@ -22,18 +23,22 @@ func TestMotorABPWM(t *testing.T) {
b := &fakeboard.Board{GPIOPins: map[string]*fakeboard.GPIOPin{}}
logger := golog.NewTestLogger(t)

mc := config.Component{
Name: "abc",
}

t.Run("motor (A/B/PWM) initialization errors", func(t *testing.T) {
m, err := NewMotor(b, Config{
Pins: PinConfig{A: "1", B: "2", PWM: "3"}, MaxPowerPct: 100, PWMFreq: 4000,
}, logger)
}, mc.Name, logger)
test.That(t, m, test.ShouldBeNil)
test.That(t, err, test.ShouldBeError, errors.New("max_power_pct must be between 0.06 and 1.0"))
})

m, err := NewMotor(b, Config{
Pins: PinConfig{A: "1", B: "2", PWM: "3"},
MaxRPM: maxRPM, PWMFreq: 4000,
}, logger)
}, mc.Name, logger)
test.That(t, err, test.ShouldBeNil)

t.Run("motor (A/B/PWM) Off testing", func(t *testing.T) {
Expand Down Expand Up @@ -133,24 +138,29 @@ func TestMotorDirPWM(t *testing.T) {
b := &fakeboard.Board{GPIOPins: map[string]*fakeboard.GPIOPin{}}
logger := golog.NewTestLogger(t)

mc := config.Component{
Name: "fake_motor",
}

t.Run("motor (DIR/PWM) initialization errors", func(t *testing.T) {
m, err := NewMotor(b, Config{Pins: PinConfig{Direction: "1", EnablePinLow: "2", PWM: "3"}, PWMFreq: 4000}, logger)
m, err := NewMotor(b, Config{Pins: PinConfig{Direction: "1", EnablePinLow: "2", PWM: "3"}, PWMFreq: 4000},
mc.Name, logger)

test.That(t, err, test.ShouldBeNil)
test.That(t, m.GoFor(ctx, 50, 10, nil), test.ShouldBeError, errors.New("not supported, define max_rpm attribute != 0"))

_, err = NewMotor(
b,
Config{Pins: PinConfig{Direction: "1", EnablePinLow: "2", PWM: "3"}, MaxPowerPct: 100, PWMFreq: 4000},
logger,
mc.Name, logger,
)
test.That(t, err, test.ShouldBeError, errors.New("max_power_pct must be between 0.06 and 1.0"))
})

m, err := NewMotor(b, Config{
Pins: PinConfig{Direction: "1", EnablePinLow: "2", PWM: "3"},
MaxRPM: maxRPM, PWMFreq: 4000,
}, logger)
}, mc.Name, logger)
test.That(t, err, test.ShouldBeNil)

t.Run("motor (DIR/PWM) Off testing", func(t *testing.T) {
Expand Down Expand Up @@ -211,11 +221,14 @@ func TestMotorAB(t *testing.T) {
ctx := context.Background()
b := &fakeboard.Board{GPIOPins: map[string]*fakeboard.GPIOPin{}}
logger := golog.NewTestLogger(t)
mc := config.Component{
Name: "fake_motor",
}

m, err := NewMotor(b, Config{
Pins: PinConfig{A: "1", B: "2", EnablePinLow: "3"},
MaxRPM: maxRPM, PWMFreq: 4000,
}, logger)
}, mc.Name, logger)
test.That(t, err, test.ShouldBeNil)

t.Run("motor (A/B) On testing", func(t *testing.T) {
Expand Down Expand Up @@ -281,16 +294,28 @@ func TestMotorABNoEncoder(t *testing.T) {
ctx := context.Background()
b := &fakeboard.Board{GPIOPins: map[string]*fakeboard.GPIOPin{}}
logger := golog.NewTestLogger(t)
mc := config.Component{
Name: "fake_motor",
}

m, err := NewMotor(b, Config{
Pins: PinConfig{A: "1", B: "2", EnablePinLow: "3"},
PWMFreq: 4000,
}, logger)
}, mc.Name, logger)
test.That(t, err, test.ShouldBeNil)

t.Run("motor no encoder GoFor testing", func(t *testing.T) {
test.That(t, m.GoFor(ctx, 50, 10, nil), test.ShouldBeError, errors.New("not supported, define max_rpm attribute != 0"))
})

t.Run("motor no encoder GoTo testing", func(t *testing.T) {
test.That(t, m.GoTo(ctx, 50, 10, nil), test.ShouldBeError, errors.New("motor with name fake_motor does not support GoTo"))
})

t.Run("motor no encoder ResetZeroPosition testing", func(t *testing.T) {
test.That(t, m.ResetZeroPosition(ctx, 5.0001, nil), test.ShouldBeError,
errors.New("motor with name fake_motor does not support ResetZeroPosition"))
})
}

func TestGoForMath(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion components/motor/gpio/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func init() {
return nil, err
}

m, err := NewMotor(actualBoard, *motorConfig, logger)
m, err := NewMotor(actualBoard, *motorConfig, config.Name, logger)
if err != nil {
return nil, err
}
Expand Down
Loading