Skip to content

Commit

Permalink
Change AnalogByName to return an Analog and an error (viamrobotics#3813)
Browse files Browse the repository at this point in the history
  • Loading branch information
martha-johnston authored and vijayvuyyuru committed Apr 22, 2024
1 parent 0e2c3ee commit 783b165
Show file tree
Hide file tree
Showing 21 changed files with 70 additions and 57 deletions.
2 changes: 1 addition & 1 deletion components/board/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Board interface {
resource.Resource

// AnalogByName returns an analog pin by name.
AnalogByName(name string) (Analog, bool)
AnalogByName(name string) (Analog, error)

// DigitalInterruptByName returns a digital interrupt by name.
DigitalInterruptByName(name string) (DigitalInterrupt, bool)
Expand Down
4 changes: 2 additions & 2 deletions components/board/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func NewClientFromConn(
return c, nil
}

func (c *client) AnalogByName(name string) (Analog, bool) {
func (c *client) AnalogByName(name string) (Analog, error) {
return &analogClient{
client: c,
boardName: c.info.name,
analogName: name,
}, true
}, nil
}

func (c *client) DigitalInterruptByName(name string) (DigitalInterrupt, bool) {
Expand Down
8 changes: 4 additions & 4 deletions components/board/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ func TestWorkingClient(t *testing.T) {

// Analog
injectAnalog := &inject.Analog{}
injectBoard.AnaloByNameFunc = func(name string) (board.Analog, bool) {
return injectAnalog, true
injectBoard.AnalogByNameFunc = func(name string) (board.Analog, error) {
return injectAnalog, nil
}
analog1, ok := injectBoard.AnalogByName("analog1")
test.That(t, ok, test.ShouldBeTrue)
analog1, err := injectBoard.AnalogByName("analog1")
test.That(t, err, test.ShouldBeNil)
test.That(t, injectBoard.AnalogByNameCap(), test.ShouldResemble, []interface{}{"analog1"})

// Analog: Read
Expand Down
2 changes: 1 addition & 1 deletion components/board/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func newAnalogCollector(resource interface{}, params data.CollectorParams) (data
return nil, data.FailedToReadErr(params.ComponentName, analogs.String(),
errors.New("Must supply reader_name in additional_params for analog collector"))
}
if reader, ok := board.AnalogByName(arg[analogReaderNameKey].String()); ok {
if reader, err := board.AnalogByName(arg[analogReaderNameKey].String()); err == nil {
value, err = reader.Read(ctx, data.FromDMExtraMap)
if err != nil {
// A modular filter component can be created to filter the readings from a component. The error ErrNoCaptureToStore
Expand Down
4 changes: 2 additions & 2 deletions components/board/collectors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ func newBoard() board.Board {
analog.ReadFunc = func(ctx context.Context, extra map[string]interface{}) (int, error) {
return 1, nil
}
b.AnaloByNameFunc = func(name string) (board.Analog, bool) {
return analog, true
b.AnalogByNameFunc = func(name string) (board.Analog, error) {
return analog, nil
}
gpioPin := &inject.GPIOPin{}
gpioPin.GetFunc = func(ctx context.Context, extra map[string]interface{}) (bool, error) {
Expand Down
7 changes: 5 additions & 2 deletions components/board/fake/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,14 @@ type Board struct {
}

// AnalogByName returns the analog pin by the given name if it exists.
func (b *Board) AnalogByName(name string) (board.Analog, bool) {
func (b *Board) AnalogByName(name string) (board.Analog, error) {
b.mu.RLock()
defer b.mu.RUnlock()
a, ok := b.Analogs[name]
return a, ok
if !ok {
return nil, errors.Errorf("can't find AnalogReader (%s)", name)
}
return a, nil
}

// DigitalInterruptByName returns the interrupt by the given name if it exists.
Expand Down
6 changes: 3 additions & 3 deletions components/board/fake/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ func TestFakeBoard(t *testing.T) {
b, err := NewBoard(context.Background(), cfg, logger)
test.That(t, err, test.ShouldBeNil)

_, ok := b.AnalogByName("blue")
test.That(t, ok, test.ShouldBeTrue)
_, err = b.AnalogByName("blue")
test.That(t, err, test.ShouldBeNil)

_, ok = b.DigitalInterruptByName("i1")
_, ok := b.DigitalInterruptByName("i1")
test.That(t, ok, test.ShouldBeTrue)
_, ok = b.DigitalInterruptByName("i2")
test.That(t, ok, test.ShouldBeTrue)
Expand Down
7 changes: 5 additions & 2 deletions components/board/genericlinux/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,12 @@ type Board struct {
}

// AnalogByName returns the analog pin by the given name if it exists.
func (b *Board) AnalogByName(name string) (board.Analog, bool) {
func (b *Board) AnalogByName(name string) (board.Analog, error) {
a, ok := b.analogReaders[name]
return a, ok
if !ok {
return nil, errors.Errorf("can't find AnalogReader (%s)", name)
}
return a, nil
}

// DigitalInterruptByName returns the interrupt by the given name if it exists.
Expand Down
8 changes: 4 additions & 4 deletions components/board/genericlinux/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ func TestGenericLinux(t *testing.T) {
ans := b.AnalogNames()
test.That(t, ans, test.ShouldResemble, []string{"an"})

an1, ok := b.AnalogByName("an")
an1, err := b.AnalogByName("an")
test.That(t, an1, test.ShouldHaveSameTypeAs, &wrappedAnalogReader{})
test.That(t, ok, test.ShouldBeTrue)
test.That(t, err, test.ShouldBeNil)

an2, ok := b.AnalogByName("missing")
an2, err := b.AnalogByName("missing")
test.That(t, an2, test.ShouldBeNil)
test.That(t, ok, test.ShouldBeFalse)
test.That(t, err, test.ShouldNotBeNil)

dns := b.DigitalInterruptNames()
test.That(t, dns, test.ShouldBeNil)
Expand Down
4 changes: 2 additions & 2 deletions components/board/hat/pca9685/pca9685.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ func (pca *PCA9685) DigitalInterruptNames() []string {
}

// AnalogByName returns the analog pin by the given name if it exists.
func (pca *PCA9685) AnalogByName(name string) (board.Analog, bool) {
return nil, false
func (pca *PCA9685) AnalogByName(name string) (board.Analog, error) {
return nil, nil
}

// DigitalInterruptByName returns the interrupt by the given name if it exists.
Expand Down
7 changes: 5 additions & 2 deletions components/board/numato/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,12 @@ func (b *numatoBoard) StreamTicks(ctx context.Context, interrupts []string, ch c
}

// AnalogByName returns an analog pin by name.
func (b *numatoBoard) AnalogByName(name string) (board.Analog, bool) {
func (b *numatoBoard) AnalogByName(name string) (board.Analog, error) {
ar, ok := b.analogs[name]
return ar, ok
if !ok {
return nil, fmt.Errorf("can't find AnalogReader (%s)", name)
}
return ar, nil
}

// DigitalInterruptByName returns a digital interrupt by name.
Expand Down
4 changes: 2 additions & 2 deletions components/board/numato/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func TestNumato1(t *testing.T) {
test.That(t, res, test.ShouldEqual, false)

// test analog
ar, ok := b.AnalogByName("foo")
test.That(t, ok, test.ShouldEqual, true)
ar, err := b.AnalogByName("foo")
test.That(t, err, test.ShouldBeNil, true)

res2, err := ar.Read(ctx, nil)
test.That(t, err, test.ShouldBeNil)
Expand Down
7 changes: 5 additions & 2 deletions components/board/pi/impl/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,14 @@ func (pi *piPigpio) DigitalInterruptNames() []string {
}

// AnalogByName returns an analog pin by name.
func (pi *piPigpio) AnalogByName(name string) (board.Analog, bool) {
func (pi *piPigpio) AnalogByName(name string) (board.Analog, error) {
pi.mu.Lock()
defer pi.mu.Unlock()
a, ok := pi.analogReaders[name]
return a, ok
if !ok {
return nil, errors.Errorf("can't find AnalogReader (%s)", name)
}
return a, nil
}

// DigitalInterruptByName returns a digital interrupt by name.
Expand Down
4 changes: 2 additions & 2 deletions components/board/pi/impl/external_hardware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func TestPiHardware(t *testing.T) {
}()

t.Run("analog test", func(t *testing.T) {
reader, ok := p.AnalogByName("blue")
test.That(t, ok, test.ShouldBeTrue)
reader, err := p.AnalogByName("blue")
test.That(t, err, test.ShouldBeNil)
if reader == nil {
t.Skip("no blue? analog")
return
Expand Down
6 changes: 3 additions & 3 deletions components/board/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ func (s *serviceServer) ReadAnalogReader(
return nil, err
}

theReader, ok := b.AnalogByName(req.AnalogReaderName)
if !ok {
return nil, errors.Errorf("unknown analog reader: %s", req.AnalogReaderName)
theReader, err := b.AnalogByName(req.AnalogReaderName)
if err != nil {
return nil, err
}

val, err := theReader.Read(ctx, req.Extra.AsMap())
Expand Down
17 changes: 9 additions & 8 deletions components/board/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var (
errFoo = errors.New("whoops")
errNotFound = errors.New("not found")
errSendFailed = errors.New("send fail")
errAnalog = errors.New("unknown analog error")
)

func newServer() (pb.BoardServiceServer, *inject.Board, error) {
Expand Down Expand Up @@ -540,7 +541,7 @@ func TestServerReadAnalogReader(t *testing.T) {

tests := []struct {
injectAnalog *inject.Analog
injectAnalogOk bool
injectAnalogErr error
injectResult int
injectErr error
req *request
Expand All @@ -551,7 +552,7 @@ func TestServerReadAnalogReader(t *testing.T) {
}{
{
injectAnalog: nil,
injectAnalogOk: false,
injectAnalogErr: errAnalog,
injectResult: 0,
injectErr: nil,
req: &request{BoardName: missingBoardName},
Expand All @@ -562,18 +563,18 @@ func TestServerReadAnalogReader(t *testing.T) {
},
{
injectAnalog: nil,
injectAnalogOk: false,
injectAnalogErr: errAnalog,
injectResult: 0,
injectErr: nil,
req: &request{BoardName: testBoardName, AnalogReaderName: "analog1"},
expCapAnalogArgs: []interface{}{"analog1"},
expCapArgs: []interface{}(nil),
expResp: nil,
expRespErr: "unknown analog reader: analog1",
expRespErr: "unknown analog error",
},
{
injectAnalog: &inject.Analog{},
injectAnalogOk: true,
injectAnalogErr: nil,
injectResult: 0,
injectErr: errFoo,
req: &request{BoardName: testBoardName, AnalogReaderName: "analog1"},
Expand All @@ -584,7 +585,7 @@ func TestServerReadAnalogReader(t *testing.T) {
},
{
injectAnalog: &inject.Analog{},
injectAnalogOk: true,
injectAnalogErr: nil,
injectResult: 8,
injectErr: nil,
req: &request{BoardName: testBoardName, AnalogReaderName: "analog1", Extra: pbExpectedExtra},
Expand All @@ -601,8 +602,8 @@ func TestServerReadAnalogReader(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
var actualExtra map[string]interface{}

injectBoard.AnaloByNameFunc = func(name string) (board.Analog, bool) {
return tc.injectAnalog, tc.injectAnalogOk
injectBoard.AnalogByNameFunc = func(name string) (board.Analog, error) {
return tc.injectAnalog, tc.injectAnalogErr
}

if tc.injectAnalog != nil {
Expand Down
6 changes: 3 additions & 3 deletions components/board/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ func CreateStatus(ctx context.Context, b Board, extra map[string]interface{}) (*
if names := b.AnalogNames(); len(names) != 0 {
status.Analogs = make(map[string]*commonpb.AnalogStatus, len(names))
for _, name := range names {
x, ok := b.AnalogByName(name)
if !ok {
return nil, fmt.Errorf("analog %q not found", name)
x, err := b.AnalogByName(name)
if err != nil {
return nil, err
}
val, err := x.Read(ctx, extra)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions components/gripper/softrobotics/gripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ func newGripper(b board.Board, conf resource.Config, logger logging.Logger) (gri
return nil, err
}

psi, ok := b.AnalogByName("psi")
if !ok {
return nil, errors.New("failed to find analog reader 'psi'")
psi, err := b.AnalogByName("psi")
if err != nil {
return nil, err
}
pinOpen, err := b.GPIOPinByName(newConf.Open)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions components/input/gpio/gpio.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@ func (c *Controller) newButton(ctx context.Context, brd board.Board, intName str
}

func (c *Controller) newAxis(ctx context.Context, brd board.Board, analogName string, cfg AxisConfig) error {
reader, ok := brd.AnalogByName(analogName)
if !ok {
return fmt.Errorf("can't find AnalogReader (%s)", analogName)
reader, err := brd.AnalogByName(analogName)
if err != nil {
return err
}

c.activeBackgroundWorkers.Add(1)
Expand Down
4 changes: 2 additions & 2 deletions robot/impl/resource_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,8 @@ func TestManagerAdd(t *testing.T) {
injectBoard.DigitalInterruptNamesFunc = func() []string {
return []string{"digital1"}
}
injectBoard.AnaloByNameFunc = func(name string) (board.Analog, bool) {
return &fakeboard.Analog{}, true
injectBoard.AnalogByNameFunc = func(name string) (board.Analog, error) {
return &fakeboard.Analog{}, nil
}
injectBoard.DigitalInterruptByNameFunc = func(name string) (board.DigitalInterrupt, bool) {
return &pinwrappers.BasicDigitalInterrupt{}, true
Expand Down
8 changes: 4 additions & 4 deletions testutils/inject/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Board struct {
board.Board
name resource.Name
DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error)
AnaloByNameFunc func(name string) (board.Analog, bool)
AnalogByNameFunc func(name string) (board.Analog, error)
analogByNameCap []interface{}
DigitalInterruptByNameFunc func(name string) (board.DigitalInterrupt, bool)
digitalInterruptByNameCap []interface{}
Expand All @@ -43,12 +43,12 @@ func (b *Board) Name() resource.Name {
}

// AnalogByName calls the injected AnalogByName or the real version.
func (b *Board) AnalogByName(name string) (board.Analog, bool) {
func (b *Board) AnalogByName(name string) (board.Analog, error) {
b.analogByNameCap = []interface{}{name}
if b.AnaloByNameFunc == nil {
if b.AnalogByNameFunc == nil {
return b.Board.AnalogByName(name)
}
return b.AnaloByNameFunc(name)
return b.AnalogByNameFunc(name)
}

// AnalogByNameCap returns the last parameters received by AnalogByName, and then clears them.
Expand Down

0 comments on commit 783b165

Please sign in to comment.