From 61f4729513f3ae362deccfe7b81f4e07901ba276 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Mon, 3 Apr 2023 16:34:17 -0400 Subject: [PATCH 01/10] update encoder driver methods --- components/board/arduino/encoder.go | 4 +- components/encoder/{ => AMS}/ams_as5048.go | 36 +++++---- .../encoder/{ => AMS}/ams_as5048_test.go | 2 +- components/encoder/collectors.go | 2 +- components/encoder/encoder.go | 17 ++-- components/encoder/fake/encoder.go | 8 +- components/encoder/fake/encoder_test.go | 10 +-- .../{ => incremental}/incremental_encoder.go | 31 +++---- .../encoder/incremental/incremental_test.go | 1 + .../encoder/{ => single}/single_encoder.go | 37 ++++----- .../{ => single}/single_encoder_test.go | 80 ++++++++++--------- components/motor/fake/motor.go | 4 +- components/motor/gpio/motor_encoder.go | 13 +-- components/motor/gpio/motor_encoder_test.go | 39 ++++----- config/config_test.go | 6 +- 15 files changed, 151 insertions(+), 139 deletions(-) rename components/encoder/{ => AMS}/ams_as5048.go (92%) rename components/encoder/{ => AMS}/ams_as5048_test.go (97%) rename components/encoder/{ => incremental}/incremental_encoder.go (88%) create mode 100644 components/encoder/incremental/incremental_test.go rename components/encoder/{ => single}/single_encoder.go (85%) rename components/encoder/{ => single}/single_encoder_test.go (70%) diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index b52be7b6fac..5c47c9bf82e 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -103,7 +103,7 @@ func (cfg *EncoderConfig) Validate(path string) ([]string, error) { } // TicksCount returns number of ticks since last zeroing. -func (e *Encoder) TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res, err := e.board.runCommand("motor-position " + e.name) if err != nil { return 0, err @@ -119,7 +119,7 @@ func (e *Encoder) TicksCount(ctx context.Context, extra map[string]interface{}) // Reset sets the current position of the motor (adjusted by a given offset) // to be its new zero position. -func (e *Encoder) Reset(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { if err := encoder.ValidateIntegerOffset(offset); err != nil { return err } diff --git a/components/encoder/ams_as5048.go b/components/encoder/AMS/ams_as5048.go similarity index 92% rename from components/encoder/ams_as5048.go rename to components/encoder/AMS/ams_as5048.go index d5c2a248eed..27f476d0155 100644 --- a/components/encoder/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -1,4 +1,4 @@ -package encoder +package AMS import ( "context" @@ -12,6 +12,7 @@ import ( "go.viam.com/utils" "go.viam.com/rdk/components/board" + "go.viam.com/rdk/components/encoder" "go.viam.com/rdk/components/generic" "go.viam.com/rdk/config" "go.viam.com/rdk/registry" @@ -36,7 +37,7 @@ var waitTimeNano = (1.0 / 50.0) * 1000000000.0 func init() { registry.RegisterComponent( - Subtype, + encoder.Subtype, modelName, registry.Component{ Constructor: func( @@ -50,19 +51,19 @@ func init() { }, ) config.RegisterComponentAttributeMapConverter( - Subtype, + encoder.Subtype, modelName, func(attributes config.AttributeMap) (interface{}, error) { - var conf AS5048Config + var conf AttrConfig return config.TransformAttributeMapToStruct(&conf, attributes) }, - &AS5048Config{}, + &AttrConfig{}, ) } // AS5048Config contains the connection information for // configuring an AS5048 encoder. -type AS5048Config struct { +type AttrConfig struct { BoardName string `json:"board"` // We include connection type here in anticipation for // future SPI support @@ -72,7 +73,7 @@ type AS5048Config struct { // Validate checks the attributes of an initialized config // for proper values. -func (conf *AS5048Config) Validate(path string) ([]string, error) { +func (conf *AttrConfig) Validate(path string) ([]string, error) { var deps []string connType := conf.ConnectionType @@ -140,8 +141,8 @@ type AS5048 struct { func newAS5048Encoder( ctx context.Context, deps registry.Dependencies, cfg config.Component, logger *zap.SugaredLogger, -) (*AS5048, error) { - attr, ok := cfg.ConvertedAttributes.(*AS5048Config) +) (encoder.Encoder, error) { + attr, ok := cfg.ConvertedAttributes.(*AttrConfig) if !ok { return nil, rdkutils.NewUnexpectedTypeError(attr, cfg.ConvertedAttributes) } @@ -177,7 +178,7 @@ func newAS5048Encoder( } func (enc *AS5048) startPositionLoop(ctx context.Context) error { - if err := enc.Reset(ctx, 0.0, map[string]interface{}{}); err != nil { + if err := enc.ResetPosition(ctx, 0.0, map[string]interface{}{}); err != nil { return err } enc.activeBackgroundWorkers.Add(1) @@ -243,13 +244,13 @@ func (enc *AS5048) updatePosition(ctx context.Context) error { return nil } -// TicksCount returns the total number of rotations detected +// GetPosition returns the total number of rotations detected // by the encoder (rather than a number of pulse state transitions) // because this encoder is absolute and not incremental. As a result // a user MUST set ticks_per_rotation on the config of the corresponding // motor to 1. Any other value will result in completely incorrect // position measurements by the motor. -func (enc *AS5048) TicksCount( +func (enc *AS5048) GetPosition( ctx context.Context, extra map[string]interface{}, ) (float64, error) { enc.mu.RLock() @@ -258,11 +259,11 @@ func (enc *AS5048) TicksCount( return ticks, nil } -// Reset sets the current position measured by the encoder to be considered -// its new zero position. If the offset provided is not 0.0, it also -// sets the positionOffset attribute and adjusts all future recorded +// ResetPosition sets the current position measured by the encoder to be +// considered its new zero position. If the offset provided is not 0.0, it +// also sets the positionOffset attribute and adjusts all future recorded // positions by that offset (until the function is called again). -func (enc *AS5048) Reset( +func (enc *AS5048) ResetPosition( ctx context.Context, offset float64, extra map[string]interface{}, ) error { enc.mu.Lock() @@ -303,9 +304,10 @@ func (enc *AS5048) Reset( // Close stops the position loop of the encoder when the component // is closed. -func (enc *AS5048) Close() { +func (enc *AS5048) Close() error { enc.cancel() enc.activeBackgroundWorkers.Wait() + return nil } // readByteDataFromBus opens a handle for the bus adhoc to perform a single read diff --git a/components/encoder/ams_as5048_test.go b/components/encoder/AMS/ams_as5048_test.go similarity index 97% rename from components/encoder/ams_as5048_test.go rename to components/encoder/AMS/ams_as5048_test.go index b559b9d856d..c84658ede81 100644 --- a/components/encoder/ams_as5048_test.go +++ b/components/encoder/AMS/ams_as5048_test.go @@ -1,4 +1,4 @@ -package encoder +package AMS import ( "math" diff --git a/components/encoder/collectors.go b/components/encoder/collectors.go index 236b38020ec..f2537d61f08 100644 --- a/components/encoder/collectors.go +++ b/components/encoder/collectors.go @@ -33,7 +33,7 @@ func newTicksCountCollector(resource interface{}, params data.CollectorParams) ( } cFunc := data.CaptureFunc(func(ctx context.Context, _ map[string]*anypb.Any) (interface{}, error) { - v, err := encoder.TicksCount(ctx, nil) + v, err := encoder.GetPosition(ctx, nil) if err != nil { return nil, data.FailedToReadErr(params.ComponentName, ticksCount.String(), err) } diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index 93572286c0d..7fd3cd7e885 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -39,12 +39,12 @@ var Subtype = resource.NewSubtype( // A Encoder turns a position into a signal. type Encoder interface { - // TicksCount returns number of ticks since last zeroing - TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) + // GetPosition returns number of ticks since last zeroing + GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) - // Reset sets the current position of the motor (adjusted by a given offset) + // ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position. - Reset(ctx context.Context, offset float64, extra map[string]interface{}) error + ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error generic.Generic } @@ -58,6 +58,7 @@ var ( _ = Encoder(&reconfigurableEncoder{}) _ = resource.Reconfigurable(&reconfigurableEncoder{}) _ = resource.Reconfigurable(&reconfigurableEncoder{}) + _ = viamutils.ContextCloser(&reconfigurableEncoder{}) ) // FromDependencies is a helper for getting the named encoder from a collection of @@ -103,16 +104,16 @@ func (r *reconfigurableEncoder) DoCommand(ctx context.Context, cmd map[string]in return r.actual.DoCommand(ctx, cmd) } -func (r *reconfigurableEncoder) TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (r *reconfigurableEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { r.mu.RLock() defer r.mu.RUnlock() - return r.actual.TicksCount(ctx, extra) + return r.actual.GetPosition(ctx, extra) } -func (r *reconfigurableEncoder) Reset(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (r *reconfigurableEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { r.mu.RLock() defer r.mu.RUnlock() - return r.actual.Reset(ctx, offset, extra) + return r.actual.ResetPosition(ctx, offset, extra) } func (r *reconfigurableEncoder) Close(ctx context.Context) error { diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 8e48034436f..5d1ebc2100b 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -65,8 +65,8 @@ type Encoder struct { generic.Unimplemented } -// TicksCount returns the current position in terms of ticks. -func (e *Encoder) TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) { +// GetPosition returns the current position in terms of ticks. +func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { e.mu.Lock() defer e.mu.Unlock() return float64(e.position), nil @@ -100,9 +100,9 @@ func (e *Encoder) Start(cancelCtx context.Context) { }, e.activeBackgroundWorkers.Done) } -// Reset sets the current position of the motor (adjusted by a given offset) +// ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position. -func (e *Encoder) Reset(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { if err := encoder.ValidateIntegerOffset(offset); err != nil { return err } diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index 9ec72f4f68b..220ee650b72 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -15,24 +15,24 @@ func TestEncoder(t *testing.T) { // Get and set position t.Run("get and set position", func(t *testing.T) { - pos, err := e.TicksCount(ctx, nil) + pos, err := e.GetPosition(ctx, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) err = e.SetPosition(ctx, 1) test.That(t, err, test.ShouldBeNil) - pos, err = e.TicksCount(ctx, nil) + pos, err = e.GetPosition(ctx, nil) test.That(t, pos, test.ShouldEqual, 1) test.That(t, err, test.ShouldBeNil) }) // Reset t.Run("reset to zero", func(t *testing.T) { - err := e.Reset(ctx, 0, nil) + err := e.ResetPosition(ctx, 0, nil) test.That(t, err, test.ShouldBeNil) - pos, err := e.TicksCount(ctx, nil) + pos, err := e.GetPosition(ctx, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) }) @@ -61,7 +61,7 @@ func TestEncoder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - pos, err := e.TicksCount(ctx, nil) + pos, err := e.GetPosition(ctx, nil) test.That(tb, pos, test.ShouldBeGreaterThan, 0) test.That(tb, err, test.ShouldBeNil) }) diff --git a/components/encoder/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go similarity index 88% rename from components/encoder/incremental_encoder.go rename to components/encoder/incremental/incremental_encoder.go index b6e84c745ec..845430d5f76 100644 --- a/components/encoder/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -1,4 +1,4 @@ -package encoder +package incremental import ( "context" @@ -10,6 +10,7 @@ import ( "go.viam.com/utils" "go.viam.com/rdk/components/board" + "go.viam.com/rdk/components/encoder" "go.viam.com/rdk/components/generic" "go.viam.com/rdk/config" "go.viam.com/rdk/registry" @@ -20,7 +21,7 @@ var incrModel = resource.NewDefaultModel("incremental") func init() { registry.RegisterComponent( - Subtype, + encoder.Subtype, incrModel, registry.Component{Constructor: func( ctx context.Context, @@ -32,13 +33,13 @@ func init() { }}) config.RegisterComponentAttributeMapConverter( - Subtype, + encoder.Subtype, incrModel, func(attributes config.AttributeMap) (interface{}, error) { - var conf IncrementalConfig + var conf AttrConfig return config.TransformAttributeMapToStruct(&conf, attributes) }, - &IncrementalConfig{}) + &AttrConfig{}) } // IncrementalEncoder keeps track of a motor position using a rotary incremental encoder. @@ -62,14 +63,14 @@ type IncrementalPins struct { B string `json:"b"` } -// IncrementalConfig describes the configuration of a quadrature encoder. -type IncrementalConfig struct { +// AttrConfig describes the configuration of a quadrature encoder. +type AttrConfig struct { Pins IncrementalPins `json:"pins"` BoardName string `json:"board"` } // Validate ensures all parts of the config are valid. -func (config *IncrementalConfig) Validate(path string) ([]string, error) { +func (config *AttrConfig) Validate(path string) ([]string, error) { var deps []string if config.Pins.A == "" { @@ -93,10 +94,10 @@ func NewIncrementalEncoder( deps registry.Dependencies, cfg config.Component, logger golog.Logger, -) (*IncrementalEncoder, error) { +) (encoder.Encoder, error) { cancelCtx, cancelFunc := context.WithCancel(ctx) e := &IncrementalEncoder{logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, pRaw: 0, pState: 0} - if cfg, ok := cfg.ConvertedAttributes.(*IncrementalConfig); ok { + if cfg, ok := cfg.ConvertedAttributes.(*AttrConfig); ok { board, err := board.FromDependencies(deps, cfg.BoardName) if err != nil { return nil, err @@ -230,16 +231,16 @@ func (e *IncrementalEncoder) Start(ctx context.Context) { }, e.activeBackgroundWorkers.Done) } -// TicksCount returns number of ticks since last zeroing. -func (e *IncrementalEncoder) TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) { +// GetPosition returns number of ticks since last zeroing. +func (e *IncrementalEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res := atomic.LoadInt64(&e.position) return float64(res), nil } -// Reset sets the current position of the motor (adjusted by a given offset) +// ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position.. -func (e *IncrementalEncoder) Reset(ctx context.Context, offset float64, extra map[string]interface{}) error { - if err := ValidateIntegerOffset(offset); err != nil { +func (e *IncrementalEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { + if err := encoder.ValidateIntegerOffset(offset); err != nil { return err } offsetInt := int64(offset) diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go new file mode 100644 index 00000000000..018ebfb203e --- /dev/null +++ b/components/encoder/incremental/incremental_test.go @@ -0,0 +1 @@ +package incremental diff --git a/components/encoder/single_encoder.go b/components/encoder/single/single_encoder.go similarity index 85% rename from components/encoder/single_encoder.go rename to components/encoder/single/single_encoder.go index cfa479bfc9f..58c44804c32 100644 --- a/components/encoder/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -1,4 +1,4 @@ -package encoder +package single /* This driver implements a single-wire odometer, such as LM393, as an encoder. @@ -32,6 +32,7 @@ import ( "go.viam.com/utils" "go.viam.com/rdk/components/board" + "go.viam.com/rdk/components/encoder" "go.viam.com/rdk/components/generic" "go.viam.com/rdk/config" "go.viam.com/rdk/registry" @@ -43,7 +44,7 @@ var singlemodelname = resource.NewDefaultModel("single") func init() { registry.RegisterComponent( - Subtype, + encoder.Subtype, singlemodelname, registry.Component{Constructor: func( ctx context.Context, @@ -55,13 +56,13 @@ func init() { }}) config.RegisterComponentAttributeMapConverter( - Subtype, + encoder.Subtype, singlemodelname, func(attributes config.AttributeMap) (interface{}, error) { - var conf SingleWireConfig + var conf AttrConfig return config.TransformAttributeMapToStruct(&conf, attributes) }, - &SingleWireConfig{}) + &AttrConfig{}) } // DirectionAware lets you ask what direction something is moving. Only used for SingleEncoder for now, unclear future. @@ -70,7 +71,7 @@ type DirectionAware interface { DirectionMoving() int64 } -// SingleEncoder keeps track of a motor position using a rotary encoder. +// SingleEncoder keeps track of a motor position using a rotary encoder.s type SingleEncoder struct { generic.Unimplemented name string @@ -85,18 +86,18 @@ type SingleEncoder struct { } // SingleWirePin describes the configuration of Pins for a Single encoder. -type SingleWirePin struct { +type SinglePin struct { I string `json:"i"` } -// SingleWireConfig describes the configuration of a single encoder. -type SingleWireConfig struct { - Pins SingleWirePin `json:"pins"` - BoardName string `json:"board"` +// AttrConfig describes the configuration of a single encoder. +type AttrConfig struct { + Pins SinglePin `json:"pins"` + BoardName string `json:"board"` } // Validate ensures all parts of the config are valid. -func (cfg *SingleWireConfig) Validate(path string) ([]string, error) { +func (cfg *AttrConfig) Validate(path string) ([]string, error) { var deps []string if cfg.Pins.I == "" { @@ -122,8 +123,8 @@ func NewSingleEncoder( deps registry.Dependencies, rawConfig config.Component, logger golog.Logger, -) (*SingleEncoder, error) { - cfg, ok := rawConfig.ConvertedAttributes.(*SingleWireConfig) +) (encoder.Encoder, error) { + cfg, ok := rawConfig.ConvertedAttributes.(*AttrConfig) if !ok { return nil, rutils.NewUnexpectedTypeError(cfg, rawConfig.ConvertedAttributes) } @@ -181,14 +182,14 @@ func (e *SingleEncoder) Start(ctx context.Context) { }, e.activeBackgroundWorkers.Done) } -// TicksCount returns the current position. -func (e *SingleEncoder) TicksCount(ctx context.Context, extra map[string]interface{}) (float64, error) { +// GetPosition returns the current position. +func (e *SingleEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res := atomic.LoadInt64(&e.position) return float64(res), nil } -// Reset sets the current position of the motor (adjusted by a given offset). -func (e *SingleEncoder) Reset(ctx context.Context, offset float64, extra map[string]interface{}) error { +// ResetPosition sets the current position of the motor (adjusted by a given offset). +func (e *SingleEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { offsetInt := int64(math.Round(offset)) atomic.StoreInt64(&e.position, offsetInt) return nil diff --git a/components/encoder/single_encoder_test.go b/components/encoder/single/single_encoder_test.go similarity index 70% rename from components/encoder/single_encoder_test.go rename to components/encoder/single/single_encoder_test.go index b159281078e..df0ae3fb011 100644 --- a/components/encoder/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -1,4 +1,4 @@ -package encoder +package single import ( "context" @@ -24,20 +24,19 @@ func TestConfig(t *testing.T) { deps[board.Named("main")] = b t.Run("valid config", func(t *testing.T) { - ic := SingleWireConfig{ + ic := AttrConfig{ BoardName: "main", - Pins: SingleWirePin{I: "10"}, + Pins: SinglePin{I: "10"}, } rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} - enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + _, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - test.That(t, enc.name, test.ShouldEqual, "enc1") }) t.Run("invalid config", func(t *testing.T) { - ic := SingleWireConfig{ + ic := AttrConfig{ BoardName: "pi", // Pins intentionally missing } @@ -57,9 +56,9 @@ func TestEnconder(t *testing.T) { deps := make(registry.Dependencies) deps[board.Named("main")] = b - ic := SingleWireConfig{ + ic := AttrConfig{ BoardName: "main", - Pins: SingleWirePin{I: "10"}, + Pins: SinglePin{I: "10"}, } rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} @@ -67,17 +66,18 @@ func TestEnconder(t *testing.T) { t.Run("run forward", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - defer enc.Close() + enc2 := enc.(*SingleEncoder) + defer enc2.Close() m := &FakeDir{1} // forward - enc.AttachDirectionalAwareness(m) + enc2.AttachDirectionalAwareness(m) - err = enc.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) test.That(t, err, test.ShouldBeNil) testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -86,17 +86,18 @@ func TestEnconder(t *testing.T) { t.Run("run backward", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - defer enc.Close() + enc2 := enc.(*SingleEncoder) + defer enc2.Close() m := &FakeDir{-1} // backward - enc.AttachDirectionalAwareness(m) + enc2.AttachDirectionalAwareness(m) - err = enc.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) test.That(t, err, test.ShouldBeNil) testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, -1) }) @@ -107,9 +108,10 @@ func TestEnconder(t *testing.T) { t.Run("run no direction", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - defer enc.Close() + enc2 := enc.(*SingleEncoder) + defer enc2.Close() - err = enc.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) test.That(t, err, test.ShouldBeNil) // Give the tick time to propagate to encoder @@ -117,7 +119,7 @@ func TestEnconder(t *testing.T) { // by the encoder worker time.Sleep(50 * time.Millisecond) - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -125,40 +127,41 @@ func TestEnconder(t *testing.T) { t.Run("reset position", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - defer enc.Close() + enc2 := enc.(*SingleEncoder) + defer enc2.Close() // set to a positive int - err = enc.Reset(context.Background(), 5.0, nil) + err = enc.ResetPosition(context.Background(), 5.0, nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 5) // set to a negative int - err = enc.Reset(context.Background(), -5.0, nil) + err = enc.ResetPosition(context.Background(), -5.0, nil) test.That(t, err, test.ShouldBeNil) - ticks, err = enc.TicksCount(context.Background(), nil) + ticks, err = enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, -5) // set to a positive float - err = enc.Reset(context.Background(), 1.2, nil) + err = enc.ResetPosition(context.Background(), 1.2, nil) test.That(t, err, test.ShouldBeNil) - ticks, err = enc.TicksCount(context.Background(), nil) + ticks, err = enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 1) // set to a negative float - err = enc.Reset(context.Background(), -1000.999, nil) + err = enc.ResetPosition(context.Background(), -1000.999, nil) test.That(t, err, test.ShouldBeNil) - ticks, err = enc.TicksCount(context.Background(), nil) + ticks, err = enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, -1001) // set to 09 - err = enc.Reset(context.Background(), 0, nil) + err = enc.ResetPosition(context.Background(), 0, nil) test.That(t, err, test.ShouldBeNil) - ticks, err = enc.TicksCount(context.Background(), nil) + ticks, err = enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -166,36 +169,37 @@ func TestEnconder(t *testing.T) { t.Run("reset position and tick", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - defer enc.Close() + enc2 := enc.(*SingleEncoder) + defer enc2.Close() m := &FakeDir{1} // forward - enc.AttachDirectionalAwareness(m) + enc2.AttachDirectionalAwareness(m) // move forward - err = enc.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) test.That(t, err, test.ShouldBeNil) testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) // reset tick - err = enc.Reset(context.Background(), 0, nil) + err = enc.ResetPosition(context.Background(), 0, nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) // now tick up again - err = enc.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) test.That(t, err, test.ShouldBeNil) testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.TicksCount(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) diff --git a/components/motor/fake/motor.go b/components/motor/fake/motor.go index dd01dd3b408..fd07ffaf6d5 100644 --- a/components/motor/fake/motor.go +++ b/components/motor/fake/motor.go @@ -151,7 +151,7 @@ func (m *Motor) Position(ctx context.Context, extra map[string]interface{}) (flo return 0, errors.New("encoder is not defined") } - ticks, err := m.Encoder.TicksCount(ctx, extra) + ticks, err := m.Encoder.GetPosition(ctx, extra) if err != nil { return 0, err } @@ -338,7 +338,7 @@ func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map return errors.New("need nonzero TicksPerRotation for motor") } - err := m.Encoder.Reset(ctx, offset*float64(m.TicksPerRotation), extra) + err := m.Encoder.ResetPosition(ctx, offset*float64(m.TicksPerRotation), extra) if err != nil { return errors.Wrapf(err, "error in ResetZeroPosition from motor (%s)", m.Name) } diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index 45d444a04e5..c5289b0302b 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -14,6 +14,7 @@ import ( "go.viam.com/utils" "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/components/encoder/single" "go.viam.com/rdk/components/generic" "go.viam.com/rdk/components/motor" "go.viam.com/rdk/config" @@ -72,7 +73,7 @@ func WrapMotorWithEncoder( return nil, err } - single, isSingle := rutils.UnwrapProxy(e).(*encoder.SingleEncoder) + single, isSingle := rutils.UnwrapProxy(e).(*single.SingleEncoder) if isSingle { single.AttachDirectionalAwareness(mm) logger.Info("direction attached to single encoder from encoded motor") @@ -198,7 +199,7 @@ type EncodedMotorState struct { // Position returns the position of the motor. func (m *EncodedMotor) Position(ctx context.Context, extra map[string]interface{}) (float64, error) { - ticks, err := m.encoder.TicksCount(ctx, extra) + ticks, err := m.encoder.GetPosition(ctx, extra) if err != nil { return 0, err } @@ -303,7 +304,7 @@ func (m *EncodedMotor) rpmMonitor() { m.startedRPMMonitor = true m.startedRPMMonitorMu.Unlock() - lastPosFl, err := m.encoder.TicksCount(m.cancelCtx, nil) + lastPosFl, err := m.encoder.GetPosition(m.cancelCtx, nil) if err != nil { panic(err) } @@ -327,7 +328,7 @@ func (m *EncodedMotor) rpmMonitor() { case <-timer.C: } - pos, err := m.encoder.TicksCount(m.cancelCtx, nil) + pos, err := m.encoder.GetPosition(m.cancelCtx, nil) if err != nil { m.logger.Info("error getting encoder position, sleeping then continuing: %w", err) if !utils.SelectContextOrWait(m.cancelCtx, 100*time.Millisecond) { @@ -563,7 +564,7 @@ func (m *EncodedMotor) goForInternal(ctx context.Context, rpm, revolutions float numTicks := int64(revolutions * float64(m.cfg.TicksPerRotation)) - pos, err := m.encoder.TicksCount(ctx, nil) + pos, err := m.encoder.GetPosition(ctx, nil) if err != nil { return err } @@ -712,5 +713,5 @@ func (m *EncodedMotor) GoTillStop(ctx context.Context, rpm float64, stopFunc fun // ResetZeroPosition sets the current position of the motor specified by the request // (adjusted by a given offset) to be its new zero position. func (m *EncodedMotor) ResetZeroPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - return m.encoder.Reset(ctx, offset*float64(m.cfg.TicksPerRotation), extra) + return m.encoder.ResetPosition(ctx, offset*float64(m.cfg.TicksPerRotation), extra) } diff --git a/components/motor/gpio/motor_encoder_test.go b/components/motor/gpio/motor_encoder_test.go index 5a1adf09d78..704db40ef71 100644 --- a/components/motor/gpio/motor_encoder_test.go +++ b/components/motor/gpio/motor_encoder_test.go @@ -13,7 +13,8 @@ import ( "go.viam.com/rdk/components/board" fakeboard "go.viam.com/rdk/components/board/fake" - "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/components/encoder/incremental" + "go.viam.com/rdk/components/encoder/single" "go.viam.com/rdk/components/motor" fakemotor "go.viam.com/rdk/components/motor/fake" "go.viam.com/rdk/config" @@ -44,7 +45,7 @@ func TestMotorEncoder1(t *testing.T) { } interrupt := &board.BasicDigitalInterrupt{} - e := &encoder.SingleEncoder{I: interrupt, CancelCtx: context.Background()} + e := &single.SingleEncoder{I: interrupt, CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: fakeMotor}) e.Start(context.Background()) dirFMotor, err := NewEncodedMotor(config.Component{}, cfg, fakeMotor, e, logger) @@ -277,7 +278,7 @@ func TestMotorEncoderIncremental(t *testing.T) { defer undo() type testHarness struct { - Encoder *encoder.IncrementalEncoder + Encoder *incremental.IncrementalEncoder EncoderA board.DigitalInterrupt EncoderB board.DigitalInterrupt RealMotor *fakemotor.Motor @@ -294,7 +295,7 @@ func TestMotorEncoderIncremental(t *testing.T) { } encoderA := &board.BasicDigitalInterrupt{} encoderB := &board.BasicDigitalInterrupt{} - encoder := &encoder.IncrementalEncoder{A: encoderA, B: encoderB, CancelCtx: context.Background()} + encoder := &incremental.IncrementalEncoder{A: encoderA, B: encoderB, CancelCtx: context.Background()} encoder.Start(context.Background()) motorIfc, err := NewEncodedMotor(config.Component{}, cfg, fakeMotor, encoder, logger) @@ -355,7 +356,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -366,7 +367,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -377,7 +378,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -388,7 +389,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -415,7 +416,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -426,7 +427,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -437,7 +438,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -448,7 +449,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -459,7 +460,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -470,7 +471,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -481,7 +482,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -492,7 +493,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 0) - posFl, err := encoder.TicksCount(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -596,7 +597,7 @@ func TestWrapMotorWithEncoder(t *testing.T) { test.That(t, err, test.ShouldBeNil) fakeMotor := &fakemotor.Motor{} b.Digitals["a"] = &board.BasicDigitalInterrupt{} - e := &encoder.SingleEncoder{I: b.Digitals["a"], CancelCtx: context.Background()} + e := &single.SingleEncoder{I: b.Digitals["a"], CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: fakeMotor}) e.Start(context.Background()) @@ -623,7 +624,7 @@ func TestWrapMotorWithEncoder(t *testing.T) { fakeMotor := &fakemotor.Motor{} b.Digitals["a"] = &board.BasicDigitalInterrupt{} b.Digitals["b"] = &board.BasicDigitalInterrupt{} - e := &encoder.IncrementalEncoder{A: b.Digitals["a"], B: b.Digitals["b"], CancelCtx: context.Background()} + e := &incremental.IncrementalEncoder{A: b.Digitals["a"], B: b.Digitals["b"], CancelCtx: context.Background()} e.Start(context.Background()) m, err := WrapMotorWithEncoder( @@ -655,7 +656,7 @@ func TestDirFlipMotor(t *testing.T) { } interrupt := &board.BasicDigitalInterrupt{} - e := &encoder.SingleEncoder{I: interrupt, CancelCtx: context.Background()} + e := &single.SingleEncoder{I: interrupt, CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: dirflipFakeMotor}) e.Start(context.Background()) dirFMotor, err := NewEncodedMotor(config.Component{}, cfg, dirflipFakeMotor, e, logger) diff --git a/config/config_test.go b/config/config_test.go index 32c96fc29bf..76e76f20132 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -24,7 +24,7 @@ import ( "go.viam.com/rdk/components/board" fakeboard "go.viam.com/rdk/components/board/fake" - "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/components/encoder/incremental" fakemotor "go.viam.com/rdk/components/motor/fake" "go.viam.com/rdk/config" "go.viam.com/rdk/referenceframe" @@ -106,8 +106,8 @@ func TestConfig3(t *testing.T) { MaxPowerPct: 0.5, TicksPerRotation: 10000, }) - test.That(t, cfg.Components[3].ConvertedAttributes, test.ShouldResemble, &encoder.IncrementalConfig{ - Pins: encoder.IncrementalPins{ + test.That(t, cfg.Components[3].ConvertedAttributes, test.ShouldResemble, &incremental.AttrConfig{ + Pins: incremental.IncrementalPins{ A: "encoder-steering-b", B: "encoder-steering-a", }, From d0686343965087f328d8016bdd7a1a2008bcc268 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Tue, 4 Apr 2023 14:37:58 -0400 Subject: [PATCH 02/10] register encoders --- components/board/arduino/encoder.go | 4 +- components/encoder/AMS/ams_as5048.go | 5 +- components/encoder/AMS/ams_as5048_test.go | 2 +- .../incremental/incremental_encoder.go | 35 ++-- .../encoder/incremental/incremental_test.go | 170 ++++++++++++++++++ components/encoder/register/register.go | 9 + components/encoder/single/single_encoder.go | 58 +++--- .../encoder/single/single_encoder_test.go | 14 +- components/motor/gpio/motor_encoder.go | 2 +- components/motor/gpio/motor_encoder_test.go | 12 +- components/register/all.go | 1 + config/config_test.go | 2 +- 12 files changed, 248 insertions(+), 66 deletions(-) create mode 100644 components/encoder/register/register.go diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index 5c47c9bf82e..d1dd8544047 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -102,7 +102,7 @@ func (cfg *EncoderConfig) Validate(path string) ([]string, error) { return deps, nil } -// TicksCount returns number of ticks since last zeroing. +// GetPosition returns number of ticks since last zeroing. func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res, err := e.board.runCommand("motor-position " + e.name) if err != nil { @@ -117,7 +117,7 @@ func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) return float64(ticks), nil } -// Reset sets the current position of the motor (adjusted by a given offset) +// ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position. func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { if err := encoder.ValidateIntegerOffset(offset); err != nil { diff --git a/components/encoder/AMS/ams_as5048.go b/components/encoder/AMS/ams_as5048.go index 27f476d0155..85b5718204c 100644 --- a/components/encoder/AMS/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -1,4 +1,5 @@ -package AMS +// Package ams implements the AMS_AS5048 encoder +package ams import ( "context" @@ -61,7 +62,7 @@ func init() { ) } -// AS5048Config contains the connection information for +// AttrConfig contains the connection information for // configuring an AS5048 encoder. type AttrConfig struct { BoardName string `json:"board"` diff --git a/components/encoder/AMS/ams_as5048_test.go b/components/encoder/AMS/ams_as5048_test.go index c84658ede81..9791d688efa 100644 --- a/components/encoder/AMS/ams_as5048_test.go +++ b/components/encoder/AMS/ams_as5048_test.go @@ -1,4 +1,4 @@ -package AMS +package ams import ( "math" diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index 845430d5f76..0820e4237a3 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -1,3 +1,4 @@ +// Package incremental implements an incremental encoder package incremental import ( @@ -42,8 +43,8 @@ func init() { &AttrConfig{}) } -// IncrementalEncoder keeps track of a motor position using a rotary incremental encoder. -type IncrementalEncoder struct { +// Encoder keeps track of a motor position using a rotary incremental encoder. +type Encoder struct { A, B board.DigitalInterrupt position int64 pRaw int64 @@ -57,16 +58,16 @@ type IncrementalEncoder struct { generic.Unimplemented } -// IncrementalPins describes the configuration of Pins for a quadrature encoder. -type IncrementalPins struct { +// Pins describes the configuration of Pins for a quadrature encoder. +type Pins struct { A string `json:"a"` B string `json:"b"` } // AttrConfig describes the configuration of a quadrature encoder. type AttrConfig struct { - Pins IncrementalPins `json:"pins"` - BoardName string `json:"board"` + Pins Pins `json:"pins"` + BoardName string `json:"board"` } // Validate ensures all parts of the config are valid. @@ -88,7 +89,7 @@ func (config *AttrConfig) Validate(path string) ([]string, error) { return deps, nil } -// NewIncrementalEncoder creates a new IncrementalEncoder. +// NewIncrementalEncoder creates a new Encoder. func NewIncrementalEncoder( ctx context.Context, deps registry.Dependencies, @@ -96,7 +97,7 @@ func NewIncrementalEncoder( logger golog.Logger, ) (encoder.Encoder, error) { cancelCtx, cancelFunc := context.WithCancel(ctx) - e := &IncrementalEncoder{logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, pRaw: 0, pState: 0} + e := &Encoder{logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, pRaw: 0, pState: 0} if cfg, ok := cfg.ConvertedAttributes.(*AttrConfig); ok { board, err := board.FromDependencies(deps, cfg.BoardName) if err != nil { @@ -120,8 +121,8 @@ func NewIncrementalEncoder( return nil, errors.New("encoder config for incremental Encoder is not valid") } -// Start starts the IncrementalEncoder background thread. -func (e *IncrementalEncoder) Start(ctx context.Context) { +// Start starts the Encoder background thread. +func (e *Encoder) Start(ctx context.Context) { /** a rotary encoder looks like @@ -232,14 +233,14 @@ func (e *IncrementalEncoder) Start(ctx context.Context) { } // GetPosition returns number of ticks since last zeroing. -func (e *IncrementalEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res := atomic.LoadInt64(&e.position) return float64(res), nil } // ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position.. -func (e *IncrementalEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { if err := encoder.ValidateIntegerOffset(offset); err != nil { return err } @@ -250,20 +251,20 @@ func (e *IncrementalEncoder) ResetPosition(ctx context.Context, offset float64, } // RawPosition returns the raw position of the encoder. -func (e *IncrementalEncoder) RawPosition() int64 { +func (e *Encoder) RawPosition() int64 { return atomic.LoadInt64(&e.pRaw) } -func (e *IncrementalEncoder) inc() { +func (e *Encoder) inc() { atomic.AddInt64(&e.pRaw, 1) } -func (e *IncrementalEncoder) dec() { +func (e *Encoder) dec() { atomic.AddInt64(&e.pRaw, -1) } -// Close shuts down the IncrementalEncoder. -func (e *IncrementalEncoder) Close() error { +// Close shuts down the Encoder. +func (e *Encoder) Close() error { e.logger.Debug("Closing incremental Encoder") e.cancelFunc() e.activeBackgroundWorkers.Wait() diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go index 018ebfb203e..9ece31db36b 100644 --- a/components/encoder/incremental/incremental_test.go +++ b/components/encoder/incremental/incremental_test.go @@ -1 +1,171 @@ package incremental + +import ( + "context" + "testing" + "time" + + "github.com/edaniels/golog" + "go.viam.com/test" + "go.viam.com/utils/testutils" + + "go.viam.com/rdk/components/board" + fakeboard "go.viam.com/rdk/components/board/fake" + "go.viam.com/rdk/config" + "go.viam.com/rdk/registry" +) + +func TestConfig(t *testing.T) { + ctx := context.Background() + + b := MakeBoard(t) + + deps := make(registry.Dependencies) + deps[board.Named("main")] = b + + t.Run("valid config", func(t *testing.T) { + ic := AttrConfig{ + BoardName: "main", + Pins: Pins{A: "11", B: "13"}, + } + + rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} + + _, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + + test.That(t, err, test.ShouldBeNil) + }) + t.Run("invalid config", func(t *testing.T) { + ic := AttrConfig{ + BoardName: "pi", + // Pins intentionally missing + } + + rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} + + _, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldNotBeNil) + }) +} + +func TestEnconder(t *testing.T) { + ctx := context.Background() + + b := MakeBoard(t) + + deps := make(registry.Dependencies) + deps[board.Named("main")] = b + + ic := AttrConfig{ + BoardName: "main", + Pins: Pins{A: "11", B: "13"}, + } + + rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} + + t.Run("run forward", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + err = enc2.B.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + err = enc2.A.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, ticks, test.ShouldEqual, 1) + }) + }) + + t.Run("run backward", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + err = enc2.A.Tick(context.Background(), false, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + err = enc2.B.Tick(context.Background(), false, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, ticks, test.ShouldEqual, -1) + }) + }) + + t.Run("reset position", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + // set to a positive int + err = enc.ResetPosition(context.Background(), 5.0, nil) + test.That(t, err, test.ShouldBeNil) + ticks, err := enc.GetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, ticks, test.ShouldEqual, 5) + + // set to a negative int + err = enc.ResetPosition(context.Background(), -5.0, nil) + test.That(t, err, test.ShouldBeNil) + ticks, err = enc.GetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, ticks, test.ShouldEqual, -5) + + // set to a positive float + err = enc.ResetPosition(context.Background(), 1, nil) + test.That(t, err, test.ShouldBeNil) + ticks, err = enc.GetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, ticks, test.ShouldEqual, 1) + + // set to a negative float + err = enc.ResetPosition(context.Background(), -1000, nil) + test.That(t, err, test.ShouldBeNil) + ticks, err = enc.GetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, ticks, test.ShouldEqual, -1000) + + // set to 09 + err = enc.ResetPosition(context.Background(), 0, nil) + test.That(t, err, test.ShouldBeNil) + ticks, err = enc.GetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, ticks, test.ShouldEqual, 0) + }) +} + +func MakeBoard(t *testing.T) *fakeboard.Board { + interrupt, _ := board.CreateDigitalInterrupt(board.DigitalInterruptConfig{ + Name: "11", + Pin: "11", + Type: "basic", + }) + + interrupt1, _ := board.CreateDigitalInterrupt(board.DigitalInterruptConfig{ + Name: "13", + Pin: "13", + Type: "basic", + }) + + interrupts := map[string]board.DigitalInterrupt{ + "11": interrupt, + "13": interrupt1, + } + + b := fakeboard.Board{ + GPIOPins: map[string]*fakeboard.GPIOPin{}, + Digitals: interrupts, + } + + return &b +} diff --git a/components/encoder/register/register.go b/components/encoder/register/register.go new file mode 100644 index 00000000000..f3d8b75d201 --- /dev/null +++ b/components/encoder/register/register.go @@ -0,0 +1,9 @@ +// Package register registers all relevant MovementSensors +package register + +import ( + // Load all encoders. + _ "go.viam.com/rdk/components/encoder/AMS" + _ "go.viam.com/rdk/components/encoder/incremental" + _ "go.viam.com/rdk/components/encoder/single" +) diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 58c44804c32..3026d4e5b26 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -1,18 +1,17 @@ -package single - /* - This driver implements a single-wire odometer, such as LM393, as an encoder. - This allows the attached motor to determine its relative position. - This class of encoders requires a single digital interrupt pin. +Package single implements a single-wire odometer, such as LM393, as an encoder. +This allows the attached motor to determine its relative position. +This class of encoders requires a single digital interrupt pin. - This encoder must be connected to a motor (or another component that supports encoders - and reports the direction it is moving) in order to record readings. - The motor indicates in which direction it is spinning, thus indicating if the encoder - should increment or decrement reading value. +This encoder must be connected to a motor (or another component that supports encoders +and reports the direction it is moving) in order to record readings. +The motor indicates in which direction it is spinning, thus indicating if the encoder +should increment or decrement reading value. - Resetting a position must set the position to an int64. A floating point input will be rounded. +Resetting a position must set the position to an int64. A floating point input will be rounded. + +Sample configuration: - Sample configuration: { "pins" : { "i": 10 @@ -20,6 +19,7 @@ package single "board": "pi" } */ +package single import ( "context" @@ -65,14 +65,14 @@ func init() { &AttrConfig{}) } -// DirectionAware lets you ask what direction something is moving. Only used for SingleEncoder for now, unclear future. +// DirectionAware lets you ask what direction something is moving. Only used for Encoder for now, unclear future. // DirectionMoving returns -1 if the motor is currently turning backwards, 1 if forwards and 0 if off. type DirectionAware interface { DirectionMoving() int64 } -// SingleEncoder keeps track of a motor position using a rotary encoder.s -type SingleEncoder struct { +// Encoder keeps track of a motor position using a rotary encoder.s. +type Encoder struct { generic.Unimplemented name string I board.DigitalInterrupt @@ -85,15 +85,15 @@ type SingleEncoder struct { activeBackgroundWorkers sync.WaitGroup } -// SingleWirePin describes the configuration of Pins for a Single encoder. -type SinglePin struct { +// Pin describes the configuration of Pins for a Single encoder. +type Pin struct { I string `json:"i"` } // AttrConfig describes the configuration of a single encoder. type AttrConfig struct { - Pins SinglePin `json:"pins"` - BoardName string `json:"board"` + Pins Pin `json:"pins"` + BoardName string `json:"board"` } // Validate ensures all parts of the config are valid. @@ -113,11 +113,11 @@ func (cfg *AttrConfig) Validate(path string) ([]string, error) { } // AttachDirectionalAwareness to pre-created encoder. -func (e *SingleEncoder) AttachDirectionalAwareness(da DirectionAware) { +func (e *Encoder) AttachDirectionalAwareness(da DirectionAware) { e.m = da } -// NewSingleEncoder creates a new SingleEncoder. +// NewSingleEncoder creates a new Encoder. func NewSingleEncoder( ctx context.Context, deps registry.Dependencies, @@ -130,7 +130,7 @@ func NewSingleEncoder( } cancelCtx, cancelFunc := context.WithCancel(ctx) - e := &SingleEncoder{name: rawConfig.Name, logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0} + e := &Encoder{name: rawConfig.Name, logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0} board, err := board.FromDependencies(deps, cfg.BoardName) if err != nil { @@ -139,7 +139,7 @@ func NewSingleEncoder( e.I, ok = board.DigitalInterruptByName(cfg.Pins.I) if !ok { - return nil, errors.Errorf("cannot find pin (%s) for SingleEncoder", cfg.Pins.I) + return nil, errors.Errorf("cannot find pin (%s) for Encoder", cfg.Pins.I) } e.Start(ctx) @@ -147,8 +147,8 @@ func NewSingleEncoder( return e, nil } -// Start starts the SingleEncoder background thread. -func (e *SingleEncoder) Start(ctx context.Context) { +// Start starts the Encoder background thread. +func (e *Encoder) Start(ctx context.Context) { encoderChannel := make(chan board.Tick) e.I.AddCallback(encoderChannel) e.activeBackgroundWorkers.Add(1) @@ -183,21 +183,21 @@ func (e *SingleEncoder) Start(ctx context.Context) { } // GetPosition returns the current position. -func (e *SingleEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { res := atomic.LoadInt64(&e.position) return float64(res), nil } // ResetPosition sets the current position of the motor (adjusted by a given offset). -func (e *SingleEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { offsetInt := int64(math.Round(offset)) atomic.StoreInt64(&e.position, offsetInt) return nil } -// Close shuts down the SingleEncoder. -func (e *SingleEncoder) Close() error { - e.logger.Debug("Closing SingleEncoder") +// Close shuts down the Encoder. +func (e *Encoder) Close() error { + e.logger.Debug("Closing Encoder") e.cancelFunc() e.activeBackgroundWorkers.Wait() return nil diff --git a/components/encoder/single/single_encoder_test.go b/components/encoder/single/single_encoder_test.go index df0ae3fb011..225cf105c6c 100644 --- a/components/encoder/single/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -26,7 +26,7 @@ func TestConfig(t *testing.T) { t.Run("valid config", func(t *testing.T) { ic := AttrConfig{ BoardName: "main", - Pins: SinglePin{I: "10"}, + Pins: Pin{I: "10"}, } rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} @@ -58,7 +58,7 @@ func TestEnconder(t *testing.T) { ic := AttrConfig{ BoardName: "main", - Pins: SinglePin{I: "10"}, + Pins: Pin{I: "10"}, } rawcfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} @@ -66,7 +66,7 @@ func TestEnconder(t *testing.T) { t.Run("run forward", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - enc2 := enc.(*SingleEncoder) + enc2 := enc.(*Encoder) defer enc2.Close() m := &FakeDir{1} // forward @@ -86,7 +86,7 @@ func TestEnconder(t *testing.T) { t.Run("run backward", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - enc2 := enc.(*SingleEncoder) + enc2 := enc.(*Encoder) defer enc2.Close() m := &FakeDir{-1} // backward @@ -108,7 +108,7 @@ func TestEnconder(t *testing.T) { t.Run("run no direction", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - enc2 := enc.(*SingleEncoder) + enc2 := enc.(*Encoder) defer enc2.Close() err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) @@ -127,7 +127,7 @@ func TestEnconder(t *testing.T) { t.Run("reset position", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - enc2 := enc.(*SingleEncoder) + enc2 := enc.(*Encoder) defer enc2.Close() // set to a positive int @@ -169,7 +169,7 @@ func TestEnconder(t *testing.T) { t.Run("reset position and tick", func(t *testing.T) { enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - enc2 := enc.(*SingleEncoder) + enc2 := enc.(*Encoder) defer enc2.Close() m := &FakeDir{1} // forward diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index c5289b0302b..5d75762bb1b 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -73,7 +73,7 @@ func WrapMotorWithEncoder( return nil, err } - single, isSingle := rutils.UnwrapProxy(e).(*single.SingleEncoder) + single, isSingle := rutils.UnwrapProxy(e).(*single.Encoder) if isSingle { single.AttachDirectionalAwareness(mm) logger.Info("direction attached to single encoder from encoded motor") diff --git a/components/motor/gpio/motor_encoder_test.go b/components/motor/gpio/motor_encoder_test.go index 704db40ef71..93b40cad0ef 100644 --- a/components/motor/gpio/motor_encoder_test.go +++ b/components/motor/gpio/motor_encoder_test.go @@ -45,7 +45,7 @@ func TestMotorEncoder1(t *testing.T) { } interrupt := &board.BasicDigitalInterrupt{} - e := &single.SingleEncoder{I: interrupt, CancelCtx: context.Background()} + e := &single.Encoder{I: interrupt, CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: fakeMotor}) e.Start(context.Background()) dirFMotor, err := NewEncodedMotor(config.Component{}, cfg, fakeMotor, e, logger) @@ -278,7 +278,7 @@ func TestMotorEncoderIncremental(t *testing.T) { defer undo() type testHarness struct { - Encoder *incremental.IncrementalEncoder + Encoder *incremental.Encoder EncoderA board.DigitalInterrupt EncoderB board.DigitalInterrupt RealMotor *fakemotor.Motor @@ -295,7 +295,7 @@ func TestMotorEncoderIncremental(t *testing.T) { } encoderA := &board.BasicDigitalInterrupt{} encoderB := &board.BasicDigitalInterrupt{} - encoder := &incremental.IncrementalEncoder{A: encoderA, B: encoderB, CancelCtx: context.Background()} + encoder := &incremental.Encoder{A: encoderA, B: encoderB, CancelCtx: context.Background()} encoder.Start(context.Background()) motorIfc, err := NewEncodedMotor(config.Component{}, cfg, fakeMotor, encoder, logger) @@ -597,7 +597,7 @@ func TestWrapMotorWithEncoder(t *testing.T) { test.That(t, err, test.ShouldBeNil) fakeMotor := &fakemotor.Motor{} b.Digitals["a"] = &board.BasicDigitalInterrupt{} - e := &single.SingleEncoder{I: b.Digitals["a"], CancelCtx: context.Background()} + e := &single.Encoder{I: b.Digitals["a"], CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: fakeMotor}) e.Start(context.Background()) @@ -624,7 +624,7 @@ func TestWrapMotorWithEncoder(t *testing.T) { fakeMotor := &fakemotor.Motor{} b.Digitals["a"] = &board.BasicDigitalInterrupt{} b.Digitals["b"] = &board.BasicDigitalInterrupt{} - e := &incremental.IncrementalEncoder{A: b.Digitals["a"], B: b.Digitals["b"], CancelCtx: context.Background()} + e := &incremental.Encoder{A: b.Digitals["a"], B: b.Digitals["b"], CancelCtx: context.Background()} e.Start(context.Background()) m, err := WrapMotorWithEncoder( @@ -656,7 +656,7 @@ func TestDirFlipMotor(t *testing.T) { } interrupt := &board.BasicDigitalInterrupt{} - e := &single.SingleEncoder{I: interrupt, CancelCtx: context.Background()} + e := &single.Encoder{I: interrupt, CancelCtx: context.Background()} e.AttachDirectionalAwareness(&fakeDirectionAware{m: dirflipFakeMotor}) e.Start(context.Background()) dirFMotor, err := NewEncodedMotor(config.Component{}, cfg, dirflipFakeMotor, e, logger) diff --git a/components/register/all.go b/components/register/all.go index 86ec6a67d9e..77d900f059b 100644 --- a/components/register/all.go +++ b/components/register/all.go @@ -8,6 +8,7 @@ import ( _ "go.viam.com/rdk/components/base/register" _ "go.viam.com/rdk/components/board/register" _ "go.viam.com/rdk/components/camera/register" + _ "go.viam.com/rdk/components/encoder/register" _ "go.viam.com/rdk/components/gantry/register" _ "go.viam.com/rdk/components/generic/register" _ "go.viam.com/rdk/components/gripper/register" diff --git a/config/config_test.go b/config/config_test.go index 76e76f20132..92af673cae5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -107,7 +107,7 @@ func TestConfig3(t *testing.T) { TicksPerRotation: 10000, }) test.That(t, cfg.Components[3].ConvertedAttributes, test.ShouldResemble, &incremental.AttrConfig{ - Pins: incremental.IncrementalPins{ + Pins: incremental.Pins{ A: "encoder-steering-b", B: "encoder-steering-a", }, From bbf4906d1333bb9e124d9b96e4af7f33ee6b9f7f Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Tue, 4 Apr 2023 15:20:42 -0400 Subject: [PATCH 03/10] update linux compatibility --- components/board/pi/impl/external_hardware_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/board/pi/impl/external_hardware_test.go b/components/board/pi/impl/external_hardware_test.go index f3ba80aaa52..24da5e284e8 100644 --- a/components/board/pi/impl/external_hardware_test.go +++ b/components/board/pi/impl/external_hardware_test.go @@ -15,6 +15,7 @@ import ( "go.viam.com/rdk/components/board/genericlinux" picommon "go.viam.com/rdk/components/board/pi/common" "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/components/encoder/incremental" "go.viam.com/rdk/components/motor" "go.viam.com/rdk/components/motor/gpio" "go.viam.com/rdk/components/servo" @@ -147,8 +148,8 @@ func TestPiHardware(t *testing.T) { deps := make(registry.Dependencies) _, err = encoderReg.Constructor(ctx, deps, config.Component{ - Name: "encoder1", ConvertedAttributes: &encoder.IncrementalConfig{ - Pins: encoder.IncrementalPins{ + Name: "encoder1", ConvertedAttributes: &incremental.AttrConfig{ + Pins: incremental.Pins{ A: "a", B: "b", }, From 98e38a812eef3c747dc35416068f45455766cd2c Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Thu, 6 Apr 2023 10:39:57 -0400 Subject: [PATCH 04/10] add getproperties to drivers --- components/board/arduino/encoder.go | 22 ++-- components/encoder/AMS/ams_as5048.go | 47 ++++---- components/encoder/collectors.go | 2 +- components/encoder/encoder.go | 43 ++++---- components/encoder/fake/encoder.go | 23 ++-- components/encoder/fake/encoder_test.go | 10 +- components/encoder/features.go | 36 +++++++ .../incremental/incremental_encoder.go | 22 ++-- .../encoder/incremental/incremental_test.go | 70 +++++++----- components/encoder/single/single_encoder.go | 19 +++- .../encoder/single/single_encoder_test.go | 102 +++++++++++------- components/motor/fake/motor.go | 4 +- components/motor/gpio/motor_encoder.go | 10 +- components/motor/gpio/motor_encoder_test.go | 24 ++--- 14 files changed, 282 insertions(+), 152 deletions(-) create mode 100644 components/encoder/features.go diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index d1dd8544047..33b10734cae 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -7,6 +7,7 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" + pb "go.viam.com/api/component/encoder/v1" rdkutils "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -103,7 +104,11 @@ func (cfg *EncoderConfig) Validate(path string) ([]string, error) { } // GetPosition returns number of ticks since last zeroing. -func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + err := errors.New("Encoder does not support PositionType Angle Degrees") + return 0, err + } res, err := e.board.runCommand("motor-position " + e.name) if err != nil { return 0, err @@ -119,11 +124,16 @@ func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) // ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position. -func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - if err := encoder.ValidateIntegerOffset(offset); err != nil { - return err - } - offsetInt := int64(offset) +func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + offsetInt := int64(0) _, err := e.board.runCommand(fmt.Sprintf("motor-zero %s %d", e.name, offsetInt)) return err } + +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: false, + }, nil +} diff --git a/components/encoder/AMS/ams_as5048.go b/components/encoder/AMS/ams_as5048.go index 85b5718204c..adeb3ce1b9e 100644 --- a/components/encoder/AMS/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -10,6 +10,7 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" "go.uber.org/zap" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -122,9 +123,9 @@ func (cfg *I2CAttrConfig) ValidateI2C(path string) error { return nil } -// AS5048 is a struct representing an instance of a hardware unit +// Encoder is a struct representing an instance of a hardware unit // in AMS's AS5048 series of Hall-effect encoders. -type AS5048 struct { +type Encoder struct { mu sync.RWMutex logger golog.Logger position float64 @@ -148,7 +149,7 @@ func newAS5048Encoder( return nil, rdkutils.NewUnexpectedTypeError(attr, cfg.ConvertedAttributes) } cancelCtx, cancel := context.WithCancel(ctx) - res := &AS5048{ + res := &Encoder{ connectionType: attr.ConnectionType, cancelCtx: cancelCtx, cancel: cancel, @@ -178,8 +179,8 @@ func newAS5048Encoder( return res, nil } -func (enc *AS5048) startPositionLoop(ctx context.Context) error { - if err := enc.ResetPosition(ctx, 0.0, map[string]interface{}{}); err != nil { +func (enc *Encoder) startPositionLoop(ctx context.Context) error { + if err := enc.ResetPosition(ctx, map[string]interface{}{}); err != nil { return err } enc.activeBackgroundWorkers.Add(1) @@ -199,7 +200,7 @@ func (enc *AS5048) startPositionLoop(ctx context.Context) error { return nil } -func (enc *AS5048) readPosition(ctx context.Context) (float64, error) { +func (enc *Encoder) readPosition(ctx context.Context) (float64, error) { // retrieve the 8 most significant bits of the 14-bit resolution // position msB, err := enc.readByteDataFromBus(ctx, enc.i2cBus, enc.i2cAddr, byte(0xFE)) @@ -223,7 +224,7 @@ func convertBytesToAngle(msB, lsB byte) float64 { return (float64(byteData) * scalingFactor) } -func (enc *AS5048) updatePosition(ctx context.Context) error { +func (enc *Encoder) updatePosition(ctx context.Context) error { enc.mu.Lock() defer enc.mu.Unlock() angleDeg, err := enc.readPosition(ctx) @@ -251,29 +252,29 @@ func (enc *AS5048) updatePosition(ctx context.Context) error { // a user MUST set ticks_per_rotation on the config of the corresponding // motor to 1. Any other value will result in completely incorrect // position measurements by the motor. -func (enc *AS5048) GetPosition( - ctx context.Context, extra map[string]interface{}, +func (enc *Encoder) GetPosition( + ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}, ) (float64, error) { enc.mu.RLock() defer enc.mu.RUnlock() + if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + return enc.position, nil + } ticks := float64(enc.rotations) + enc.position/360.0 return ticks, nil } // ResetPosition sets the current position measured by the encoder to be -// considered its new zero position. If the offset provided is not 0.0, it -// also sets the positionOffset attribute and adjusts all future recorded -// positions by that offset (until the function is called again). -func (enc *AS5048) ResetPosition( - ctx context.Context, offset float64, extra map[string]interface{}, +// considered its new zero position. +func (enc *Encoder) ResetPosition( + ctx context.Context, extra map[string]interface{}, ) error { enc.mu.Lock() defer enc.mu.Unlock() // NOTE (GV): potential improvement could be writing the offset position // to the zero register of the encoder rather than keeping track // on the struct - enc.positionOffset = offset - enc.position = 0.0 + offset + enc.position = 0.0 currentMSB, err := enc.readByteDataFromBus(ctx, enc.i2cBus, enc.i2cAddr, byte(0xFE)) if err != nil { return err @@ -303,9 +304,17 @@ func (enc *AS5048) ResetPosition( return nil } +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (enc *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: true, + }, nil +} + // Close stops the position loop of the encoder when the component // is closed. -func (enc *AS5048) Close() error { +func (enc *Encoder) Close() error { enc.cancel() enc.activeBackgroundWorkers.Wait() return nil @@ -313,7 +322,7 @@ func (enc *AS5048) Close() error { // readByteDataFromBus opens a handle for the bus adhoc to perform a single read // and returns the result. The handle is closed at the end. -func (enc *AS5048) readByteDataFromBus(ctx context.Context, bus board.I2C, addr, register byte) (byte, error) { +func (enc *Encoder) readByteDataFromBus(ctx context.Context, bus board.I2C, addr, register byte) (byte, error) { i2cHandle, err := bus.OpenHandle(addr) if err != nil { return 0, err @@ -328,7 +337,7 @@ func (enc *AS5048) readByteDataFromBus(ctx context.Context, bus board.I2C, addr, // writeByteDataToBus opens a handle for the bus adhoc to perform a single write. // The handle is closed at the end. -func (enc *AS5048) writeByteDataToBus(ctx context.Context, bus board.I2C, addr, register, data byte) error { +func (enc *Encoder) writeByteDataToBus(ctx context.Context, bus board.I2C, addr, register, data byte) error { i2cHandle, err := bus.OpenHandle(addr) if err != nil { return err diff --git a/components/encoder/collectors.go b/components/encoder/collectors.go index f2537d61f08..1040ec32fe4 100644 --- a/components/encoder/collectors.go +++ b/components/encoder/collectors.go @@ -33,7 +33,7 @@ func newTicksCountCollector(resource interface{}, params data.CollectorParams) ( } cFunc := data.CaptureFunc(func(ctx context.Context, _ map[string]*anypb.Any) (interface{}, error) { - v, err := encoder.GetPosition(ctx, nil) + v, err := encoder.GetPosition(ctx, nil, nil) if err != nil { return nil, data.FailedToReadErr(params.ComponentName, ticksCount.String(), err) } diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index 7fd3cd7e885..03aaa3b4e64 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -6,7 +6,7 @@ import ( "sync" "github.com/edaniels/golog" - "github.com/pkg/errors" + pb "go.viam.com/api/component/encoder/v1" viamutils "go.viam.com/utils" "go.viam.com/rdk/components/generic" @@ -40,11 +40,13 @@ var Subtype = resource.NewSubtype( // A Encoder turns a position into a signal. type Encoder interface { // GetPosition returns number of ticks since last zeroing - GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) + GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) - // ResetPosition sets the current position of the motor (adjusted by a given offset) - // to be its new zero position. - ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error + // ResetPosition sets the current position of the motor to be its new zero position. + ResetPosition(ctx context.Context, extra map[string]interface{}) error + + // GetProperties returns a list of all the position types that are supported by a given encoder + GetProperties(ctx context.Context, extra map[string]interface{}) (map[Feature]bool, error) generic.Generic } @@ -104,16 +106,26 @@ func (r *reconfigurableEncoder) DoCommand(ctx context.Context, cmd map[string]in return r.actual.DoCommand(ctx, cmd) } -func (r *reconfigurableEncoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (r *reconfigurableEncoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, error) { + r.mu.RLock() + defer r.mu.RUnlock() + return r.actual.GetPosition(ctx, positionType, extra) +} + +func (r *reconfigurableEncoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { r.mu.RLock() defer r.mu.RUnlock() - return r.actual.GetPosition(ctx, extra) + return r.actual.ResetPosition(ctx, extra) } -func (r *reconfigurableEncoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { +func (r *reconfigurableEncoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[Feature]bool, error) { r.mu.RLock() defer r.mu.RUnlock() - return r.actual.ResetPosition(ctx, offset, extra) + return r.actual.GetProperties(ctx, extra) } func (r *reconfigurableEncoder) Close(ctx context.Context) error { @@ -152,16 +164,3 @@ func WrapWithReconfigurable(r interface{}, name resource.Name) (resource.Reconfi } return &reconfigurableEncoder{name: name, actual: m}, nil } - -// ValidateIntegerOffset returns an error if a non-integral value for offset -// is passed to Reset for an incremental encoder (these encoders count based on -// square-wave pulses and so cannot be supplied an offset that is not an integer). -func ValidateIntegerOffset(offset float64) error { - if offset != float64(int64(offset)) { - return errors.Errorf( - "incremental encoders can only reset with integer value offsets, value passed was %f", - offset, - ) - } - return nil -} diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 5d1ebc2100b..837427cebe9 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -3,10 +3,12 @@ package fake import ( "context" + "errors" "sync" "time" "github.com/edaniels/golog" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/encoder" @@ -66,7 +68,11 @@ type Encoder struct { } // GetPosition returns the current position in terms of ticks. -func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + err := errors.New("Encoder does not support PositionType Angle Degrees") + return 0, err + } e.mu.Lock() defer e.mu.Unlock() return float64(e.position), nil @@ -102,16 +108,21 @@ func (e *Encoder) Start(cancelCtx context.Context) { // ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position. -func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - if err := encoder.ValidateIntegerOffset(offset); err != nil { - return err - } +func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { e.mu.Lock() defer e.mu.Unlock() - e.position = int64(offset) + e.position = int64(0) return nil } +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: true, + }, nil +} + // SetSpeed sets the speed of the fake motor the encoder is measuring. func (e *Encoder) SetSpeed(ctx context.Context, speed float64) error { e.mu.Lock() diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index 220ee650b72..5dd5180f174 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -15,24 +15,24 @@ func TestEncoder(t *testing.T) { // Get and set position t.Run("get and set position", func(t *testing.T) { - pos, err := e.GetPosition(ctx, nil) + pos, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) err = e.SetPosition(ctx, 1) test.That(t, err, test.ShouldBeNil) - pos, err = e.GetPosition(ctx, nil) + pos, err = e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 1) test.That(t, err, test.ShouldBeNil) }) // Reset t.Run("reset to zero", func(t *testing.T) { - err := e.ResetPosition(ctx, 0, nil) + err := e.ResetPosition(ctx, nil) test.That(t, err, test.ShouldBeNil) - pos, err := e.GetPosition(ctx, nil) + pos, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) }) @@ -61,7 +61,7 @@ func TestEncoder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - pos, err := e.GetPosition(ctx, nil) + pos, err := e.GetPosition(ctx, nil, nil) test.That(tb, pos, test.ShouldBeGreaterThan, 0) test.That(tb, err, test.ShouldBeNil) }) diff --git a/components/encoder/features.go b/components/encoder/features.go new file mode 100644 index 00000000000..c6982e02092 --- /dev/null +++ b/components/encoder/features.go @@ -0,0 +1,36 @@ +// Package encoder contains an enum representing optional encoder features +package encoder + +import ( + pb "go.viam.com/api/component/encoder/v1" +) + +// Feature is an enum representing an optional motor feature. +type Feature string + +// PositionReporting represesnts the feature of a motor being +// able to report its own position. +const ( + TicksCountSupported Feature = "Ticks" + AngleDegreesSupported Feature = "Degrees" +) + +// ProtoFeaturesToMap takes a GetPropertiesResponse and returns +// an equivalent Feature-to-boolean map. +func ProtoFeaturesToMap(resp *pb.GetPropertiesResponse) map[Feature]bool { + return map[Feature]bool{ + TicksCountSupported: resp.TicksCountSupported, + AngleDegreesSupported: resp.AngleDegreesSupported, + } +} + +// FeatureMapToProtoResponse takes a map of features to booleans (indicating +// whether the feature is supported) and converts it to a GetPropertiesResponse. +func FeatureMapToProtoResponse( + featureMap map[Feature]bool, +) (*pb.GetPropertiesResponse, error) { + return &pb.GetPropertiesResponse{ + TicksCountSupported: featureMap[TicksCountSupported], + AngleDegreesSupported: featureMap[AngleDegreesSupported], + }, nil +} diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index 0820e4237a3..adc4a1d1798 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -8,6 +8,7 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -233,23 +234,32 @@ func (e *Encoder) Start(ctx context.Context) { } // GetPosition returns number of ticks since last zeroing. -func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + err := errors.New("Encoder does not support PositionType Angle Degrees") + return 0, err + } res := atomic.LoadInt64(&e.position) return float64(res), nil } // ResetPosition sets the current position of the motor (adjusted by a given offset) // to be its new zero position.. -func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - if err := encoder.ValidateIntegerOffset(offset); err != nil { - return err - } - offsetInt := int64(offset) +func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + offsetInt := int64(0) atomic.StoreInt64(&e.position, offsetInt) atomic.StoreInt64(&e.pRaw, (offsetInt<<1)|atomic.LoadInt64(&e.pRaw)&0x1) return nil } +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: false, + }, nil +} + // RawPosition returns the raw position of the encoder. func (e *Encoder) RawPosition() int64 { return atomic.LoadInt64(&e.pRaw) diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go index 9ece31db36b..e219720d8e0 100644 --- a/components/encoder/incremental/incremental_test.go +++ b/components/encoder/incremental/incremental_test.go @@ -6,11 +6,13 @@ import ( "time" "github.com/edaniels/golog" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" "go.viam.com/rdk/components/board" fakeboard "go.viam.com/rdk/components/board/fake" + "go.viam.com/rdk/components/encoder" "go.viam.com/rdk/config" "go.viam.com/rdk/registry" ) @@ -76,7 +78,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -95,7 +97,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, -1) }) @@ -107,40 +109,54 @@ func TestEnconder(t *testing.T) { enc2 := enc.(*Encoder) defer enc2.Close() - // set to a positive int - err = enc.ResetPosition(context.Background(), 5.0, nil) + // reset position to 0 + err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, 5) + test.That(t, ticks, test.ShouldEqual, 0) + }) - // set to a negative int - err = enc.ResetPosition(context.Background(), -5.0, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) + t.Run("specify correct position type", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, -5) + enc2 := enc.(*Encoder) + defer enc2.Close() - // set to a positive float - err = enc.ResetPosition(context.Background(), 1, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, ticks, test.ShouldEqual, 0) + }) + }) + t.Run("specify wrong position type", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, 1) + enc2 := enc.(*Encoder) + defer enc2.Close() - // set to a negative float - err = enc.ResetPosition(context.Background(), -1000, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) - test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, -1000) + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + test.That(tb, err, test.ShouldNotBeNil) + test.That(tb, ticks, test.ShouldEqual, 0) + }) + }) - // set to 09 - err = enc.ResetPosition(context.Background(), 0, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) + t.Run("get properties", func(t *testing.T) { + enc, err := NewIncrementalEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, 0) + enc2 := enc.(*Encoder) + defer enc2.Close() + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + props, err := enc.GetProperties(ctx, nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue) + test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse) + }) }) } diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 3026d4e5b26..247d4e9fe3d 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -29,6 +29,7 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -183,18 +184,30 @@ func (e *Encoder) Start(ctx context.Context) { } // GetPosition returns the current position. -func (e *Encoder) GetPosition(ctx context.Context, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + err := errors.New("Encoder does not support PositionType Angle Degrees") + return 0, err + } res := atomic.LoadInt64(&e.position) return float64(res), nil } // ResetPosition sets the current position of the motor (adjusted by a given offset). -func (e *Encoder) ResetPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - offsetInt := int64(math.Round(offset)) +func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + offsetInt := int64(math.Round(0)) atomic.StoreInt64(&e.position, offsetInt) return nil } +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: false, + }, nil +} + // Close shuts down the Encoder. func (e *Encoder) Close() error { e.logger.Debug("Closing Encoder") diff --git a/components/encoder/single/single_encoder_test.go b/components/encoder/single/single_encoder_test.go index 225cf105c6c..0b80c03a26f 100644 --- a/components/encoder/single/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -6,11 +6,13 @@ import ( "time" "github.com/edaniels/golog" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" "go.viam.com/rdk/components/board" fakeboard "go.viam.com/rdk/components/board/fake" + "go.viam.com/rdk/components/encoder" "go.viam.com/rdk/config" "go.viam.com/rdk/registry" ) @@ -77,7 +79,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -97,7 +99,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, -1) }) @@ -119,7 +121,7 @@ func TestEnconder(t *testing.T) { // by the encoder worker time.Sleep(50 * time.Millisecond) - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -130,38 +132,10 @@ func TestEnconder(t *testing.T) { enc2 := enc.(*Encoder) defer enc2.Close() - // set to a positive int - err = enc.ResetPosition(context.Background(), 5.0, nil) + // reset position to 0 + err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil) - test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, 5) - - // set to a negative int - err = enc.ResetPosition(context.Background(), -5.0, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) - test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, -5) - - // set to a positive float - err = enc.ResetPosition(context.Background(), 1.2, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) - test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, 1) - - // set to a negative float - err = enc.ResetPosition(context.Background(), -1000.999, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) - test.That(t, err, test.ShouldBeNil) - test.That(t, ticks, test.ShouldEqual, -1001) - - // set to 09 - err = enc.ResetPosition(context.Background(), 0, nil) - test.That(t, err, test.ShouldBeNil) - ticks, err = enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -181,15 +155,15 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) // reset tick - err = enc.ResetPosition(context.Background(), 0, nil) + err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) @@ -199,11 +173,63 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil) + ticks, err := enc.GetPosition(context.Background(), nil, nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, ticks, test.ShouldEqual, 1) + }) + }) + t.Run("specify correct position type", func(t *testing.T) { + enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + m := &FakeDir{1} // forward + enc2.AttachDirectionalAwareness(m) + + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) }) + t.Run("specify wrong position type", func(t *testing.T) { + enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + m := &FakeDir{-1} // backward + enc2.AttachDirectionalAwareness(m) + + err = enc2.I.Tick(context.Background(), true, uint64(time.Now().UnixNano())) + test.That(t, err, test.ShouldBeNil) + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + test.That(tb, err, test.ShouldNotBeNil) + test.That(tb, ticks, test.ShouldEqual, 0) + }) + }) + t.Run("get properties", func(t *testing.T) { + enc, err := NewSingleEncoder(ctx, deps, rawcfg, golog.NewTestLogger(t)) + test.That(t, err, test.ShouldBeNil) + enc2 := enc.(*Encoder) + defer enc2.Close() + + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + props, err := enc.GetProperties(ctx, nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue) + test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse) + }) + }) } func MakeBoard(t *testing.T) *fakeboard.Board { diff --git a/components/motor/fake/motor.go b/components/motor/fake/motor.go index fd07ffaf6d5..c38325b0b56 100644 --- a/components/motor/fake/motor.go +++ b/components/motor/fake/motor.go @@ -151,7 +151,7 @@ func (m *Motor) Position(ctx context.Context, extra map[string]interface{}) (flo return 0, errors.New("encoder is not defined") } - ticks, err := m.Encoder.GetPosition(ctx, extra) + ticks, err := m.Encoder.GetPosition(ctx, nil, extra) if err != nil { return 0, err } @@ -338,7 +338,7 @@ func (m *Motor) ResetZeroPosition(ctx context.Context, offset float64, extra map return errors.New("need nonzero TicksPerRotation for motor") } - err := m.Encoder.ResetPosition(ctx, offset*float64(m.TicksPerRotation), extra) + err := m.Encoder.ResetPosition(ctx, extra) if err != nil { return errors.Wrapf(err, "error in ResetZeroPosition from motor (%s)", m.Name) } diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index 5d75762bb1b..bb85e63b8c3 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -199,7 +199,7 @@ type EncodedMotorState struct { // Position returns the position of the motor. func (m *EncodedMotor) Position(ctx context.Context, extra map[string]interface{}) (float64, error) { - ticks, err := m.encoder.GetPosition(ctx, extra) + ticks, err := m.encoder.GetPosition(ctx, nil, extra) if err != nil { return 0, err } @@ -304,7 +304,7 @@ func (m *EncodedMotor) rpmMonitor() { m.startedRPMMonitor = true m.startedRPMMonitorMu.Unlock() - lastPosFl, err := m.encoder.GetPosition(m.cancelCtx, nil) + lastPosFl, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) if err != nil { panic(err) } @@ -328,7 +328,7 @@ func (m *EncodedMotor) rpmMonitor() { case <-timer.C: } - pos, err := m.encoder.GetPosition(m.cancelCtx, nil) + pos, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) if err != nil { m.logger.Info("error getting encoder position, sleeping then continuing: %w", err) if !utils.SelectContextOrWait(m.cancelCtx, 100*time.Millisecond) { @@ -564,7 +564,7 @@ func (m *EncodedMotor) goForInternal(ctx context.Context, rpm, revolutions float numTicks := int64(revolutions * float64(m.cfg.TicksPerRotation)) - pos, err := m.encoder.GetPosition(ctx, nil) + pos, err := m.encoder.GetPosition(ctx, nil, nil) if err != nil { return err } @@ -713,5 +713,5 @@ func (m *EncodedMotor) GoTillStop(ctx context.Context, rpm float64, stopFunc fun // ResetZeroPosition sets the current position of the motor specified by the request // (adjusted by a given offset) to be its new zero position. func (m *EncodedMotor) ResetZeroPosition(ctx context.Context, offset float64, extra map[string]interface{}) error { - return m.encoder.ResetPosition(ctx, offset*float64(m.cfg.TicksPerRotation), extra) + return m.encoder.ResetPosition(ctx, extra) } diff --git a/components/motor/gpio/motor_encoder_test.go b/components/motor/gpio/motor_encoder_test.go index 93b40cad0ef..c4b1715372e 100644 --- a/components/motor/gpio/motor_encoder_test.go +++ b/components/motor/gpio/motor_encoder_test.go @@ -356,7 +356,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -367,7 +367,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -378,7 +378,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -389,7 +389,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -416,7 +416,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -427,7 +427,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -438,7 +438,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -449,7 +449,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -460,7 +460,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -471,7 +471,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -482,7 +482,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -493,7 +493,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 0) - posFl, err := (*encoder).GetPosition(context.Background(), nil) + posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) From 06730f570d315790927e6f68dd152bc598769caf Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Thu, 6 Apr 2023 12:53:01 -0400 Subject: [PATCH 05/10] update client and server to include GetProperties and PositionType --- components/encoder/client.go | 75 ++++++++ components/encoder/client_test.go | 178 ++++++++++++++++++ components/encoder/encoder.go | 15 +- components/encoder/encoder_test.go | 285 +++++++++++++++++++++++++++++ components/encoder/server.go | 96 ++++++++++ components/encoder/server_test.go | 108 +++++++++++ testutils/inject/encoder.go | 50 +++++ 7 files changed, 806 insertions(+), 1 deletion(-) create mode 100644 components/encoder/client.go create mode 100644 components/encoder/client_test.go create mode 100644 components/encoder/encoder_test.go create mode 100644 components/encoder/server.go create mode 100644 components/encoder/server_test.go create mode 100644 testutils/inject/encoder.go diff --git a/components/encoder/client.go b/components/encoder/client.go new file mode 100644 index 00000000000..d4dda78dab9 --- /dev/null +++ b/components/encoder/client.go @@ -0,0 +1,75 @@ +package encoder + +import ( + "context" + + "github.com/edaniels/golog" + pb "go.viam.com/api/component/encoder/v1" + "go.viam.com/utils/rpc" + "google.golang.org/protobuf/types/known/structpb" + + "go.viam.com/rdk/protoutils" +) + +// client implements EncoderServiceClient. +type client struct { + name string + conn rpc.ClientConn + client pb.EncoderServiceClient + logger golog.Logger +} + +// NewClientFromConn constructs a new Client from connection passed in. +func NewClientFromConn(ctx context.Context, conn rpc.ClientConn, name string, logger golog.Logger) Encoder { + c := pb.NewEncoderServiceClient(conn) + return &client{ + name: name, + conn: conn, + client: c, + logger: logger, + } +} + +// GetPosition returns number of ticks since last zeroing. +func (c *client) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + ext, err := structpb.NewStruct(extra) + if err != nil { + return 0, err + } + req := &pb.GetPositionRequest{Name: c.name, Extra: ext} + resp, err := c.client.GetPosition(ctx, req) + if err != nil { + return 0, err + } + return float64(resp.Value), nil +} + +// ResetPosition sets the current position of the motor (adjusted by a given offset) +// to be its new zero position. +func (c *client) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + ext, err := structpb.NewStruct(extra) + if err != nil { + return err + } + req := &pb.ResetPositionRequest{Name: c.name, Extra: ext} + _, err = c.client.ResetPosition(ctx, req) + return err +} + +// GetProperties returns a list of all the position types that are supported by a given encoder. +func (c *client) GetProperties(ctx context.Context, extra map[string]interface{}) (map[Feature]bool, error) { + ext, err := structpb.NewStruct(extra) + if err != nil { + return nil, err + } + req := &pb.GetPropertiesRequest{Name: c.name, Extra: ext} + resp, err := c.client.GetProperties(ctx, req) + if err != nil { + return nil, err + } + return ProtoFeaturesToMap(resp), nil +} + +func (c *client) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) { + return protoutils.DoFromResourceClient(ctx, c.client, c.name, cmd) +} diff --git a/components/encoder/client_test.go b/components/encoder/client_test.go new file mode 100644 index 00000000000..9db9f00b3b4 --- /dev/null +++ b/components/encoder/client_test.go @@ -0,0 +1,178 @@ +package encoder_test + +import ( + "context" + "errors" + "net" + "testing" + + "github.com/edaniels/golog" + pb "go.viam.com/api/component/encoder/v1" + "go.viam.com/test" + "go.viam.com/utils" + "go.viam.com/utils/rpc" + "google.golang.org/grpc" + + "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/components/generic" + viamgrpc "go.viam.com/rdk/grpc" + "go.viam.com/rdk/registry" + "go.viam.com/rdk/resource" + "go.viam.com/rdk/subtype" + "go.viam.com/rdk/testutils" + "go.viam.com/rdk/testutils/inject" +) + +func TestClient(t *testing.T) { + logger := golog.NewTestLogger(t) + listener1, err := net.Listen("tcp", "localhost:0") + test.That(t, err, test.ShouldBeNil) + rpcServer, err := rpc.NewServer(logger, rpc.WithUnauthenticated()) + test.That(t, err, test.ShouldBeNil) + + workingEncoder := &inject.Encoder{} + failingEncoder := &inject.Encoder{} + + var actualExtra map[string]interface{} + + workingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { + actualExtra = extra + return nil + } + workingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + actualExtra = extra + return 42.0, nil + } + + failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { + return errors.New("set to zero failed") + } + failingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + return 0, errors.New("position unavailable") + } + + resourceMap := map[resource.Name]interface{}{ + encoder.Named(testEncoderName): workingEncoder, + encoder.Named(failEncoderName): failingEncoder, + } + encoderSvc, err := subtype.New(resourceMap) + test.That(t, err, test.ShouldBeNil) + resourceSubtype := registry.ResourceSubtypeLookup(encoder.Subtype) + resourceSubtype.RegisterRPCService(context.Background(), rpcServer, encoderSvc) + + workingEncoder.DoFunc = generic.EchoFunc + generic.RegisterService(rpcServer, encoderSvc) + + go rpcServer.Serve(listener1) + defer rpcServer.Stop() + + t.Run("Failing client", func(t *testing.T) { + cancelCtx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := viamgrpc.Dial(cancelCtx, listener1.Addr().String(), logger) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err.Error(), test.ShouldContainSubstring, "canceled") + }) + + conn, err := viamgrpc.Dial(context.Background(), listener1.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + workingEncoderClient := encoder.NewClientFromConn(context.Background(), conn, testEncoderName, logger) + + t.Run("client tests for working encoder", func(t *testing.T) { + // DoCommand + resp, err := workingEncoderClient.DoCommand(context.Background(), generic.TestCommand) + test.That(t, err, test.ShouldBeNil) + test.That(t, resp["command"], test.ShouldEqual, generic.TestCommand["command"]) + test.That(t, resp["data"], test.ShouldEqual, generic.TestCommand["data"]) + + err = workingEncoderClient.ResetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + + pos, err := workingEncoderClient.GetPosition( + context.Background(), + pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), + map[string]interface{}{"foo": "bar", "baz": []interface{}{1., 2., 3.}}) + test.That(t, err, test.ShouldBeNil) + test.That(t, pos, test.ShouldEqual, 42.0) + + test.That(t, actualExtra, test.ShouldResemble, map[string]interface{}{"foo": "bar", "baz": []interface{}{1., 2., 3.}}) + + test.That(t, utils.TryClose(context.Background(), workingEncoderClient), test.ShouldBeNil) + + test.That(t, conn.Close(), test.ShouldBeNil) + }) + + conn, err = viamgrpc.Dial(context.Background(), listener1.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + failingEncoderClient := encoder.NewClientFromConn(context.Background(), conn, failEncoderName, logger) + + t.Run("client tests for failing encoder", func(t *testing.T) { + err = failingEncoderClient.ResetPosition(context.Background(), nil) + test.That(t, err, test.ShouldNotBeNil) + + pos, err := failingEncoderClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, pos, test.ShouldEqual, 0.0) + + test.That(t, utils.TryClose(context.Background(), failingEncoderClient), test.ShouldBeNil) + }) + + t.Run("dialed client tests for working encoder", func(t *testing.T) { + conn, err := viamgrpc.Dial(context.Background(), listener1.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + workingEncoderDialedClient := encoder.NewClientFromConn(context.Background(), conn, testEncoderName, logger) + + pos, err := workingEncoderDialedClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, pos, test.ShouldEqual, 42.0) + + err = workingEncoderDialedClient.ResetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + + test.That(t, utils.TryClose(context.Background(), workingEncoderDialedClient), test.ShouldBeNil) + test.That(t, conn.Close(), test.ShouldBeNil) + }) + + t.Run("dialed client tests for failing encoder", func(t *testing.T) { + conn, err := viamgrpc.Dial(context.Background(), listener1.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + failingEncoderDialedClient := encoder.NewClientFromConn(context.Background(), conn, failEncoderName, logger) + + test.That(t, utils.TryClose(context.Background(), failingEncoderDialedClient), test.ShouldBeNil) + test.That(t, conn.Close(), test.ShouldBeNil) + }) + test.That(t, conn.Close(), test.ShouldBeNil) +} + +func TestClientDialerOption(t *testing.T) { + logger := golog.NewTestLogger(t) + listener, err := net.Listen("tcp", "localhost:0") + test.That(t, err, test.ShouldBeNil) + gServer := grpc.NewServer() + injectEncoder := &inject.Encoder{} + + encoderSvc, err := subtype.New(map[resource.Name]interface{}{encoder.Named(testEncoderName): injectEncoder}) + test.That(t, err, test.ShouldBeNil) + pb.RegisterEncoderServiceServer(gServer, encoder.NewServer(encoderSvc)) + + go gServer.Serve(listener) + defer gServer.Stop() + + td := &testutils.TrackingDialer{Dialer: rpc.NewCachedDialer()} + ctx := rpc.ContextWithDialer(context.Background(), td) + conn1, err := viamgrpc.Dial(ctx, listener.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + client1 := encoder.NewClientFromConn(ctx, conn1, testEncoderName, logger) + test.That(t, td.NewConnections, test.ShouldEqual, 3) + conn2, err := viamgrpc.Dial(ctx, listener.Addr().String(), logger) + test.That(t, err, test.ShouldBeNil) + client2 := encoder.NewClientFromConn(ctx, conn2, testEncoderName, logger) + test.That(t, td.NewConnections, test.ShouldEqual, 3) + + err = utils.TryClose(context.Background(), client1) + test.That(t, err, test.ShouldBeNil) + err = utils.TryClose(context.Background(), client2) + test.That(t, err, test.ShouldBeNil) + test.That(t, conn1.Close(), test.ShouldBeNil) + test.That(t, conn2.Close(), test.ShouldBeNil) +} diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index 03aaa3b4e64..eb52747bca7 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -8,18 +8,32 @@ import ( "github.com/edaniels/golog" pb "go.viam.com/api/component/encoder/v1" viamutils "go.viam.com/utils" + "go.viam.com/utils/rpc" "go.viam.com/rdk/components/generic" "go.viam.com/rdk/data" "go.viam.com/rdk/registry" "go.viam.com/rdk/resource" "go.viam.com/rdk/robot" + "go.viam.com/rdk/subtype" "go.viam.com/rdk/utils" ) func init() { registry.RegisterResourceSubtype(Subtype, registry.ResourceSubtype{ Reconfigurable: WrapWithReconfigurable, + RegisterRPCService: func(ctx context.Context, rpcServer rpc.Server, subtypeSvc subtype.Service) error { + return rpcServer.RegisterServiceServer( + ctx, + &pb.EncoderService_ServiceDesc, + NewServer(subtypeSvc), + pb.RegisterEncoderServiceHandlerFromEndpoint, + ) + }, + RPCServiceDesc: &pb.EncoderService_ServiceDesc, + RPCClient: func(ctx context.Context, conn rpc.ClientConn, name string, logger golog.Logger) interface{} { + return NewClientFromConn(ctx, conn, name, logger) + }, }) data.RegisterCollector(data.MethodMetadata{ Subtype: Subtype, @@ -59,7 +73,6 @@ func Named(name string) resource.Name { var ( _ = Encoder(&reconfigurableEncoder{}) _ = resource.Reconfigurable(&reconfigurableEncoder{}) - _ = resource.Reconfigurable(&reconfigurableEncoder{}) _ = viamutils.ContextCloser(&reconfigurableEncoder{}) ) diff --git a/components/encoder/encoder_test.go b/components/encoder/encoder_test.go new file mode 100644 index 00000000000..b1c842c4e3b --- /dev/null +++ b/components/encoder/encoder_test.go @@ -0,0 +1,285 @@ +package encoder_test + +import ( + "context" + "testing" + + pb "go.viam.com/api/component/encoder/v1" + "go.viam.com/test" + "go.viam.com/utils" + + "go.viam.com/rdk/components/arm" + "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/registry" + "go.viam.com/rdk/resource" + "go.viam.com/rdk/testutils/inject" + rutils "go.viam.com/rdk/utils" +) + +const ( + testEncoderName = "encoder1" + testEncoderName2 = "encoder2" + failEncoderName = "encoder3" + fakeEncoderName = "encoder4" + missingEncoderName = "encoder5" +) + +func setupDependencies(t *testing.T) registry.Dependencies { + t.Helper() + + deps := make(registry.Dependencies) + deps[encoder.Named(testEncoderName)] = &mock{Name: testEncoderName} + deps[encoder.Named(fakeEncoderName)] = "not a encoder" + return deps +} + +func setupInjectRobot() *inject.Robot { + encoder1 := &mock{Name: testEncoderName} + r := &inject.Robot{} + r.ResourceByNameFunc = func(name resource.Name) (interface{}, error) { + switch name { + case encoder.Named(testEncoderName): + return encoder1, nil + case encoder.Named(fakeEncoderName): + return "not a encoder", nil + default: + return nil, rutils.NewResourceNotFoundError(name) + } + } + r.ResourceNamesFunc = func() []resource.Name { + return []resource.Name{encoder.Named(testEncoderName), arm.Named("arm1")} + } + return r +} + +func TestGenericDo(t *testing.T) { + r := setupInjectRobot() + + m, err := encoder.FromRobot(r, testEncoderName) + test.That(t, err, test.ShouldBeNil) + test.That(t, m, test.ShouldNotBeNil) + + command := map[string]interface{}{"cmd": "test", "data1": 500} + ret, err := m.DoCommand(context.Background(), command) + test.That(t, err, test.ShouldBeNil) + test.That(t, ret, test.ShouldEqual, command) +} + +func TestFromDependencies(t *testing.T) { + deps := setupDependencies(t) + + res, err := encoder.FromDependencies(deps, testEncoderName) + test.That(t, err, test.ShouldBeNil) + test.That(t, res, test.ShouldNotBeNil) + + result, err := res.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, result, test.ShouldResemble, position) + + res, err = encoder.FromDependencies(deps, fakeEncoderName) + test.That(t, err, test.ShouldBeError, rutils.DependencyTypeError[encoder.Encoder](fakeEncoderName, "string")) + test.That(t, res, test.ShouldBeNil) + + res, err = encoder.FromDependencies(deps, missingEncoderName) + test.That(t, err, test.ShouldBeError, rutils.DependencyNotFoundError(missingEncoderName)) + test.That(t, res, test.ShouldBeNil) +} + +func TestFromRobot(t *testing.T) { + r := setupInjectRobot() + + res, err := encoder.FromRobot(r, testEncoderName) + test.That(t, err, test.ShouldBeNil) + test.That(t, res, test.ShouldNotBeNil) + + result, err := res.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, result, test.ShouldResemble, position) + + res, err = encoder.FromRobot(r, fakeEncoderName) + test.That(t, err, test.ShouldBeError, encoder.NewUnimplementedInterfaceError("string")) + test.That(t, res, test.ShouldBeNil) + + res, err = encoder.FromRobot(r, missingEncoderName) + test.That(t, err, test.ShouldBeError, rutils.NewResourceNotFoundError(encoder.Named(missingEncoderName))) + test.That(t, res, test.ShouldBeNil) +} + +func TestNamesFromRobot(t *testing.T) { + r := setupInjectRobot() + + names := encoder.NamesFromRobot(r) + test.That(t, names, test.ShouldResemble, []string{testEncoderName}) +} + +func TestEncoderName(t *testing.T) { + for _, tc := range []struct { + TestName string + Name string + Expected resource.Name + }{ + { + "missing name", + "", + resource.Name{ + Subtype: resource.Subtype{ + Type: resource.Type{Namespace: resource.ResourceNamespaceRDK, ResourceType: resource.ResourceTypeComponent}, + ResourceSubtype: encoder.SubtypeName, + }, + Name: "", + }, + }, + { + "all fields included", + testEncoderName, + resource.Name{ + Subtype: resource.Subtype{ + Type: resource.Type{Namespace: resource.ResourceNamespaceRDK, ResourceType: resource.ResourceTypeComponent}, + ResourceSubtype: encoder.SubtypeName, + }, + Name: testEncoderName, + }, + }, + } { + t.Run(tc.TestName, func(t *testing.T) { + observed := encoder.Named(tc.Name) + test.That(t, observed, test.ShouldResemble, tc.Expected) + }) + } +} + +func TestWrapWithReconfigurable(t *testing.T) { + var actualEncoder1 encoder.Encoder = &mock{Name: testEncoderName} + reconfEncoder1, err := encoder.WrapWithReconfigurable(actualEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + _, err = encoder.WrapWithReconfigurable(nil, resource.Name{}) + test.That(t, err, test.ShouldBeError, encoder.NewUnimplementedInterfaceError(nil)) + + reconfEncoder2, err := encoder.WrapWithReconfigurable(reconfEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + test.That(t, reconfEncoder2, test.ShouldEqual, reconfEncoder1) + + var actualEncoder2 encoder.Encoder = &mockLocal{Name: testEncoderName} + reconfEncoder3, err := encoder.WrapWithReconfigurable(actualEncoder2, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + reconfEncoder4, err := encoder.WrapWithReconfigurable(reconfEncoder3, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + test.That(t, reconfEncoder4, test.ShouldResemble, reconfEncoder3) + + _, ok := reconfEncoder4.(encoder.Encoder) + test.That(t, ok, test.ShouldBeTrue) +} + +func TestReconfigurableEncoder(t *testing.T) { + actualEncoder1 := &mock{Name: testEncoderName} + reconfEncoder1, err := encoder.WrapWithReconfigurable(actualEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + actualEncoder2 := &mock{Name: testEncoderName2} + reconfEncoder2, err := encoder.WrapWithReconfigurable(actualEncoder2, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + test.That(t, actualEncoder1.reconfCount, test.ShouldEqual, 0) + + err = reconfEncoder1.Reconfigure(context.Background(), reconfEncoder2) + test.That(t, err, test.ShouldBeNil) + test.That(t, reconfEncoder1, test.ShouldResemble, reconfEncoder2) + test.That(t, actualEncoder1.reconfCount, test.ShouldEqual, 1) + + test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) + test.That(t, actualEncoder2.posCount, test.ShouldEqual, 0) + result, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, result, test.ShouldResemble, position) + test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) + test.That(t, actualEncoder2.posCount, test.ShouldEqual, 1) + + err = reconfEncoder1.Reconfigure(context.Background(), nil) + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err, test.ShouldBeError, rutils.NewUnexpectedTypeError(reconfEncoder1, nil)) + + actualEncoder3 := &mock{Name: failEncoderName} + reconfEncoder3, err := encoder.WrapWithReconfigurable(actualEncoder3, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + test.That(t, reconfEncoder3, test.ShouldNotBeNil) +} + +func TestResetPosition(t *testing.T) { + actualEncoder1 := &mock{Name: testEncoderName} + reconfEncoder1, err := encoder.WrapWithReconfigurable(actualEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + test.That(t, actualEncoder1.zeroCount, test.ShouldEqual, 0) + err = reconfEncoder1.(encoder.Encoder).ResetPosition(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, actualEncoder1.zeroCount, test.ShouldEqual, 1) +} + +func TestGetPosition(t *testing.T) { + actualEncoder1 := &mock{Name: testEncoderName} + reconfEncoder1, err := encoder.WrapWithReconfigurable(actualEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) + pos1, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, pos1, test.ShouldResemble, position) + test.That(t, actualEncoder1.posCount, test.ShouldEqual, 1) +} + +func TestClose(t *testing.T) { + actualEncoder1 := &mock{Name: testEncoderName} + reconfEncoder1, err := encoder.WrapWithReconfigurable(actualEncoder1, resource.Name{}) + test.That(t, err, test.ShouldBeNil) + + test.That(t, actualEncoder1.reconfCount, test.ShouldEqual, 0) + test.That(t, utils.TryClose(context.Background(), reconfEncoder1), test.ShouldBeNil) + test.That(t, actualEncoder1.reconfCount, test.ShouldEqual, 1) +} + +var ( + position = 5.5 + features = map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: true, + } +) + +type mock struct { + Name string + + zeroCount int + posCount int + featuresCount int + reconfCount int + extra map[string]interface{} +} + +func (m *mock) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + m.zeroCount++ + return nil +} + +func (m *mock) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + m.posCount++ + m.extra = extra + return position, nil +} + +func (m *mock) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + m.featuresCount++ + m.extra = extra + return features, nil +} + +func (m *mock) Close() { m.reconfCount++ } + +func (m *mock) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) { + return cmd, nil +} + +type mockLocal struct { + mock + Name string +} diff --git a/components/encoder/server.go b/components/encoder/server.go new file mode 100644 index 00000000000..0b2126acf84 --- /dev/null +++ b/components/encoder/server.go @@ -0,0 +1,96 @@ +package encoder + +import ( + "context" + + "github.com/pkg/errors" + commonpb "go.viam.com/api/common/v1" + pb "go.viam.com/api/component/encoder/v1" + + "go.viam.com/rdk/protoutils" + "go.viam.com/rdk/subtype" +) + +type subtypeServer struct { + pb.UnimplementedEncoderServiceServer + s subtype.Service +} + +// NewServer constructs an Encoder gRPC service subtypeServer. +func NewServer(s subtype.Service) pb.EncoderServiceServer { + return &subtypeServer{s: s} +} + +// getEncoder returns the specified encoder or nil. +func (s *subtypeServer) getEncoder(name string) (Encoder, error) { + resource := s.s.Resource(name) + if resource == nil { + return nil, errors.Errorf("no Encoder with name (%s)", name) + } + enc, ok := resource.(Encoder) + if !ok { + return nil, errors.Errorf("resource with name (%s) is not an Encoder", name) + } + return enc, nil +} + +// GetPosition reports the position of the encoder. +func (s *subtypeServer) GetPosition( + ctx context.Context, + req *pb.GetPositionRequest, +) (*pb.GetPositionResponse, error) { + enc, err := s.getEncoder(req.Name) + if err != nil { + return nil, err + } + position, err := enc.GetPosition(ctx, req.GetPositionType().Enum(), req.Extra.AsMap()) + if err != nil { + return nil, err + } + return &pb.GetPositionResponse{ + Value: float32(position), + }, nil +} + +// ResetPosition sets the current position of the motor specified by the request +// (adjusted by a given offset) to be its new zero position. +func (s *subtypeServer) ResetPosition( + ctx context.Context, + req *pb.ResetPositionRequest, +) (*pb.ResetPositionResponse, error) { + encName := req.GetName() + enc, err := s.getEncoder(encName) + if err != nil { + return nil, errors.Errorf("no encoder (%s) found", encName) + } + + return &pb.ResetPositionResponse{}, enc.ResetPosition(ctx, req.Extra.AsMap()) +} + +// GetProperties returns a message of booleans indicating which optional features the robot's encoder supports. +func (s *subtypeServer) GetProperties( + ctx context.Context, + req *pb.GetPropertiesRequest, +) (*pb.GetPropertiesResponse, error) { + encoderName := req.GetName() + enc, err := s.getEncoder(encoderName) + if err != nil { + return nil, errors.Errorf("no motor (%s) found", encoderName) + } + features, err := enc.GetProperties(ctx, req.Extra.AsMap()) + if err != nil { + return nil, err + } + return FeatureMapToProtoResponse(features) +} + +// DoCommand receives arbitrary commands. +func (s *subtypeServer) DoCommand(ctx context.Context, + req *commonpb.DoCommandRequest, +) (*commonpb.DoCommandResponse, error) { + enc, err := s.getEncoder(req.GetName()) + if err != nil { + return nil, err + } + return protoutils.DoFromResourceServer(ctx, enc, req) +} diff --git a/components/encoder/server_test.go b/components/encoder/server_test.go new file mode 100644 index 00000000000..e8b2dc10594 --- /dev/null +++ b/components/encoder/server_test.go @@ -0,0 +1,108 @@ +package encoder_test + +import ( + "context" + "errors" + "testing" + + pb "go.viam.com/api/component/encoder/v1" + "go.viam.com/test" + "go.viam.com/utils/protoutils" + + "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/resource" + "go.viam.com/rdk/subtype" + "go.viam.com/rdk/testutils/inject" +) + +func newServer() (pb.EncoderServiceServer, *inject.Encoder, *inject.Encoder, error) { + injectEncoder1 := &inject.Encoder{} + injectEncoder2 := &inject.Encoder{} + + resourceMap := map[resource.Name]interface{}{ + encoder.Named(testEncoderName): injectEncoder1, + encoder.Named(failEncoderName): injectEncoder2, + encoder.Named(fakeEncoderName): "not a encoder", + } + + injectSvc, err := subtype.New(resourceMap) + if err != nil { + return nil, nil, nil, err + } + return encoder.NewServer(injectSvc), injectEncoder1, injectEncoder2, nil +} + +func TestServerGetPosition(t *testing.T) { + encoderServer, workingEncoder, failingEncoder, _ := newServer() + + // fails on a bad encoder + req := pb.GetPositionRequest{Name: fakeEncoderName} + resp, err := encoderServer.GetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldBeNil) + test.That(t, err, test.ShouldNotBeNil) + + failingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + return 0, errors.New("position unavailable") + } + req = pb.GetPositionRequest{Name: failEncoderName} + resp, err = encoderServer.GetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldBeNil) + test.That(t, err, test.ShouldNotBeNil) + + workingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + return 42.0, nil + } + req = pb.GetPositionRequest{Name: testEncoderName} + resp, err = encoderServer.GetPosition(context.Background(), &req) + test.That(t, float64(resp.Value), test.ShouldEqual, 42.0) + test.That(t, err, test.ShouldBeNil) +} + +func TestServerResetPosition(t *testing.T) { + encoderServer, workingEncoder, failingEncoder, _ := newServer() + + // fails on a bad encoder + req := pb.ResetPositionRequest{Name: fakeEncoderName} + resp, err := encoderServer.ResetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldBeNil) + test.That(t, err, test.ShouldNotBeNil) + + failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { + return errors.New("set to zero failed") + } + req = pb.ResetPositionRequest{Name: failEncoderName, Offset: 1.1} + resp, err = encoderServer.ResetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldNotBeNil) + test.That(t, err, test.ShouldNotBeNil) + + workingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { + return nil + } + req = pb.ResetPositionRequest{Name: testEncoderName, Offset: 1.1} + resp, err = encoderServer.ResetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldNotBeNil) + test.That(t, err, test.ShouldBeNil) +} + +func TestServerExtraParams(t *testing.T) { + encoderServer, workingEncoder, _, _ := newServer() + + var actualExtra map[string]interface{} + workingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { + actualExtra = extra + return nil + } + + expectedExtra := map[string]interface{}{"foo": "bar", "baz": []interface{}{1., 2., 3.}} + + ext, err := protoutils.StructToStructPb(expectedExtra) + test.That(t, err, test.ShouldBeNil) + + req := pb.ResetPositionRequest{Name: testEncoderName, Offset: 1.1, Extra: ext} + resp, err := encoderServer.ResetPosition(context.Background(), &req) + test.That(t, resp, test.ShouldNotBeNil) + test.That(t, err, test.ShouldBeNil) + + test.That(t, actualExtra["foo"], test.ShouldEqual, expectedExtra["foo"]) + test.That(t, actualExtra["baz"], test.ShouldResemble, expectedExtra["baz"]) +} diff --git a/testutils/inject/encoder.go b/testutils/inject/encoder.go new file mode 100644 index 00000000000..401d49dd081 --- /dev/null +++ b/testutils/inject/encoder.go @@ -0,0 +1,50 @@ +package inject + +import ( + "context" + + pb "go.viam.com/api/component/encoder/v1" + + "go.viam.com/rdk/components/encoder" +) + +// Encoder is an injected encoder. +type Encoder struct { + encoder.Encoder + DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) + ResetPositionFunc func(ctx context.Context, extra map[string]interface{}) error + GetPositionFunc func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) + GetPropertiesFunc func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) +} + +// ResetPosition calls the injected Zero or the real version. +func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error { + if e.ResetPositionFunc == nil { + return e.Encoder.ResetPosition(ctx, extra) + } + return e.ResetPositionFunc(ctx, extra) +} + +// GetPosition calls the injected Position or the real version. +func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + if e.GetPositionFunc == nil { + return e.Encoder.GetPosition(ctx, positionType, extra) + } + return e.GetPositionFunc(ctx, positionType, extra) +} + +// GetProperties calls the injected Properties or the real version. +func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + if e.GetPropertiesFunc == nil { + return e.Encoder.GetProperties(ctx, extra) + } + return e.GetPropertiesFunc(ctx, extra) +} + +// DoCommand calls the injected DoCommand or the real version. +func (e *Encoder) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) { + if e.DoFunc == nil { + return e.Encoder.DoCommand(ctx, cmd) + } + return e.DoFunc(ctx, cmd) +} From aa0f5d1b29b9e534305f026f33b03ab2a47d93c4 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Fri, 7 Apr 2023 11:57:45 -0400 Subject: [PATCH 06/10] add required PositionType to GetPositionResponse --- components/board/arduino/encoder.go | 17 ++++--- components/encoder/AMS/ams_as5048.go | 7 +-- components/encoder/client.go | 18 ++++--- components/encoder/client_test.go | 35 ++++++++++--- components/encoder/collectors.go | 2 +- components/encoder/encoder.go | 4 +- components/encoder/encoder_test.go | 22 +++++--- components/encoder/fake/encoder.go | 17 ++++--- components/encoder/fake/encoder_test.go | 30 +++++++++-- .../incremental/incremental_encoder.go | 23 +++++++-- .../encoder/incremental/incremental_test.go | 12 +++-- components/encoder/server.go | 11 ++-- components/encoder/server_test.go | 51 ++++++++++++++++--- components/encoder/single/single_encoder.go | 22 ++++++-- .../encoder/single/single_encoder_test.go | 20 ++++---- components/motor/fake/motor.go | 2 +- components/motor/gpio/motor_encoder.go | 8 +-- components/motor/gpio/motor_encoder_test.go | 24 ++++----- testutils/inject/encoder.go | 8 ++- 19 files changed, 235 insertions(+), 98 deletions(-) diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index 33b10734cae..2e8cb0061cd 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -77,6 +77,7 @@ type Encoder struct { A, B string name string + positionType pb.PositionType generic.Unimplemented } @@ -104,22 +105,26 @@ func (cfg *EncoderConfig) Validate(path string) ([]string, error) { } // GetPosition returns number of ticks since last zeroing. -func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees") - return 0, err + err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + return 0, *positionType, err } res, err := e.board.runCommand("motor-position " + e.name) if err != nil { - return 0, err + return 0, e.positionType, err } ticks, err := strconv.ParseInt(res, 10, 64) if err != nil { - return 0, fmt.Errorf("couldn't parse # ticks (%s) : %w", res, err) + return 0, e.positionType, fmt.Errorf("couldn't parse # ticks (%s) : %w", res, err) } - return float64(ticks), nil + return float64(ticks), e.positionType, nil } // ResetPosition sets the current position of the motor (adjusted by a given offset) diff --git a/components/encoder/AMS/ams_as5048.go b/components/encoder/AMS/ams_as5048.go index adeb3ce1b9e..f8e5f161383 100644 --- a/components/encoder/AMS/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -132,6 +132,7 @@ type Encoder struct { positionOffset float64 rotations int connectionType string + positionType pb.PositionType i2cBus board.I2C i2cAddr byte cancelCtx context.Context @@ -254,14 +255,14 @@ func (enc *Encoder) updatePosition(ctx context.Context) error { // position measurements by the motor. func (enc *Encoder) GetPosition( ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}, -) (float64, error) { +) (float64, pb.PositionType, error) { enc.mu.RLock() defer enc.mu.RUnlock() if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - return enc.position, nil + return enc.position, enc.positionType, nil } ticks := float64(enc.rotations) + enc.position/360.0 - return ticks, nil + return ticks, enc.positionType, nil } // ResetPosition sets the current position measured by the encoder to be diff --git a/components/encoder/client.go b/components/encoder/client.go index d4dda78dab9..4c4a63ee81a 100644 --- a/components/encoder/client.go +++ b/components/encoder/client.go @@ -31,21 +31,25 @@ func NewClientFromConn(ctx context.Context, conn rpc.ClientConn, name string, lo } // GetPosition returns number of ticks since last zeroing. -func (c *client) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (c *client) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { ext, err := structpb.NewStruct(extra) if err != nil { - return 0, err + return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, err } - req := &pb.GetPositionRequest{Name: c.name, Extra: ext} + req := &pb.GetPositionRequest{Name: c.name, PositionType: positionType, Extra: ext} resp, err := c.client.GetPosition(ctx, req) if err != nil { - return 0, err + return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, err } - return float64(resp.Value), nil + return float64(resp.Value), resp.PositionType, nil } -// ResetPosition sets the current position of the motor (adjusted by a given offset) -// to be its new zero position. +// ResetPosition sets the current position of +// the encoder to be its new zero position. func (c *client) ResetPosition(ctx context.Context, extra map[string]interface{}) error { ext, err := structpb.NewStruct(extra) if err != nil { diff --git a/components/encoder/client_test.go b/components/encoder/client_test.go index 9db9f00b3b4..2a0806ffec0 100644 --- a/components/encoder/client_test.go +++ b/components/encoder/client_test.go @@ -39,16 +39,34 @@ func TestClient(t *testing.T) { actualExtra = extra return nil } - workingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { + workingEncoder.GetPositionFunc = func( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, + ) (float64, pb.PositionType, error) { actualExtra = extra - return 42.0, nil + return 42.0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + } + workingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + actualExtra = extra + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: false, + }, nil } failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { return errors.New("set to zero failed") } - failingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { - return 0, errors.New("position unavailable") + failingEncoder.GetPositionFunc = func( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, + ) (float64, pb.PositionType, error) { + return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") + } + failingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return nil, errors.New("get properties failed") } resourceMap := map[resource.Name]interface{}{ @@ -88,12 +106,13 @@ func TestClient(t *testing.T) { err = workingEncoderClient.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - pos, err := workingEncoderClient.GetPosition( + pos, positionType, err := workingEncoderClient.GetPosition( context.Background(), - pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), + nil, map[string]interface{}{"foo": "bar", "baz": []interface{}{1., 2., 3.}}) test.That(t, err, test.ShouldBeNil) test.That(t, pos, test.ShouldEqual, 42.0) + test.That(t, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_UNSPECIFIED) test.That(t, actualExtra, test.ShouldResemble, map[string]interface{}{"foo": "bar", "baz": []interface{}{1., 2., 3.}}) @@ -110,7 +129,7 @@ func TestClient(t *testing.T) { err = failingEncoderClient.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldNotBeNil) - pos, err := failingEncoderClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := failingEncoderClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldNotBeNil) test.That(t, pos, test.ShouldEqual, 0.0) @@ -122,7 +141,7 @@ func TestClient(t *testing.T) { test.That(t, err, test.ShouldBeNil) workingEncoderDialedClient := encoder.NewClientFromConn(context.Background(), conn, testEncoderName, logger) - pos, err := workingEncoderDialedClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := workingEncoderDialedClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, pos, test.ShouldEqual, 42.0) diff --git a/components/encoder/collectors.go b/components/encoder/collectors.go index 1040ec32fe4..98e510609c0 100644 --- a/components/encoder/collectors.go +++ b/components/encoder/collectors.go @@ -33,7 +33,7 @@ func newTicksCountCollector(resource interface{}, params data.CollectorParams) ( } cFunc := data.CaptureFunc(func(ctx context.Context, _ map[string]*anypb.Any) (interface{}, error) { - v, err := encoder.GetPosition(ctx, nil, nil) + v, _, err := encoder.GetPosition(ctx, nil, nil) if err != nil { return nil, data.FailedToReadErr(params.ComponentName, ticksCount.String(), err) } diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index eb52747bca7..54ea98ee2fc 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -54,7 +54,7 @@ var Subtype = resource.NewSubtype( // A Encoder turns a position into a signal. type Encoder interface { // GetPosition returns number of ticks since last zeroing - GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) + GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, pb.PositionType, error) // ResetPosition sets the current position of the motor to be its new zero position. ResetPosition(ctx context.Context, extra map[string]interface{}) error @@ -123,7 +123,7 @@ func (r *reconfigurableEncoder) GetPosition( ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}, -) (float64, error) { +) (float64, pb.PositionType, error) { r.mu.RLock() defer r.mu.RUnlock() return r.actual.GetPosition(ctx, positionType, extra) diff --git a/components/encoder/encoder_test.go b/components/encoder/encoder_test.go index b1c842c4e3b..dc073c5e709 100644 --- a/components/encoder/encoder_test.go +++ b/components/encoder/encoder_test.go @@ -72,7 +72,7 @@ func TestFromDependencies(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, res, test.ShouldNotBeNil) - result, err := res.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + result, _, err := res.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, result, test.ShouldResemble, position) @@ -92,7 +92,7 @@ func TestFromRobot(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, res, test.ShouldNotBeNil) - result, err := res.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + result, _, err := res.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, result, test.ShouldResemble, position) @@ -189,7 +189,7 @@ func TestReconfigurableEncoder(t *testing.T) { test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) test.That(t, actualEncoder2.posCount, test.ShouldEqual, 0) - result, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + result, _, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, result, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) @@ -222,10 +222,16 @@ func TestGetPosition(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) - pos1, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos1, positionType, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, pos1, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 1) + test.That(t, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_UNSPECIFIED) + + props, err := reconfEncoder1.(encoder.Encoder).GetProperties(context.Background(), nil) + test.That(t, err, test.ShouldBeNil) + test.That(t, props[encoder.TicksCountSupported], test.ShouldBeTrue) + test.That(t, props[encoder.AngleDegreesSupported], test.ShouldBeTrue) } func TestClose(t *testing.T) { @@ -261,10 +267,14 @@ func (m *mock) ResetPosition(ctx context.Context, extra map[string]interface{}) return nil } -func (m *mock) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (m *mock) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { m.posCount++ m.extra = extra - return position, nil + return position, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil } func (m *mock) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 837427cebe9..8cfac00d576 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -60,6 +60,7 @@ func (cfg *AttrConfig) Validate(path string) error { type Encoder struct { mu sync.Mutex position int64 + positionType pb.PositionType speed float64 // ticks per minute updateRate int64 // update position in start every updateRate ms activeBackgroundWorkers sync.WaitGroup @@ -68,14 +69,18 @@ type Encoder struct { } // GetPosition returns the current position in terms of ticks. -func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees") - return 0, err + err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + return 0, *positionType, err } e.mu.Lock() defer e.mu.Unlock() - return float64(e.position), nil + return float64(e.position), e.positionType, nil } // Start starts a background thread to run the encoder. @@ -118,8 +123,8 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{ // GetProperties returns a list of all the position types that are supported by a given encoder. func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { return map[encoder.Feature]bool{ - encoder.TicksCountSupported: true, - encoder.AngleDegreesSupported: true, + encoder.TicksCountSupported: false, + encoder.AngleDegreesSupported: false, }, nil } diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index 5dd5180f174..9836ef3aa26 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -4,8 +4,11 @@ import ( "context" "testing" + pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" + + "go.viam.com/rdk/components/encoder" ) func TestEncoder(t *testing.T) { @@ -15,14 +18,14 @@ func TestEncoder(t *testing.T) { // Get and set position t.Run("get and set position", func(t *testing.T) { - pos, err := e.GetPosition(ctx, nil, nil) + pos, _, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) err = e.SetPosition(ctx, 1) test.That(t, err, test.ShouldBeNil) - pos, err = e.GetPosition(ctx, nil, nil) + pos, _, err = e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 1) test.That(t, err, test.ShouldBeNil) }) @@ -32,11 +35,30 @@ func TestEncoder(t *testing.T) { err := e.ResetPosition(ctx, nil) test.That(t, err, test.ShouldBeNil) - pos, err := e.GetPosition(ctx, nil, nil) + pos, _, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) }) + t.Run("specify a type", func(t *testing.T) { + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + ticks, positionType, err := e.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, ticks, test.ShouldEqual, 0) + test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_UNSPECIFIED) + }) + }) + t.Run("get properties", func(t *testing.T) { + testutils.WaitForAssertion(t, func(tb testing.TB) { + tb.Helper() + props, err := e.GetProperties(ctx, nil) + test.That(tb, err, test.ShouldBeNil) + test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeFalse) + test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse) + }) + }) + // Set Speed t.Run("set speed", func(t *testing.T) { err := e.SetSpeed(ctx, 1) @@ -61,7 +83,7 @@ func TestEncoder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - pos, err := e.GetPosition(ctx, nil, nil) + pos, _, err := e.GetPosition(ctx, nil, nil) test.That(tb, pos, test.ShouldBeGreaterThan, 0) test.That(tb, err, test.ShouldBeNil) }) diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index adc4a1d1798..ec81dbc8a27 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -51,6 +51,7 @@ type Encoder struct { pRaw int64 pState int64 + positionType pb.PositionType logger golog.Logger CancelCtx context.Context cancelFunc func() @@ -98,7 +99,15 @@ func NewIncrementalEncoder( logger golog.Logger, ) (encoder.Encoder, error) { cancelCtx, cancelFunc := context.WithCancel(ctx) - e := &Encoder{logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, pRaw: 0, pState: 0} + e := &Encoder{ + logger: logger, + CancelCtx: cancelCtx, + cancelFunc: cancelFunc, + position: 0, + positionType: pb.PositionType_POSITION_TYPE_TICKS_COUNT, + pRaw: 0, + pState: 0, + } if cfg, ok := cfg.ConvertedAttributes.(*AttrConfig); ok { board, err := board.FromDependencies(deps, cfg.BoardName) if err != nil { @@ -234,13 +243,17 @@ func (e *Encoder) Start(ctx context.Context) { } // GetPosition returns number of ticks since last zeroing. -func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees") - return 0, err + err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + return 0, *positionType, err } res := atomic.LoadInt64(&e.position) - return float64(res), nil + return float64(res), e.positionType, nil } // ResetPosition sets the current position of the motor (adjusted by a given offset) diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go index e219720d8e0..4020504abf3 100644 --- a/components/encoder/incremental/incremental_test.go +++ b/components/encoder/incremental/incremental_test.go @@ -78,7 +78,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -97,7 +97,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, -1) }) @@ -112,7 +112,7 @@ func TestEnconder(t *testing.T) { // reset position to 0 err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -125,9 +125,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 0) + test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_TICKS_COUNT) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -138,9 +139,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) + test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_ANGLE_DEGREES) }) }) diff --git a/components/encoder/server.go b/components/encoder/server.go index 0b2126acf84..324944adae3 100644 --- a/components/encoder/server.go +++ b/components/encoder/server.go @@ -43,17 +43,18 @@ func (s *subtypeServer) GetPosition( if err != nil { return nil, err } - position, err := enc.GetPosition(ctx, req.GetPositionType().Enum(), req.Extra.AsMap()) + position, positionType, err := enc.GetPosition(ctx, req.PositionType, req.Extra.AsMap()) if err != nil { return nil, err } return &pb.GetPositionResponse{ - Value: float32(position), + Value: float32(position), + PositionType: positionType, }, nil } -// ResetPosition sets the current position of the motor specified by the request -// (adjusted by a given offset) to be its new zero position. +// ResetPosition sets the current position of the encoder +// specified by the request to be its new zero position. func (s *subtypeServer) ResetPosition( ctx context.Context, req *pb.ResetPositionRequest, @@ -75,7 +76,7 @@ func (s *subtypeServer) GetProperties( encoderName := req.GetName() enc, err := s.getEncoder(encoderName) if err != nil { - return nil, errors.Errorf("no motor (%s) found", encoderName) + return nil, errors.Errorf("no encoder (%s) found", encoderName) } features, err := enc.GetProperties(ctx, req.Extra.AsMap()) if err != nil { diff --git a/components/encoder/server_test.go b/components/encoder/server_test.go index e8b2dc10594..808929f910d 100644 --- a/components/encoder/server_test.go +++ b/components/encoder/server_test.go @@ -41,16 +41,24 @@ func TestServerGetPosition(t *testing.T) { test.That(t, resp, test.ShouldBeNil) test.That(t, err, test.ShouldNotBeNil) - failingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { - return 0, errors.New("position unavailable") + failingEncoder.GetPositionFunc = func( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, + ) (float64, pb.PositionType, error) { + return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") } req = pb.GetPositionRequest{Name: failEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) test.That(t, resp, test.ShouldBeNil) test.That(t, err, test.ShouldNotBeNil) - workingEncoder.GetPositionFunc = func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { - return 42.0, nil + workingEncoder.GetPositionFunc = func( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, + ) (float64, pb.PositionType, error) { + return 42.0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil } req = pb.GetPositionRequest{Name: testEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) @@ -70,7 +78,7 @@ func TestServerResetPosition(t *testing.T) { failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { return errors.New("set to zero failed") } - req = pb.ResetPositionRequest{Name: failEncoderName, Offset: 1.1} + req = pb.ResetPositionRequest{Name: failEncoderName} resp, err = encoderServer.ResetPosition(context.Background(), &req) test.That(t, resp, test.ShouldNotBeNil) test.That(t, err, test.ShouldNotBeNil) @@ -78,12 +86,41 @@ func TestServerResetPosition(t *testing.T) { workingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error { return nil } - req = pb.ResetPositionRequest{Name: testEncoderName, Offset: 1.1} + req = pb.ResetPositionRequest{Name: testEncoderName} resp, err = encoderServer.ResetPosition(context.Background(), &req) test.That(t, resp, test.ShouldNotBeNil) test.That(t, err, test.ShouldBeNil) } +func TestServerGetProperties(t *testing.T) { + encoderServer, workingEncoder, failingEncoder, _ := newServer() + + // fails on a bad encoder + req := pb.GetPropertiesRequest{Name: fakeEncoderName} + resp, err := encoderServer.GetProperties(context.Background(), &req) + test.That(t, resp, test.ShouldBeNil) + test.That(t, err, test.ShouldNotBeNil) + + failingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return nil, errors.New("properties not found") + } + req = pb.GetPropertiesRequest{Name: failEncoderName} + resp, err = encoderServer.GetProperties(context.Background(), &req) + test.That(t, resp, test.ShouldBeNil) + test.That(t, err, test.ShouldNotBeNil) + + workingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { + return map[encoder.Feature]bool{ + encoder.TicksCountSupported: true, + encoder.AngleDegreesSupported: false, + }, nil + } + req = pb.GetPropertiesRequest{Name: testEncoderName} + resp, err = encoderServer.GetProperties(context.Background(), &req) + test.That(t, resp, test.ShouldNotBeNil) + test.That(t, err, test.ShouldBeNil) +} + func TestServerExtraParams(t *testing.T) { encoderServer, workingEncoder, _, _ := newServer() @@ -98,7 +135,7 @@ func TestServerExtraParams(t *testing.T) { ext, err := protoutils.StructToStructPb(expectedExtra) test.That(t, err, test.ShouldBeNil) - req := pb.ResetPositionRequest{Name: testEncoderName, Offset: 1.1, Extra: ext} + req := pb.ResetPositionRequest{Name: testEncoderName, Extra: ext} resp, err := encoderServer.ResetPosition(context.Background(), &req) test.That(t, resp, test.ShouldNotBeNil) test.That(t, err, test.ShouldBeNil) diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 247d4e9fe3d..34f1ea77113 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -80,6 +80,7 @@ type Encoder struct { position int64 m DirectionAware + positionType pb.PositionType logger golog.Logger CancelCtx context.Context cancelFunc func() @@ -131,7 +132,14 @@ func NewSingleEncoder( } cancelCtx, cancelFunc := context.WithCancel(ctx) - e := &Encoder{name: rawConfig.Name, logger: logger, CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0} + e := &Encoder{ + name: rawConfig.Name, + logger: logger, + CancelCtx: cancelCtx, + cancelFunc: cancelFunc, + position: 0, + positionType: pb.PositionType_POSITION_TYPE_TICKS_COUNT, + } board, err := board.FromDependencies(deps, cfg.BoardName) if err != nil { @@ -184,13 +192,17 @@ func (e *Encoder) Start(ctx context.Context) { } // GetPosition returns the current position. -func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees") - return 0, err + err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + return 0, *positionType, err } res := atomic.LoadInt64(&e.position) - return float64(res), nil + return float64(res), e.positionType, nil } // ResetPosition sets the current position of the motor (adjusted by a given offset). diff --git a/components/encoder/single/single_encoder_test.go b/components/encoder/single/single_encoder_test.go index 0b80c03a26f..eb4299fe8d0 100644 --- a/components/encoder/single/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -79,7 +79,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -99,7 +99,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, -1) }) @@ -121,7 +121,7 @@ func TestEnconder(t *testing.T) { // by the encoder worker time.Sleep(50 * time.Millisecond) - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -135,7 +135,7 @@ func TestEnconder(t *testing.T) { // reset position to 0 err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) }) @@ -155,7 +155,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -163,7 +163,7 @@ func TestEnconder(t *testing.T) { // reset tick err = enc.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldBeNil) - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(t, err, test.ShouldBeNil) test.That(t, ticks, test.ShouldEqual, 0) @@ -173,7 +173,7 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), nil, nil) + ticks, _, err := enc.GetPosition(context.Background(), nil, nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) }) @@ -192,9 +192,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) + test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_TICKS_COUNT) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -211,9 +212,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) + test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_ANGLE_DEGREES) }) }) t.Run("get properties", func(t *testing.T) { diff --git a/components/motor/fake/motor.go b/components/motor/fake/motor.go index c38325b0b56..50146ea76b9 100644 --- a/components/motor/fake/motor.go +++ b/components/motor/fake/motor.go @@ -151,7 +151,7 @@ func (m *Motor) Position(ctx context.Context, extra map[string]interface{}) (flo return 0, errors.New("encoder is not defined") } - ticks, err := m.Encoder.GetPosition(ctx, nil, extra) + ticks, _, err := m.Encoder.GetPosition(ctx, nil, extra) if err != nil { return 0, err } diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index bb85e63b8c3..dbe9e36bcc3 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -199,7 +199,7 @@ type EncodedMotorState struct { // Position returns the position of the motor. func (m *EncodedMotor) Position(ctx context.Context, extra map[string]interface{}) (float64, error) { - ticks, err := m.encoder.GetPosition(ctx, nil, extra) + ticks, _, err := m.encoder.GetPosition(ctx, nil, extra) if err != nil { return 0, err } @@ -304,7 +304,7 @@ func (m *EncodedMotor) rpmMonitor() { m.startedRPMMonitor = true m.startedRPMMonitorMu.Unlock() - lastPosFl, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) + lastPosFl, _, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) if err != nil { panic(err) } @@ -328,7 +328,7 @@ func (m *EncodedMotor) rpmMonitor() { case <-timer.C: } - pos, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) + pos, _, err := m.encoder.GetPosition(m.cancelCtx, nil, nil) if err != nil { m.logger.Info("error getting encoder position, sleeping then continuing: %w", err) if !utils.SelectContextOrWait(m.cancelCtx, 100*time.Millisecond) { @@ -564,7 +564,7 @@ func (m *EncodedMotor) goForInternal(ctx context.Context, rpm, revolutions float numTicks := int64(revolutions * float64(m.cfg.TicksPerRotation)) - pos, err := m.encoder.GetPosition(ctx, nil, nil) + pos, _, err := m.encoder.GetPosition(ctx, nil, nil) if err != nil { return err } diff --git a/components/motor/gpio/motor_encoder_test.go b/components/motor/gpio/motor_encoder_test.go index c4b1715372e..48f8f5f09d7 100644 --- a/components/motor/gpio/motor_encoder_test.go +++ b/components/motor/gpio/motor_encoder_test.go @@ -356,7 +356,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -367,7 +367,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -378,7 +378,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -389,7 +389,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -416,7 +416,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -427,7 +427,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -438,7 +438,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -449,7 +449,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 4) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 2) test.That(tb, err, test.ShouldBeNil) @@ -460,7 +460,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 3) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -471,7 +471,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 2) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 1) test.That(tb, err, test.ShouldBeNil) @@ -482,7 +482,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 1) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) @@ -493,7 +493,7 @@ func TestMotorEncoderIncremental(t *testing.T) { tb.Helper() pos := encoder.RawPosition() test.That(tb, pos, test.ShouldEqual, 0) - posFl, err := (*encoder).GetPosition(context.Background(), nil, nil) + posFl, _, err := (*encoder).GetPosition(context.Background(), nil, nil) pos = int64(posFl) test.That(tb, pos, test.ShouldEqual, 0) test.That(tb, err, test.ShouldBeNil) diff --git a/testutils/inject/encoder.go b/testutils/inject/encoder.go index 401d49dd081..75327279af0 100644 --- a/testutils/inject/encoder.go +++ b/testutils/inject/encoder.go @@ -13,7 +13,7 @@ type Encoder struct { encoder.Encoder DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) ResetPositionFunc func(ctx context.Context, extra map[string]interface{}) error - GetPositionFunc func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) + GetPositionFunc func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, pb.PositionType, error) GetPropertiesFunc func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) } @@ -26,7 +26,11 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{ } // GetPosition calls the injected Position or the real version. -func (e *Encoder) GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, error) { +func (e *Encoder) GetPosition( + ctx context.Context, + positionType *pb.PositionType, + extra map[string]interface{}, +) (float64, pb.PositionType, error) { if e.GetPositionFunc == nil { return e.Encoder.GetPosition(ctx, positionType, extra) } From ae836c14522e5d8f18f791f248aa70cb79aff524 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Fri, 7 Apr 2023 17:33:31 -0400 Subject: [PATCH 07/10] replace autogenerated PositionType with encoder struct --- components/board/arduino/encoder.go | 12 ++-- components/encoder/AMS/ams_as5048.go | 12 ++-- components/encoder/client.go | 17 +++--- components/encoder/client_test.go | 16 ++--- components/encoder/encoder.go | 59 +++++++++++++++++-- components/encoder/encoder_test.go | 11 ++-- components/encoder/fake/encoder.go | 35 +++++++---- components/encoder/fake/encoder_test.go | 36 ++++++----- components/encoder/features.go | 6 +- .../incremental/incremental_encoder.go | 14 ++--- .../encoder/incremental/incremental_test.go | 9 ++- components/encoder/server.go | 9 ++- components/encoder/server_test.go | 12 ++-- components/encoder/single/single_encoder.go | 14 ++--- .../encoder/single/single_encoder_test.go | 9 ++- components/motor/gpio/motor_encoder.go | 28 ++++++--- testutils/inject/encoder.go | 10 ++-- 17 files changed, 196 insertions(+), 113 deletions(-) diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index 2e8cb0061cd..2456ae8c772 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -7,7 +7,6 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" - pb "go.viam.com/api/component/encoder/v1" rdkutils "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -77,7 +76,7 @@ type Encoder struct { A, B string name string - positionType pb.PositionType + positionType encoder.PositionType generic.Unimplemented } @@ -104,13 +103,14 @@ func (cfg *EncoderConfig) Validate(path string) ([]string, error) { return deps, nil } -// GetPosition returns number of ticks since last zeroing. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (e *Encoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { - if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { +) (float64, encoder.PositionType, error) { + if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } diff --git a/components/encoder/AMS/ams_as5048.go b/components/encoder/AMS/ams_as5048.go index f8e5f161383..b89ec309d14 100644 --- a/components/encoder/AMS/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -10,7 +10,6 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" "go.uber.org/zap" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -132,7 +131,7 @@ type Encoder struct { positionOffset float64 rotations int connectionType string - positionType pb.PositionType + positionType encoder.PositionType i2cBus board.I2C i2cAddr byte cancelCtx context.Context @@ -155,6 +154,7 @@ func newAS5048Encoder( cancelCtx: cancelCtx, cancel: cancel, logger: logger, + positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, } brd, err := board.FromDependencies(deps, attr.BoardName) if err != nil { @@ -254,14 +254,16 @@ func (enc *Encoder) updatePosition(ctx context.Context) error { // motor to 1. Any other value will result in completely incorrect // position measurements by the motor. func (enc *Encoder) GetPosition( - ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { + ctx context.Context, positionType *encoder.PositionType, extra map[string]interface{}, +) (float64, encoder.PositionType, error) { enc.mu.RLock() defer enc.mu.RUnlock() - if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { + enc.positionType = encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES return enc.position, enc.positionType, nil } ticks := float64(enc.rotations) + enc.position/360.0 + enc.positionType = encoder.PositionType_POSITION_TYPE_TICKS_COUNT return ticks, enc.positionType, nil } diff --git a/components/encoder/client.go b/components/encoder/client.go index 4c4a63ee81a..c61632a89e2 100644 --- a/components/encoder/client.go +++ b/components/encoder/client.go @@ -30,22 +30,25 @@ func NewClientFromConn(ctx context.Context, conn rpc.ClientConn, name string, lo } } -// GetPosition returns number of ticks since last zeroing. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (c *client) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { +) (float64, PositionType, error) { ext, err := structpb.NewStruct(extra) if err != nil { - return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, err + return 0, PositionType_POSITION_TYPE_UNSPECIFIED, err } - req := &pb.GetPositionRequest{Name: c.name, PositionType: positionType, Extra: ext} + posType, err := EncoderToProtoPositionType(positionType) + req := &pb.GetPositionRequest{Name: c.name, PositionType: &posType, Extra: ext} resp, err := c.client.GetPosition(ctx, req) if err != nil { - return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, err + return 0, PositionType_POSITION_TYPE_UNSPECIFIED, err } - return float64(resp.Value), resp.PositionType, nil + posType1, err := ProtoToEncoderPositionType(&resp.PositionType) + return float64(resp.Value), posType1, nil } // ResetPosition sets the current position of diff --git a/components/encoder/client_test.go b/components/encoder/client_test.go index 2a0806ffec0..f7a5da9233c 100644 --- a/components/encoder/client_test.go +++ b/components/encoder/client_test.go @@ -41,11 +41,11 @@ func TestClient(t *testing.T) { } workingEncoder.GetPositionFunc = func( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, - ) (float64, pb.PositionType, error) { + ) (float64, encoder.PositionType, error) { actualExtra = extra - return 42.0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return 42.0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil } workingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { actualExtra = extra @@ -60,10 +60,10 @@ func TestClient(t *testing.T) { } failingEncoder.GetPositionFunc = func( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, - ) (float64, pb.PositionType, error) { - return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") + ) (float64, encoder.PositionType, error) { + return 0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") } failingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { return nil, errors.New("get properties failed") @@ -129,7 +129,7 @@ func TestClient(t *testing.T) { err = failingEncoderClient.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldNotBeNil) - pos, _, err := failingEncoderClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := failingEncoderClient.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldNotBeNil) test.That(t, pos, test.ShouldEqual, 0.0) @@ -141,7 +141,7 @@ func TestClient(t *testing.T) { test.That(t, err, test.ShouldBeNil) workingEncoderDialedClient := encoder.NewClientFromConn(context.Background(), conn, testEncoderName, logger) - pos, _, err := workingEncoderDialedClient.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := workingEncoderDialedClient.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, pos, test.ShouldEqual, 42.0) diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index 54ea98ee2fc..61d51c89fbd 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -51,10 +51,31 @@ var Subtype = resource.NewSubtype( SubtypeName, ) +// PositionType is an enum representing the encoders position type. +type PositionType int32 + +// PositionReporting represesnts the feature of a motor being +// able to report its own position. +const ( + PositionType_POSITION_TYPE_UNSPECIFIED PositionType = 0 + // Return type for relative encoders that report + // how far they've gone from a start position + PositionType_POSITION_TYPE_TICKS_COUNT PositionType = 1 + // Return type for absolute encoders that report + // their position in degrees along the radial axis + PositionType_POSITION_TYPE_ANGLE_DEGREES PositionType = 2 +) + +func (x PositionType) Enum() *PositionType { + p := new(PositionType) + *p = x + return p +} + // A Encoder turns a position into a signal. type Encoder interface { - // GetPosition returns number of ticks since last zeroing - GetPosition(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, pb.PositionType, error) + // GetPosition returns the current position in terms of ticks or degrees, and whether it is a relative or absolute position. + GetPosition(ctx context.Context, positionType *PositionType, extra map[string]interface{}) (float64, PositionType, error) // ResetPosition sets the current position of the motor to be its new zero position. ResetPosition(ctx context.Context, extra map[string]interface{}) error @@ -121,9 +142,9 @@ func (r *reconfigurableEncoder) DoCommand(ctx context.Context, cmd map[string]in func (r *reconfigurableEncoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { +) (float64, PositionType, error) { r.mu.RLock() defer r.mu.RUnlock() return r.actual.GetPosition(ctx, positionType, extra) @@ -177,3 +198,33 @@ func WrapWithReconfigurable(r interface{}, name resource.Name) (resource.Reconfi } return &reconfigurableEncoder{name: name, actual: m}, nil } + +// ProtoToEncoderPositionType takes a GetPositionResponse and returns +// an equivalent PositionType-to-int map. +func ProtoToEncoderPositionType(positionType *pb.PositionType) (PositionType, error) { + if positionType == nil { + return PositionType_POSITION_TYPE_UNSPECIFIED, nil + } + if *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { + return PositionType_POSITION_TYPE_ANGLE_DEGREES, nil + } + if *positionType == pb.PositionType_POSITION_TYPE_TICKS_COUNT { + return PositionType_POSITION_TYPE_TICKS_COUNT, nil + } + return PositionType_POSITION_TYPE_UNSPECIFIED, nil +} + +// EncoderToProtoPositionType takes a map of PositionType-to-int (indicating +// the PositionType) and converts it to a GetPositionResponse. +func EncoderToProtoPositionType(positionType *PositionType) (pb.PositionType, error) { + if positionType == nil { + return pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + } + if *positionType == PositionType_POSITION_TYPE_ANGLE_DEGREES { + return pb.PositionType_POSITION_TYPE_ANGLE_DEGREES, nil + } + if *positionType == PositionType_POSITION_TYPE_TICKS_COUNT { + return pb.PositionType_POSITION_TYPE_TICKS_COUNT, nil + } + return pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil +} diff --git a/components/encoder/encoder_test.go b/components/encoder/encoder_test.go index dc073c5e709..38512c94a9b 100644 --- a/components/encoder/encoder_test.go +++ b/components/encoder/encoder_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils" @@ -189,7 +188,7 @@ func TestReconfigurableEncoder(t *testing.T) { test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) test.That(t, actualEncoder2.posCount, test.ShouldEqual, 0) - result, _, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + result, _, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, result, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) @@ -226,7 +225,7 @@ func TestGetPosition(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, pos1, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 1) - test.That(t, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_UNSPECIFIED) + test.That(t, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_UNSPECIFIED) props, err := reconfEncoder1.(encoder.Encoder).GetProperties(context.Background(), nil) test.That(t, err, test.ShouldBeNil) @@ -269,12 +268,12 @@ func (m *mock) ResetPosition(ctx context.Context, extra map[string]interface{}) func (m *mock) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { +) (float64, encoder.PositionType, error) { m.posCount++ m.extra = extra - return position, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return position, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil } func (m *mock) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 8cfac00d576..c8d1a25882f 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -8,7 +8,6 @@ import ( "time" "github.com/edaniels/golog" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/encoder" @@ -28,11 +27,7 @@ func init() { cfg config.Component, logger golog.Logger, ) (interface{}, error) { - e := &Encoder{} - e.updateRate = cfg.ConvertedAttributes.(*AttrConfig).UpdateRate - - e.Start(ctx) - return e, nil + return NewFakeEncoder(ctx, cfg) }, } registry.RegisterComponent(encoder.Subtype, fakeModel, _encoder) @@ -46,6 +41,21 @@ func init() { }, &AttrConfig{}) } +// NewFakeEncoder creates a new Encoder. +func NewFakeEncoder( + ctx context.Context, + cfg config.Component, +) (encoder.Encoder, error) { + e := &Encoder{ + position: 0, + positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, + } + e.updateRate = cfg.ConvertedAttributes.(*AttrConfig).UpdateRate + + e.Start(ctx) + return e, nil +} + // AttrConfig describes the configuration of a fake encoder. type AttrConfig struct { UpdateRate int64 `json:"update_rate_msec,omitempty"` @@ -60,7 +70,7 @@ func (cfg *AttrConfig) Validate(path string) error { type Encoder struct { mu sync.Mutex position int64 - positionType pb.PositionType + positionType encoder.PositionType speed float64 // ticks per minute updateRate int64 // update position in start every updateRate ms activeBackgroundWorkers sync.WaitGroup @@ -68,13 +78,14 @@ type Encoder struct { generic.Unimplemented } -// GetPosition returns the current position in terms of ticks. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (e *Encoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { - if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { +) (float64, encoder.PositionType, error) { + if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } @@ -123,7 +134,7 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{ // GetProperties returns a list of all the position types that are supported by a given encoder. func (e *Encoder) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { return map[encoder.Feature]bool{ - encoder.TicksCountSupported: false, + encoder.TicksCountSupported: true, encoder.AngleDegreesSupported: false, }, nil } diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index 9836ef3aa26..dd341ecd152 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -4,25 +4,31 @@ import ( "context" "testing" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" "go.viam.com/rdk/components/encoder" + "go.viam.com/rdk/config" ) func TestEncoder(t *testing.T) { ctx := context.Background() - - e := &Encoder{} + ic := AttrConfig{ + UpdateRate: 100, + } + cfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} + e, _ := NewFakeEncoder(ctx, cfg) // Get and set position t.Run("get and set position", func(t *testing.T) { - pos, _, err := e.GetPosition(ctx, nil, nil) + pos, positionType, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) + test.That(t, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) + + e1 := e.(*Encoder) - err = e.SetPosition(ctx, 1) + err = e1.SetPosition(ctx, 1) test.That(t, err, test.ShouldBeNil) pos, _, err = e.GetPosition(ctx, nil, nil) @@ -43,10 +49,10 @@ func TestEncoder(t *testing.T) { t.Run("specify a type", func(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := e.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := e.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_UNSPECIFIED) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) }) }) t.Run("get properties", func(t *testing.T) { @@ -54,31 +60,33 @@ func TestEncoder(t *testing.T) { tb.Helper() props, err := e.GetProperties(ctx, nil) test.That(tb, err, test.ShouldBeNil) - test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeFalse) + test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue) test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse) }) }) // Set Speed t.Run("set speed", func(t *testing.T) { - err := e.SetSpeed(ctx, 1) + e1 := e.(*Encoder) + err := e1.SetSpeed(ctx, 1) test.That(t, err, test.ShouldBeNil) - test.That(t, e.speed, test.ShouldEqual, 1) + test.That(t, e1.speed, test.ShouldEqual, 1) }) // Start with default update rate t.Run("start default update rate", func(t *testing.T) { - err := e.SetSpeed(ctx, 0) + e1 := e.(*Encoder) + err := e1.SetSpeed(ctx, 0) test.That(t, err, test.ShouldBeNil) - e.Start(ctx) + // e1.Start(ctx) testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - test.That(t, e.updateRate, test.ShouldEqual, 100) + test.That(t, e1.updateRate, test.ShouldEqual, 100) }) - err = e.SetSpeed(ctx, 600) + err = e1.SetSpeed(ctx, 600) test.That(t, err, test.ShouldBeNil) testutils.WaitForAssertion(t, func(tb testing.TB) { diff --git a/components/encoder/features.go b/components/encoder/features.go index c6982e02092..3a08019a73d 100644 --- a/components/encoder/features.go +++ b/components/encoder/features.go @@ -5,11 +5,11 @@ import ( pb "go.viam.com/api/component/encoder/v1" ) -// Feature is an enum representing an optional motor feature. +// Feature is an enum representing an optional encoder feature. type Feature string -// PositionReporting represesnts the feature of a motor being -// able to report its own position. +// TicksCountSupported and AngleDegreesSupported represesnts the feature +// of an encoder being able to report ticks and/or degrees, respectively const ( TicksCountSupported Feature = "Ticks" AngleDegreesSupported Feature = "Degrees" diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index ec81dbc8a27..766388481ba 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -8,7 +8,6 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -51,7 +50,7 @@ type Encoder struct { pRaw int64 pState int64 - positionType pb.PositionType + positionType encoder.PositionType logger golog.Logger CancelCtx context.Context cancelFunc func() @@ -104,7 +103,7 @@ func NewIncrementalEncoder( CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, - positionType: pb.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, pRaw: 0, pState: 0, } @@ -242,13 +241,14 @@ func (e *Encoder) Start(ctx context.Context) { }, e.activeBackgroundWorkers.Done) } -// GetPosition returns number of ticks since last zeroing. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (e *Encoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { - if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { +) (float64, encoder.PositionType, error) { + if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go index 4020504abf3..0c13a199fa7 100644 --- a/components/encoder/incremental/incremental_test.go +++ b/components/encoder/incremental/incremental_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/edaniels/golog" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" @@ -125,10 +124,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -139,10 +138,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_ANGLE_DEGREES) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES) }) }) diff --git a/components/encoder/server.go b/components/encoder/server.go index 324944adae3..8ff9478462d 100644 --- a/components/encoder/server.go +++ b/components/encoder/server.go @@ -34,7 +34,8 @@ func (s *subtypeServer) getEncoder(name string) (Encoder, error) { return enc, nil } -// GetPosition reports the position of the encoder. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (s *subtypeServer) GetPosition( ctx context.Context, req *pb.GetPositionRequest, @@ -43,13 +44,15 @@ func (s *subtypeServer) GetPosition( if err != nil { return nil, err } - position, positionType, err := enc.GetPosition(ctx, req.PositionType, req.Extra.AsMap()) + posType, err := ProtoToEncoderPositionType(req.PositionType) + position, positionType, err := enc.GetPosition(ctx, &posType, req.Extra.AsMap()) if err != nil { return nil, err } + posType1, err := EncoderToProtoPositionType(&positionType) return &pb.GetPositionResponse{ Value: float32(position), - PositionType: positionType, + PositionType: posType1, }, nil } diff --git a/components/encoder/server_test.go b/components/encoder/server_test.go index 808929f910d..bb80d9c9b19 100644 --- a/components/encoder/server_test.go +++ b/components/encoder/server_test.go @@ -43,10 +43,10 @@ func TestServerGetPosition(t *testing.T) { failingEncoder.GetPositionFunc = func( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, - ) (float64, pb.PositionType, error) { - return 0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") + ) (float64, encoder.PositionType, error) { + return 0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") } req = pb.GetPositionRequest{Name: failEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) @@ -55,10 +55,10 @@ func TestServerGetPosition(t *testing.T) { workingEncoder.GetPositionFunc = func( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, - ) (float64, pb.PositionType, error) { - return 42.0, pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + ) (float64, encoder.PositionType, error) { + return 42.0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil } req = pb.GetPositionRequest{Name: testEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 34f1ea77113..8f92ea77582 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -29,7 +29,6 @@ import ( "github.com/edaniels/golog" "github.com/pkg/errors" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/utils" "go.viam.com/rdk/components/board" @@ -80,7 +79,7 @@ type Encoder struct { position int64 m DirectionAware - positionType pb.PositionType + positionType encoder.PositionType logger golog.Logger CancelCtx context.Context cancelFunc func() @@ -138,7 +137,7 @@ func NewSingleEncoder( CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, - positionType: pb.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, } board, err := board.FromDependencies(deps, cfg.BoardName) @@ -191,13 +190,14 @@ func (e *Encoder) Start(ctx context.Context) { }, e.activeBackgroundWorkers.Done) } -// GetPosition returns the current position. +// GetPosition returns the current position in terms of ticks or +// degrees, and whether it is a relative or absolute position. func (e *Encoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { - if positionType != nil && *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { +) (float64, encoder.PositionType, error) { + if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } diff --git a/components/encoder/single/single_encoder_test.go b/components/encoder/single/single_encoder_test.go index eb4299fe8d0..97fad864b20 100644 --- a/components/encoder/single/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/edaniels/golog" - pb "go.viam.com/api/component/encoder/v1" "go.viam.com/test" "go.viam.com/utils/testutils" @@ -192,10 +191,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) - test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -212,10 +211,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), pb.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, pb.PositionType_POSITION_TYPE_ANGLE_DEGREES) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES) }) }) t.Run("get properties", func(t *testing.T) { diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index dbe9e36bcc3..7981b680242 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -110,16 +110,26 @@ func newEncodedMotor( activeBackgroundWorkers: &sync.WaitGroup{}, cfg: motorConfig, real: localReal, - encoder: realEncoder, - cancelCtx: cancelCtx, - cancel: cancel, - stateMu: &sync.RWMutex{}, - startedRPMMonitorMu: &sync.Mutex{}, - rampRate: motorConfig.RampRate, - maxPowerPct: motorConfig.MaxPowerPct, - logger: logger, - loop: nil, + // encoder: realEncoder, + cancelCtx: cancelCtx, + cancel: cancel, + stateMu: &sync.RWMutex{}, + startedRPMMonitorMu: &sync.Mutex{}, + rampRate: motorConfig.RampRate, + maxPowerPct: motorConfig.MaxPowerPct, + logger: logger, + loop: nil, + } + + props, err := realEncoder.GetProperties(context.Background(), nil) + if err != nil { + return nil, errors.New("cannot get encoder properties") + } + if !props[encoder.TicksCountSupported] { + return nil, + errors.New("cannot create an encoded motor without an encoder that has propery TicksCountSupported") } + em.encoder = realEncoder if len(motorConfig.ControlLoop.Blocks) != 0 { cLoop, err := control.NewLoop(logger, motorConfig.ControlLoop, em) diff --git a/testutils/inject/encoder.go b/testutils/inject/encoder.go index 75327279af0..510f833647f 100644 --- a/testutils/inject/encoder.go +++ b/testutils/inject/encoder.go @@ -3,8 +3,6 @@ package inject import ( "context" - pb "go.viam.com/api/component/encoder/v1" - "go.viam.com/rdk/components/encoder" ) @@ -13,7 +11,7 @@ type Encoder struct { encoder.Encoder DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) ResetPositionFunc func(ctx context.Context, extra map[string]interface{}) error - GetPositionFunc func(ctx context.Context, positionType *pb.PositionType, extra map[string]interface{}) (float64, pb.PositionType, error) + GetPositionFunc func(ctx context.Context, positionType *encoder.PositionType, extra map[string]interface{}) (float64, encoder.PositionType, error) GetPropertiesFunc func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) } @@ -25,12 +23,12 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{ return e.ResetPositionFunc(ctx, extra) } -// GetPosition calls the injected Position or the real version. +// GetPosition calls the injected GetPosition or the real version. func (e *Encoder) GetPosition( ctx context.Context, - positionType *pb.PositionType, + positionType *encoder.PositionType, extra map[string]interface{}, -) (float64, pb.PositionType, error) { +) (float64, encoder.PositionType, error) { if e.GetPositionFunc == nil { return e.Encoder.GetPosition(ctx, positionType, extra) } From 4067e86340279ef4802ce80acce492eec103fb75 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Mon, 10 Apr 2023 10:24:36 -0400 Subject: [PATCH 08/10] add common errors to errors.go --- components/encoder/errors.go | 25 +++++++++++++++++++ components/encoder/fake/encoder.go | 6 ++--- components/encoder/fake/encoder_test.go | 4 +-- .../incremental/incremental_encoder.go | 2 +- components/encoder/single/single_encoder.go | 2 +- components/motor/gpio/motor_encoder.go | 19 +++++++------- 6 files changed, 40 insertions(+), 18 deletions(-) create mode 100644 components/encoder/errors.go diff --git a/components/encoder/errors.go b/components/encoder/errors.go new file mode 100644 index 00000000000..87a9a3309e2 --- /dev/null +++ b/components/encoder/errors.go @@ -0,0 +1,25 @@ +package encoder + +import "github.com/pkg/errors" + +// NewEncoderTypeUnsupportedError returns a standard error for when +// an encoder does not support the given PositionType. +func NewEncoderTypeUnsupportedError(positionType PositionType) error { + if positionType == 1 { + return errors.Errorf( + "Encoder does not support Ticks, use a different PositionType") + } + if positionType == 2 { + return errors.Errorf( + "Encoder does not support Angles, use a different PositionType") + } + return errors.Errorf("Cannot identify PositionType") +} + +func NewEncodedMotorTypeUnsupportedError(props map[Feature]bool) error { + if props[AngleDegreesSupported] { + return errors.Errorf( + "encoder type is Angle Degrees, need an encoder that supports Ticks for an encoded motor") + } + return errors.Errorf("need an encoder that supports Ticks for an encoded motor") +} diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index c8d1a25882f..16039ac8ad1 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -27,7 +27,7 @@ func init() { cfg config.Component, logger golog.Logger, ) (interface{}, error) { - return NewFakeEncoder(ctx, cfg) + return newFakeEncoder(ctx, cfg) }, } registry.RegisterComponent(encoder.Subtype, fakeModel, _encoder) @@ -41,8 +41,8 @@ func init() { }, &AttrConfig{}) } -// NewFakeEncoder creates a new Encoder. -func NewFakeEncoder( +// newFakeEncoder creates a new Encoder. +func newFakeEncoder( ctx context.Context, cfg config.Component, ) (encoder.Encoder, error) { diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index dd341ecd152..b34835c6681 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -17,7 +17,7 @@ func TestEncoder(t *testing.T) { UpdateRate: 100, } cfg := config.Component{Name: "enc1", ConvertedAttributes: &ic} - e, _ := NewFakeEncoder(ctx, cfg) + e, _ := newFakeEncoder(ctx, cfg) // Get and set position t.Run("get and set position", func(t *testing.T) { @@ -79,8 +79,6 @@ func TestEncoder(t *testing.T) { err := e1.SetSpeed(ctx, 0) test.That(t, err, test.ShouldBeNil) - // e1.Start(ctx) - testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() test.That(t, e1.updateRate, test.ShouldEqual, 100) diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index 766388481ba..fbd4378f5fd 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -249,7 +249,7 @@ func (e *Encoder) GetPosition( extra map[string]interface{}, ) (float64, encoder.PositionType, error) { if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + err := encoder.NewEncoderTypeUnsupportedError(*positionType) return 0, *positionType, err } res := atomic.LoadInt64(&e.position) diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 8f92ea77582..27444dbff63 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -198,7 +198,7 @@ func (e *Encoder) GetPosition( extra map[string]interface{}, ) (float64, encoder.PositionType, error) { if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { - err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") + err := encoder.NewEncoderTypeUnsupportedError(*positionType) return 0, *positionType, err } res := atomic.LoadInt64(&e.position) diff --git a/components/motor/gpio/motor_encoder.go b/components/motor/gpio/motor_encoder.go index 7981b680242..bfaf4268299 100644 --- a/components/motor/gpio/motor_encoder.go +++ b/components/motor/gpio/motor_encoder.go @@ -110,15 +110,14 @@ func newEncodedMotor( activeBackgroundWorkers: &sync.WaitGroup{}, cfg: motorConfig, real: localReal, - // encoder: realEncoder, - cancelCtx: cancelCtx, - cancel: cancel, - stateMu: &sync.RWMutex{}, - startedRPMMonitorMu: &sync.Mutex{}, - rampRate: motorConfig.RampRate, - maxPowerPct: motorConfig.MaxPowerPct, - logger: logger, - loop: nil, + cancelCtx: cancelCtx, + cancel: cancel, + stateMu: &sync.RWMutex{}, + startedRPMMonitorMu: &sync.Mutex{}, + rampRate: motorConfig.RampRate, + maxPowerPct: motorConfig.MaxPowerPct, + logger: logger, + loop: nil, } props, err := realEncoder.GetProperties(context.Background(), nil) @@ -127,7 +126,7 @@ func newEncodedMotor( } if !props[encoder.TicksCountSupported] { return nil, - errors.New("cannot create an encoded motor without an encoder that has propery TicksCountSupported") + encoder.NewEncodedMotorTypeUnsupportedError(props) } em.encoder = realEncoder From 836c6ffca8e8672101a40efee3ccd51da66bf635 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Mon, 10 Apr 2023 11:45:43 -0400 Subject: [PATCH 09/10] fix lint issues --- components/board/arduino/encoder.go | 2 +- components/encoder/AMS/ams_as5048.go | 8 ++-- components/encoder/client.go | 8 ++-- components/encoder/client_test.go | 8 ++-- components/encoder/encoder.go | 47 ++++++++++--------- components/encoder/encoder_test.go | 9 ++-- components/encoder/errors.go | 12 +++-- components/encoder/fake/encoder.go | 7 ++- components/encoder/fake/encoder_test.go | 6 +-- components/encoder/features.go | 2 +- .../incremental/incremental_encoder.go | 4 +- .../encoder/incremental/incremental_test.go | 8 ++-- components/encoder/server.go | 4 +- components/encoder/server_test.go | 4 +- components/encoder/single/single_encoder.go | 4 +- .../encoder/single/single_encoder_test.go | 8 ++-- go.mod | 2 +- go.sum | 4 +- testutils/inject/encoder.go | 5 +- 19 files changed, 82 insertions(+), 70 deletions(-) diff --git a/components/board/arduino/encoder.go b/components/board/arduino/encoder.go index 2456ae8c772..b6547a84ede 100644 --- a/components/board/arduino/encoder.go +++ b/components/board/arduino/encoder.go @@ -110,7 +110,7 @@ func (e *Encoder) GetPosition( positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { + if positionType != nil && *positionType == encoder.PositionTypeDEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } diff --git a/components/encoder/AMS/ams_as5048.go b/components/encoder/AMS/ams_as5048.go index b89ec309d14..bb825848115 100644 --- a/components/encoder/AMS/ams_as5048.go +++ b/components/encoder/AMS/ams_as5048.go @@ -154,7 +154,7 @@ func newAS5048Encoder( cancelCtx: cancelCtx, cancel: cancel, logger: logger, - positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionTypeTICKS, } brd, err := board.FromDependencies(deps, attr.BoardName) if err != nil { @@ -258,12 +258,12 @@ func (enc *Encoder) GetPosition( ) (float64, encoder.PositionType, error) { enc.mu.RLock() defer enc.mu.RUnlock() - if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { - enc.positionType = encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES + if positionType != nil && *positionType == encoder.PositionTypeDEGREES { + enc.positionType = encoder.PositionTypeDEGREES return enc.position, enc.positionType, nil } ticks := float64(enc.rotations) + enc.position/360.0 - enc.positionType = encoder.PositionType_POSITION_TYPE_TICKS_COUNT + enc.positionType = encoder.PositionTypeTICKS return ticks, enc.positionType, nil } diff --git a/components/encoder/client.go b/components/encoder/client.go index c61632a89e2..2ea8058df66 100644 --- a/components/encoder/client.go +++ b/components/encoder/client.go @@ -39,15 +39,15 @@ func (c *client) GetPosition( ) (float64, PositionType, error) { ext, err := structpb.NewStruct(extra) if err != nil { - return 0, PositionType_POSITION_TYPE_UNSPECIFIED, err + return 0, PositionTypeUNSPECIFIED, err } - posType, err := EncoderToProtoPositionType(positionType) + posType := ToProtoPositionType(positionType) req := &pb.GetPositionRequest{Name: c.name, PositionType: &posType, Extra: ext} resp, err := c.client.GetPosition(ctx, req) if err != nil { - return 0, PositionType_POSITION_TYPE_UNSPECIFIED, err + return 0, PositionTypeUNSPECIFIED, err } - posType1, err := ProtoToEncoderPositionType(&resp.PositionType) + posType1 := ToEncoderPositionType(&resp.PositionType) return float64(resp.Value), posType1, nil } diff --git a/components/encoder/client_test.go b/components/encoder/client_test.go index f7a5da9233c..f8869d7486d 100644 --- a/components/encoder/client_test.go +++ b/components/encoder/client_test.go @@ -45,7 +45,7 @@ func TestClient(t *testing.T) { extra map[string]interface{}, ) (float64, encoder.PositionType, error) { actualExtra = extra - return 42.0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return 42.0, encoder.PositionTypeUNSPECIFIED, nil } workingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { actualExtra = extra @@ -63,7 +63,7 @@ func TestClient(t *testing.T) { positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - return 0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") + return 0, encoder.PositionTypeUNSPECIFIED, errors.New("position unavailable") } failingEncoder.GetPropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { return nil, errors.New("get properties failed") @@ -129,7 +129,7 @@ func TestClient(t *testing.T) { err = failingEncoderClient.ResetPosition(context.Background(), nil) test.That(t, err, test.ShouldNotBeNil) - pos, _, err := failingEncoderClient.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := failingEncoderClient.GetPosition(context.Background(), encoder.PositionTypeUNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldNotBeNil) test.That(t, pos, test.ShouldEqual, 0.0) @@ -141,7 +141,7 @@ func TestClient(t *testing.T) { test.That(t, err, test.ShouldBeNil) workingEncoderDialedClient := encoder.NewClientFromConn(context.Background(), conn, testEncoderName, logger) - pos, _, err := workingEncoderDialedClient.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + pos, _, err := workingEncoderDialedClient.GetPosition(context.Background(), encoder.PositionTypeUNSPECIFIED.Enum(), nil) test.That(t, err, test.ShouldBeNil) test.That(t, pos, test.ShouldEqual, 42.0) diff --git a/components/encoder/encoder.go b/components/encoder/encoder.go index 61d51c89fbd..72175b69c93 100644 --- a/components/encoder/encoder.go +++ b/components/encoder/encoder.go @@ -54,18 +54,19 @@ var Subtype = resource.NewSubtype( // PositionType is an enum representing the encoders position type. type PositionType int32 -// PositionReporting represesnts the feature of a motor being -// able to report its own position. const ( - PositionType_POSITION_TYPE_UNSPECIFIED PositionType = 0 - // Return type for relative encoders that report - // how far they've gone from a start position - PositionType_POSITION_TYPE_TICKS_COUNT PositionType = 1 - // Return type for absolute encoders that report - // their position in degrees along the radial axis - PositionType_POSITION_TYPE_ANGLE_DEGREES PositionType = 2 + // PositionTypeUNSPECIFIED is the return type for + // when the user has not specified a PositionType. + PositionTypeUNSPECIFIED PositionType = 0 + // PositionTypeTICKS is the return type for relative encoders + // that report how far they've gone from a start position. + PositionTypeTICKS PositionType = 1 + // PositionTypeDEGREES is the return type for absolute encoders + // that report their position in degrees along the radial axis. + PositionTypeDEGREES PositionType = 2 ) +// Enum reveals the value of PositionType. func (x PositionType) Enum() *PositionType { p := new(PositionType) *p = x @@ -199,32 +200,32 @@ func WrapWithReconfigurable(r interface{}, name resource.Name) (resource.Reconfi return &reconfigurableEncoder{name: name, actual: m}, nil } -// ProtoToEncoderPositionType takes a GetPositionResponse and returns +// ToEncoderPositionType takes a GetPositionResponse and returns // an equivalent PositionType-to-int map. -func ProtoToEncoderPositionType(positionType *pb.PositionType) (PositionType, error) { +func ToEncoderPositionType(positionType *pb.PositionType) PositionType { if positionType == nil { - return PositionType_POSITION_TYPE_UNSPECIFIED, nil + return PositionTypeUNSPECIFIED } if *positionType == pb.PositionType_POSITION_TYPE_ANGLE_DEGREES { - return PositionType_POSITION_TYPE_ANGLE_DEGREES, nil + return PositionTypeDEGREES } if *positionType == pb.PositionType_POSITION_TYPE_TICKS_COUNT { - return PositionType_POSITION_TYPE_TICKS_COUNT, nil + return PositionTypeTICKS } - return PositionType_POSITION_TYPE_UNSPECIFIED, nil + return PositionTypeUNSPECIFIED } -// EncoderToProtoPositionType takes a map of PositionType-to-int (indicating +// ToProtoPositionType takes a map of PositionType-to-int (indicating // the PositionType) and converts it to a GetPositionResponse. -func EncoderToProtoPositionType(positionType *PositionType) (pb.PositionType, error) { +func ToProtoPositionType(positionType *PositionType) pb.PositionType { if positionType == nil { - return pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return pb.PositionType_POSITION_TYPE_UNSPECIFIED } - if *positionType == PositionType_POSITION_TYPE_ANGLE_DEGREES { - return pb.PositionType_POSITION_TYPE_ANGLE_DEGREES, nil + if *positionType == PositionTypeDEGREES { + return pb.PositionType_POSITION_TYPE_ANGLE_DEGREES } - if *positionType == PositionType_POSITION_TYPE_TICKS_COUNT { - return pb.PositionType_POSITION_TYPE_TICKS_COUNT, nil + if *positionType == PositionTypeTICKS { + return pb.PositionType_POSITION_TYPE_TICKS_COUNT } - return pb.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return pb.PositionType_POSITION_TYPE_UNSPECIFIED } diff --git a/components/encoder/encoder_test.go b/components/encoder/encoder_test.go index 38512c94a9b..45a065921a7 100644 --- a/components/encoder/encoder_test.go +++ b/components/encoder/encoder_test.go @@ -188,7 +188,10 @@ func TestReconfigurableEncoder(t *testing.T) { test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) test.That(t, actualEncoder2.posCount, test.ShouldEqual, 0) - result, _, err := reconfEncoder1.(encoder.Encoder).GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_UNSPECIFIED.Enum(), nil) + result, _, err := reconfEncoder1.(encoder.Encoder).GetPosition( + context.Background(), + encoder.PositionTypeUNSPECIFIED.Enum(), + nil) test.That(t, err, test.ShouldBeNil) test.That(t, result, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 0) @@ -225,7 +228,7 @@ func TestGetPosition(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, pos1, test.ShouldResemble, position) test.That(t, actualEncoder1.posCount, test.ShouldEqual, 1) - test.That(t, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_UNSPECIFIED) + test.That(t, positionType, test.ShouldEqual, encoder.PositionTypeUNSPECIFIED) props, err := reconfEncoder1.(encoder.Encoder).GetProperties(context.Background(), nil) test.That(t, err, test.ShouldBeNil) @@ -273,7 +276,7 @@ func (m *mock) GetPosition( ) (float64, encoder.PositionType, error) { m.posCount++ m.extra = extra - return position, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return position, encoder.PositionTypeUNSPECIFIED, nil } func (m *mock) GetProperties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) { diff --git a/components/encoder/errors.go b/components/encoder/errors.go index 87a9a3309e2..2dea9ba46e6 100644 --- a/components/encoder/errors.go +++ b/components/encoder/errors.go @@ -6,20 +6,22 @@ import "github.com/pkg/errors" // an encoder does not support the given PositionType. func NewEncoderTypeUnsupportedError(positionType PositionType) error { if positionType == 1 { - return errors.Errorf( + return errors.New( "Encoder does not support Ticks, use a different PositionType") } if positionType == 2 { - return errors.Errorf( + return errors.New( "Encoder does not support Angles, use a different PositionType") } - return errors.Errorf("Cannot identify PositionType") + return errors.New("Cannot identify PositionType") } +// NewEncodedMotorTypeUnsupportedError returns a standard error for when +// an encoded motor tries to use an encoder that doesn't support Ticks. func NewEncodedMotorTypeUnsupportedError(props map[Feature]bool) error { if props[AngleDegreesSupported] { - return errors.Errorf( + return errors.New( "encoder type is Angle Degrees, need an encoder that supports Ticks for an encoded motor") } - return errors.Errorf("need an encoder that supports Ticks for an encoded motor") + return errors.New("need an encoder that supports Ticks for an encoded motor") } diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 16039ac8ad1..2f02db62abd 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -48,7 +48,10 @@ func newFakeEncoder( ) (encoder.Encoder, error) { e := &Encoder{ position: 0, - positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionTypeTICKS, + } + if e.positionType != encoder.PositionTypeTICKS { + return nil, errors.New("could not set PositionType to Ticks") } e.updateRate = cfg.ConvertedAttributes.(*AttrConfig).UpdateRate @@ -85,7 +88,7 @@ func (e *Encoder) GetPosition( positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { + if positionType != nil && *positionType == encoder.PositionTypeDEGREES { err := errors.New("Encoder does not support PositionType Angle Degrees, use a different PositionType") return 0, *positionType, err } diff --git a/components/encoder/fake/encoder_test.go b/components/encoder/fake/encoder_test.go index b34835c6681..b7ce6c57762 100644 --- a/components/encoder/fake/encoder_test.go +++ b/components/encoder/fake/encoder_test.go @@ -24,7 +24,7 @@ func TestEncoder(t *testing.T) { pos, positionType, err := e.GetPosition(ctx, nil, nil) test.That(t, pos, test.ShouldEqual, 0) test.That(t, err, test.ShouldBeNil) - test.That(t, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(t, positionType, test.ShouldEqual, encoder.PositionTypeTICKS) e1 := e.(*Encoder) @@ -49,10 +49,10 @@ func TestEncoder(t *testing.T) { t.Run("specify a type", func(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := e.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := e.GetPosition(context.Background(), encoder.PositionTypeTICKS.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionTypeTICKS) }) }) t.Run("get properties", func(t *testing.T) { diff --git a/components/encoder/features.go b/components/encoder/features.go index 3a08019a73d..6ab9b422f34 100644 --- a/components/encoder/features.go +++ b/components/encoder/features.go @@ -9,7 +9,7 @@ import ( type Feature string // TicksCountSupported and AngleDegreesSupported represesnts the feature -// of an encoder being able to report ticks and/or degrees, respectively +// of an encoder being able to report ticks and/or degrees, respectively. const ( TicksCountSupported Feature = "Ticks" AngleDegreesSupported Feature = "Degrees" diff --git a/components/encoder/incremental/incremental_encoder.go b/components/encoder/incremental/incremental_encoder.go index fbd4378f5fd..7ad830f73bf 100644 --- a/components/encoder/incremental/incremental_encoder.go +++ b/components/encoder/incremental/incremental_encoder.go @@ -103,7 +103,7 @@ func NewIncrementalEncoder( CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, - positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionTypeTICKS, pRaw: 0, pState: 0, } @@ -248,7 +248,7 @@ func (e *Encoder) GetPosition( positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { + if positionType != nil && *positionType == encoder.PositionTypeDEGREES { err := encoder.NewEncoderTypeUnsupportedError(*positionType) return 0, *positionType, err } diff --git a/components/encoder/incremental/incremental_test.go b/components/encoder/incremental/incremental_test.go index 0c13a199fa7..0edf55b2957 100644 --- a/components/encoder/incremental/incremental_test.go +++ b/components/encoder/incremental/incremental_test.go @@ -124,10 +124,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionTypeTICKS.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionTypeTICKS) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -138,10 +138,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionTypeDEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionTypeDEGREES) }) }) diff --git a/components/encoder/server.go b/components/encoder/server.go index 8ff9478462d..538ba916669 100644 --- a/components/encoder/server.go +++ b/components/encoder/server.go @@ -44,12 +44,12 @@ func (s *subtypeServer) GetPosition( if err != nil { return nil, err } - posType, err := ProtoToEncoderPositionType(req.PositionType) + posType := ToEncoderPositionType(req.PositionType) position, positionType, err := enc.GetPosition(ctx, &posType, req.Extra.AsMap()) if err != nil { return nil, err } - posType1, err := EncoderToProtoPositionType(&positionType) + posType1 := ToProtoPositionType(&positionType) return &pb.GetPositionResponse{ Value: float32(position), PositionType: posType1, diff --git a/components/encoder/server_test.go b/components/encoder/server_test.go index bb80d9c9b19..29077eea134 100644 --- a/components/encoder/server_test.go +++ b/components/encoder/server_test.go @@ -46,7 +46,7 @@ func TestServerGetPosition(t *testing.T) { positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - return 0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, errors.New("position unavailable") + return 0, encoder.PositionTypeUNSPECIFIED, errors.New("position unavailable") } req = pb.GetPositionRequest{Name: failEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) @@ -58,7 +58,7 @@ func TestServerGetPosition(t *testing.T) { positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - return 42.0, encoder.PositionType_POSITION_TYPE_UNSPECIFIED, nil + return 42.0, encoder.PositionTypeUNSPECIFIED, nil } req = pb.GetPositionRequest{Name: testEncoderName} resp, err = encoderServer.GetPosition(context.Background(), &req) diff --git a/components/encoder/single/single_encoder.go b/components/encoder/single/single_encoder.go index 27444dbff63..8ad867f4ab3 100644 --- a/components/encoder/single/single_encoder.go +++ b/components/encoder/single/single_encoder.go @@ -137,7 +137,7 @@ func NewSingleEncoder( CancelCtx: cancelCtx, cancelFunc: cancelFunc, position: 0, - positionType: encoder.PositionType_POSITION_TYPE_TICKS_COUNT, + positionType: encoder.PositionTypeTICKS, } board, err := board.FromDependencies(deps, cfg.BoardName) @@ -197,7 +197,7 @@ func (e *Encoder) GetPosition( positionType *encoder.PositionType, extra map[string]interface{}, ) (float64, encoder.PositionType, error) { - if positionType != nil && *positionType == encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES { + if positionType != nil && *positionType == encoder.PositionTypeDEGREES { err := encoder.NewEncoderTypeUnsupportedError(*positionType) return 0, *positionType, err } diff --git a/components/encoder/single/single_encoder_test.go b/components/encoder/single/single_encoder_test.go index 97fad864b20..1b97ef07243 100644 --- a/components/encoder/single/single_encoder_test.go +++ b/components/encoder/single/single_encoder_test.go @@ -191,10 +191,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_TICKS_COUNT.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionTypeTICKS.Enum(), nil) test.That(tb, err, test.ShouldBeNil) test.That(tb, ticks, test.ShouldEqual, 1) - test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_TICKS_COUNT) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionTypeTICKS) }) }) t.Run("specify wrong position type", func(t *testing.T) { @@ -211,10 +211,10 @@ func TestEnconder(t *testing.T) { testutils.WaitForAssertion(t, func(tb testing.TB) { tb.Helper() - ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES.Enum(), nil) + ticks, positionType, err := enc.GetPosition(context.Background(), encoder.PositionTypeDEGREES.Enum(), nil) test.That(tb, err, test.ShouldNotBeNil) test.That(tb, ticks, test.ShouldEqual, 0) - test.That(tb, positionType, test.ShouldEqual, encoder.PositionType_POSITION_TYPE_ANGLE_DEGREES) + test.That(tb, positionType, test.ShouldEqual, encoder.PositionTypeDEGREES) }) }) t.Run("get properties", func(t *testing.T) { diff --git a/go.mod b/go.mod index ffabff1439c..691602ed602 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,7 @@ require ( go.uber.org/atomic v1.10.0 go.uber.org/multierr v1.10.0 go.uber.org/zap v1.24.0 - go.viam.com/api v0.1.106 + go.viam.com/api v0.1.108 go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2 go.viam.com/utils v0.1.18-0.20230327140716-bfeb34d89117 goji.io v2.0.2+incompatible diff --git a/go.sum b/go.sum index b47c29679bd..e4d503cba03 100644 --- a/go.sum +++ b/go.sum @@ -1580,8 +1580,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.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60= go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= -go.viam.com/api v0.1.106 h1:H//FPoU/KHRtyZI5wBJzQmf5hJUronqnfAVv4Q9fk28= -go.viam.com/api v0.1.106/go.mod h1:NQC9FZnerAfIPJLJ42vgzQMoe1WAQRuVoQEf1JauxtQ= +go.viam.com/api v0.1.108 h1:dM7cG6bRK1ZuG2w3YvWIKg+cyYRxgQdRUvm45jR5UIQ= +go.viam.com/api v0.1.108/go.mod h1:NQC9FZnerAfIPJLJ42vgzQMoe1WAQRuVoQEf1JauxtQ= go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2 h1:oBiK580EnEIzgFLU4lHOXmGAE3MxnVbeR7s1wp/F3Ps= go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2/go.mod h1:XM0tej6riszsiNLT16uoyq1YjuYPWlRBweTPRDanIts= go.viam.com/utils v0.1.18-0.20230327140716-bfeb34d89117 h1:ZNROrfA1vEfzwbZCQMBBln/mj26+yL7s9GMpENnwmEA= diff --git a/testutils/inject/encoder.go b/testutils/inject/encoder.go index 510f833647f..4a126893655 100644 --- a/testutils/inject/encoder.go +++ b/testutils/inject/encoder.go @@ -11,7 +11,10 @@ type Encoder struct { encoder.Encoder DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) ResetPositionFunc func(ctx context.Context, extra map[string]interface{}) error - GetPositionFunc func(ctx context.Context, positionType *encoder.PositionType, extra map[string]interface{}) (float64, encoder.PositionType, error) + GetPositionFunc func(ctx context.Context, + positionType *encoder.PositionType, + extra map[string]interface{}, + ) (float64, encoder.PositionType, error) GetPropertiesFunc func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) } From 2231c84ecc9fa10bbf60a7e7ae74c3e0fccf81d4 Mon Sep 17 00:00:00 2001 From: martha-johnston Date: Mon, 10 Apr 2023 11:50:25 -0400 Subject: [PATCH 10/10] return no error new fake encoder --- components/encoder/fake/encoder.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/encoder/fake/encoder.go b/components/encoder/fake/encoder.go index 2f02db62abd..63c7e26d140 100644 --- a/components/encoder/fake/encoder.go +++ b/components/encoder/fake/encoder.go @@ -42,6 +42,8 @@ func init() { } // newFakeEncoder creates a new Encoder. +// +//nolint:unparam func newFakeEncoder( ctx context.Context, cfg config.Component, @@ -50,9 +52,6 @@ func newFakeEncoder( position: 0, positionType: encoder.PositionTypeTICKS, } - if e.positionType != encoder.PositionTypeTICKS { - return nil, errors.New("could not set PositionType to Ticks") - } e.updateRate = cfg.ConvertedAttributes.(*AttrConfig).UpdateRate e.Start(ctx)