Skip to content

Commit 8cdecaf

Browse files
oliviamilleroliviamiller
authored
RSDK-3509 Add Encoder properties struct and review client/server tests (#2655)
1 parent d554bca commit 8cdecaf

17 files changed

+94
-88
lines changed

‎components/encoder/ams/ams_as5048.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,10 @@ func (enc *Encoder) ResetPosition(
319319
}
320320

321321
// Properties returns a list of all the position types that are supported by a given encoder.
322-
func (enc *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
323-
return map[encoder.Feature]bool{
324-
encoder.TicksCountSupported: true,
325-
encoder.AngleDegreesSupported: true,
322+
func (enc *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
323+
return encoder.Properties{
324+
TicksCountSupported: true,
325+
AngleDegreesSupported: true,
326326
}, nil
327327
}
328328

‎components/encoder/client.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,17 @@ func (c *client) ResetPosition(ctx context.Context, extra map[string]interface{}
7575
}
7676

7777
// Properties returns a list of all the position types that are supported by a given encoder.
78-
func (c *client) Properties(ctx context.Context, extra map[string]interface{}) (map[Feature]bool, error) {
78+
func (c *client) Properties(ctx context.Context, extra map[string]interface{}) (Properties, error) {
7979
ext, err := structpb.NewStruct(extra)
8080
if err != nil {
81-
return nil, err
81+
return Properties{}, err
8282
}
8383
req := &pb.GetPropertiesRequest{Name: c.name, Extra: ext}
8484
resp, err := c.client.GetProperties(ctx, req)
8585
if err != nil {
86-
return nil, err
86+
return Properties{}, err
8787
}
88-
return ProtoFeaturesToMap(resp), nil
88+
return ProtoFeaturesToProperties(resp), nil
8989
}
9090

9191
func (c *client) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) {

‎components/encoder/client_test.go

+11-10
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package encoder_test
22

33
import (
44
"context"
5-
"errors"
65
"net"
76
"testing"
87

@@ -48,26 +47,26 @@ func TestClient(t *testing.T) {
4847
actualExtra = extra
4948
return 42.0, encoder.PositionTypeUnspecified, nil
5049
}
51-
workingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
50+
workingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
5251
actualExtra = extra
53-
return map[encoder.Feature]bool{
54-
encoder.TicksCountSupported: true,
55-
encoder.AngleDegreesSupported: false,
52+
return encoder.Properties{
53+
TicksCountSupported: true,
54+
AngleDegreesSupported: false,
5655
}, nil
5756
}
5857

5958
failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error {
60-
return errors.New("set to zero failed")
59+
return errSetToZeroFailed
6160
}
6261
failingEncoder.PositionFunc = func(
6362
ctx context.Context,
6463
positionType encoder.PositionType,
6564
extra map[string]interface{},
6665
) (float64, encoder.PositionType, error) {
67-
return 0, encoder.PositionTypeUnspecified, errors.New("position unavailable")
66+
return 0, encoder.PositionTypeUnspecified, errPositionUnavailable
6867
}
69-
failingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
70-
return nil, errors.New("get properties failed")
68+
failingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
69+
return encoder.Properties{}, errGetPropertiesFailed
7170
}
7271

7372
resourceMap := map[resource.Name]encoder.Encoder{
@@ -91,7 +90,7 @@ func TestClient(t *testing.T) {
9190
cancel()
9291
_, err := viamgrpc.Dial(cancelCtx, listener1.Addr().String(), logger)
9392
test.That(t, err, test.ShouldNotBeNil)
94-
test.That(t, err.Error(), test.ShouldContainSubstring, "canceled")
93+
test.That(t, err, test.ShouldBeError, context.Canceled)
9594
})
9695

9796
conn, err := viamgrpc.Dial(context.Background(), listener1.Addr().String(), logger)
@@ -132,9 +131,11 @@ func TestClient(t *testing.T) {
132131
t.Run("client tests for failing encoder", func(t *testing.T) {
133132
err = failingEncoderClient.ResetPosition(context.Background(), nil)
134133
test.That(t, err, test.ShouldNotBeNil)
134+
test.That(t, err.Error(), test.ShouldContainSubstring, errSetToZeroFailed.Error())
135135

136136
pos, _, err := failingEncoderClient.Position(context.Background(), encoder.PositionTypeUnspecified, nil)
137137
test.That(t, err, test.ShouldNotBeNil)
138+
test.That(t, err.Error(), test.ShouldContainSubstring, errPositionUnavailable.Error())
138139
test.That(t, pos, test.ShouldEqual, 0.0)
139140

140141
test.That(t, failingEncoderClient.Close(context.Background()), test.ShouldBeNil)

‎components/encoder/encoder.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ type Encoder interface {
6868
ResetPosition(ctx context.Context, extra map[string]interface{}) error
6969

7070
// Properties returns a list of all the position types that are supported by a given encoder
71-
Properties(ctx context.Context, extra map[string]interface{}) (map[Feature]bool, error)
71+
Properties(ctx context.Context, extra map[string]interface{}) (Properties, error)
7272
}
7373

7474
// Named is a helper for getting the named Encoder's typed resource name.

‎components/encoder/errors.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ func NewPositionTypeUnsupportedError(positionType PositionType) error {
1010

1111
// NewEncodedMotorPositionTypeUnsupportedError returns a standard error for when
1212
// an encoded motor tries to use an encoder that doesn't support Ticks.
13-
func NewEncodedMotorPositionTypeUnsupportedError(props map[Feature]bool) error {
14-
if props[AngleDegreesSupported] {
13+
func NewEncodedMotorPositionTypeUnsupportedError(props Properties) error {
14+
if props.AngleDegreesSupported {
1515
return errors.New(
1616
"encoder position type is Angle Degrees, need an encoder that supports Ticks")
1717
}

‎components/encoder/fake/encoder.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,10 @@ func (e *fakeEncoder) ResetPosition(ctx context.Context, extra map[string]interf
139139
}
140140

141141
// Properties returns a list of all the position types that are supported by a given encoder.
142-
func (e *fakeEncoder) Properties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
143-
return map[encoder.Feature]bool{
144-
encoder.TicksCountSupported: true,
145-
encoder.AngleDegreesSupported: false,
142+
func (e *fakeEncoder) Properties(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
143+
return encoder.Properties{
144+
TicksCountSupported: true,
145+
AngleDegreesSupported: false,
146146
}, nil
147147
}
148148

‎components/encoder/fake/encoder_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ func TestEncoder(t *testing.T) {
6060
tb.Helper()
6161
props, err := e.Properties(ctx, nil)
6262
test.That(tb, err, test.ShouldBeNil)
63-
test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue)
64-
test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse)
63+
test.That(tb, props.TicksCountSupported, test.ShouldBeTrue)
64+
test.That(tb, props.AngleDegreesSupported, test.ShouldBeFalse)
6565
})
6666
})
6767

‎components/encoder/features.go

-36
This file was deleted.

‎components/encoder/incremental/incremental_encoder.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,10 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{
310310
}
311311

312312
// Properties returns a list of all the position types that are supported by a given encoder.
313-
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
314-
return map[encoder.Feature]bool{
315-
encoder.TicksCountSupported: true,
316-
encoder.AngleDegreesSupported: false,
313+
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
314+
return encoder.Properties{
315+
TicksCountSupported: true,
316+
AngleDegreesSupported: false,
317317
}, nil
318318
}
319319

‎components/encoder/incremental/incremental_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ func TestEncoder(t *testing.T) {
154154
tb.Helper()
155155
props, err := enc.Properties(ctx, nil)
156156
test.That(tb, err, test.ShouldBeNil)
157-
test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue)
158-
test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse)
157+
test.That(tb, props.TicksCountSupported, test.ShouldBeTrue)
158+
test.That(tb, props.AngleDegreesSupported, test.ShouldBeFalse)
159159
})
160160
})
161161
}

‎components/encoder/properties.go

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package encoder
2+
3+
import pb "go.viam.com/api/component/encoder/v1"
4+
5+
// Properties holds the properties of the encoder.
6+
type Properties struct {
7+
TicksCountSupported bool
8+
AngleDegreesSupported bool
9+
}
10+
11+
// ProtoFeaturesToProperties takes a GetPropertiesResponse and returns
12+
// an equivalent Properties struct.
13+
func ProtoFeaturesToProperties(resp *pb.GetPropertiesResponse) Properties {
14+
return Properties{
15+
TicksCountSupported: resp.TicksCountSupported,
16+
AngleDegreesSupported: resp.AngleDegreesSupported,
17+
}
18+
}
19+
20+
// PropertiesToProtoResponse takes a properties struct and converts it
21+
// to a GetPropertiesResponse.
22+
func PropertiesToProtoResponse(
23+
props Properties,
24+
) (*pb.GetPropertiesResponse, error) {
25+
return &pb.GetPropertiesResponse{
26+
TicksCountSupported: props.TicksCountSupported,
27+
AngleDegreesSupported: props.AngleDegreesSupported,
28+
}, nil
29+
}

‎components/encoder/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (s *serviceServer) GetProperties(
7070
if err != nil {
7171
return nil, err
7272
}
73-
return FeatureMapToProtoResponse(features)
73+
return PropertiesToProtoResponse(features)
7474
}
7575

7676
// DoCommand receives arbitrary commands.

‎components/encoder/server_test.go

+21-9
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ import (
1414
"go.viam.com/rdk/testutils/inject"
1515
)
1616

17+
var (
18+
errPositionUnavailable = errors.New("position unavailable")
19+
errSetToZeroFailed = errors.New("set to zero failed")
20+
errPropertiesNotFound = errors.New("properties not found")
21+
errGetPropertiesFailed = errors.New("get properties failed")
22+
)
23+
1724
func newServer() (pb.EncoderServiceServer, *inject.Encoder, *inject.Encoder, error) {
1825
injectEncoder1 := &inject.Encoder{}
1926
injectEncoder2 := &inject.Encoder{}
@@ -38,18 +45,21 @@ func TestServerGetPosition(t *testing.T) {
3845
resp, err := encoderServer.GetPosition(context.Background(), &req)
3946
test.That(t, resp, test.ShouldBeNil)
4047
test.That(t, err, test.ShouldNotBeNil)
48+
test.That(t, resource.IsNotFoundError(err), test.ShouldBeTrue)
4149

4250
failingEncoder.PositionFunc = func(
4351
ctx context.Context,
4452
positionType encoder.PositionType,
4553
extra map[string]interface{},
4654
) (float64, encoder.PositionType, error) {
47-
return 0, encoder.PositionTypeUnspecified, errors.New("position unavailable")
55+
return 0, encoder.PositionTypeUnspecified, errPositionUnavailable
4856
}
57+
58+
// Position unavailable test
4959
req = pb.GetPositionRequest{Name: failEncoderName}
5060
resp, err = encoderServer.GetPosition(context.Background(), &req)
5161
test.That(t, resp, test.ShouldBeNil)
52-
test.That(t, err, test.ShouldNotBeNil)
62+
test.That(t, err, test.ShouldBeError, errPositionUnavailable)
5363

5464
workingEncoder.PositionFunc = func(
5565
ctx context.Context,
@@ -74,12 +84,13 @@ func TestServerResetPosition(t *testing.T) {
7484
test.That(t, err, test.ShouldNotBeNil)
7585

7686
failingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error {
77-
return errors.New("set to zero failed")
87+
return errSetToZeroFailed
7888
}
7989
req = pb.ResetPositionRequest{Name: failEncoderName}
8090
resp, err = encoderServer.ResetPosition(context.Background(), &req)
8191
test.That(t, resp, test.ShouldNotBeNil)
8292
test.That(t, err, test.ShouldNotBeNil)
93+
test.That(t, err, test.ShouldBeError, errSetToZeroFailed)
8394

8495
workingEncoder.ResetPositionFunc = func(ctx context.Context, extra map[string]interface{}) error {
8596
return nil
@@ -99,18 +110,19 @@ func TestServerGetProperties(t *testing.T) {
99110
test.That(t, resp, test.ShouldBeNil)
100111
test.That(t, err, test.ShouldNotBeNil)
101112

102-
failingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
103-
return nil, errors.New("properties not found")
113+
failingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
114+
return encoder.Properties{}, errPropertiesNotFound
104115
}
105116
req = pb.GetPropertiesRequest{Name: failEncoderName}
106117
resp, err = encoderServer.GetProperties(context.Background(), &req)
107118
test.That(t, resp, test.ShouldBeNil)
108119
test.That(t, err, test.ShouldNotBeNil)
120+
test.That(t, err, test.ShouldBeError, errPropertiesNotFound)
109121

110-
workingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
111-
return map[encoder.Feature]bool{
112-
encoder.TicksCountSupported: true,
113-
encoder.AngleDegreesSupported: false,
122+
workingEncoder.PropertiesFunc = func(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
123+
return encoder.Properties{
124+
TicksCountSupported: true,
125+
AngleDegreesSupported: false,
114126
}, nil
115127
}
116128
req = pb.GetPropertiesRequest{Name: testEncoderName}

‎components/encoder/single/single_encoder.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,10 @@ func (e *Encoder) ResetPosition(ctx context.Context, extra map[string]interface{
236236
}
237237

238238
// Properties returns a list of all the position types that are supported by a given encoder.
239-
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
240-
return map[encoder.Feature]bool{
241-
encoder.TicksCountSupported: true,
242-
encoder.AngleDegreesSupported: false,
239+
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
240+
return encoder.Properties{
241+
TicksCountSupported: true,
242+
AngleDegreesSupported: false,
243243
}, nil
244244
}
245245

‎components/encoder/single/single_encoder_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ func TestEncoder(t *testing.T) {
225225
tb.Helper()
226226
props, err := enc.Properties(ctx, nil)
227227
test.That(tb, err, test.ShouldBeNil)
228-
test.That(tb, props[encoder.TicksCountSupported], test.ShouldBeTrue)
229-
test.That(tb, props[encoder.AngleDegreesSupported], test.ShouldBeFalse)
228+
test.That(tb, props.TicksCountSupported, test.ShouldBeTrue)
229+
test.That(tb, props.AngleDegreesSupported, test.ShouldBeFalse)
230230
})
231231
})
232232
}

‎components/motor/gpio/motor_encoder.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func newEncodedMotor(
128128
if err != nil {
129129
return nil, errors.New("cannot get encoder properties")
130130
}
131-
if !props[encoder.TicksCountSupported] {
131+
if !props.TicksCountSupported {
132132
return nil,
133133
encoder.NewEncodedMotorPositionTypeUnsupportedError(props)
134134
}

‎testutils/inject/encoder.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type Encoder struct {
1717
positionType encoder.PositionType,
1818
extra map[string]interface{},
1919
) (float64, encoder.PositionType, error)
20-
PropertiesFunc func(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error)
20+
PropertiesFunc func(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error)
2121
}
2222

2323
// NewEncoder returns a new injected Encoder.
@@ -51,7 +51,7 @@ func (e *Encoder) Position(
5151
}
5252

5353
// Properties calls the injected Properties or the real version.
54-
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (map[encoder.Feature]bool, error) {
54+
func (e *Encoder) Properties(ctx context.Context, extra map[string]interface{}) (encoder.Properties, error) {
5555
if e.PropertiesFunc == nil {
5656
return e.Encoder.Properties(ctx, extra)
5757
}

0 commit comments

Comments
 (0)