[RSDK-7322] Move analog writers from Board interface to Analog interf…

…ace (#3916) Everything compiles, the linter is happy, and the fake board works. but that's as far as I've tested; I don't have an analog writer to try this out with.
viamrobotics · May 8, 2024 · d3f3bff · d3f3bff
1 parent ed2f36c
commit d3f3bff
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 65 deletions.
diff --git a/components/board/board.go b/components/board/board.go
@@ -69,9 +69,6 @@ type Board interface {
 	// the specified duration.
 	SetPowerMode(ctx context.Context, mode pb.PowerMode, duration *time.Duration) error
 
-	// WriteAnalog writes an analog value to a pin on the board.
-	WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error
-
 	// StreamTicks starts a stream of digital interrupt ticks.
 	StreamTicks(ctx context.Context, interrupts []DigitalInterrupt, ch chan Tick,
 		extra map[string]interface{}) error

diff --git a/components/board/client.go b/components/board/client.go
@@ -126,22 +126,6 @@ func (c *client) DoCommand(ctx context.Context, cmd map[string]interface{}) (map
 	return rprotoutils.DoFromResourceClient(ctx, c.client, c.info.name, cmd)
 }
 
-// WriteAnalog writes the analog value to the specified pin.
-func (c *client) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	ext, err := protoutils.StructToStructPb(extra)
-	if err != nil {
-		return err
-	}
-	_, err = c.client.WriteAnalog(ctx, &pb.WriteAnalogRequest{
-		Name:  c.info.name,
-		Pin:   pin,
-		Value: value,
-		Extra: ext,
-	})
-
-	return err
-}
-
 // analogClient satisfies a gRPC based board.AnalogReader. Refer to the interface
 // for descriptions of its methods.
 type analogClient struct {
@@ -155,7 +139,7 @@ func (ac *analogClient) Read(ctx context.Context, extra map[string]interface{})
 	if err != nil {
 		return 0, err
 	}
-	// the api method is named ReadAnalogReader, it is named differenlty than
+	// the api method is named ReadAnalogReader, it is named differently than
 	// the board interface functions.
 	resp, err := ac.client.client.ReadAnalogReader(ctx, &pb.ReadAnalogReaderRequest{
 		BoardName:        ac.boardName,
@@ -169,7 +153,20 @@ func (ac *analogClient) Read(ctx context.Context, extra map[string]interface{})
 }
 
 func (ac *analogClient) Write(ctx context.Context, value int, extra map[string]interface{}) error {
-	return errors.New("unimplemented")
+	ext, err := protoutils.StructToStructPb(extra)
+	if err != nil {
+		return err
+	}
+	_, err = ac.client.client.WriteAnalog(ctx, &pb.WriteAnalogRequest{
+		Name:  ac.boardName,
+		Pin:   ac.analogName,
+		Value: int32(value),
+		Extra: ext,
+	})
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 // digitalInterruptClient satisfies a gRPC based board.DigitalInterrupt. Refer to the

diff --git a/components/board/client_test.go b/components/board/client_test.go
@@ -152,11 +152,13 @@ func TestWorkingClient(t *testing.T) {
 		actualExtra = nil
 
 		// write analog
-		injectBoard.WriteAnalogFunc = func(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
+		injectAnalog.WriteFunc = func(ctx context.Context, value int, extra map[string]interface{}) error {
 			actualExtra = extra
 			return nil
 		}
-		err = injectBoard.WriteAnalog(context.Background(), "pin1", 6, expectedExtra)
+		analog2, err := injectBoard.AnalogByName("pin1")
+		test.That(t, err, test.ShouldBeNil)
+		err = analog2.Write(context.Background(), 6, expectedExtra)
 		test.That(t, err, test.ShouldBeNil)
 		test.That(t, actualExtra, test.ShouldResemble, expectedExtra)
 		actualExtra = nil

diff --git a/components/board/fake/board.go b/components/board/fake/board.go
@@ -19,9 +19,10 @@ import (
 	"go.viam.com/rdk/resource"
 )
 
-// In order to maintain test functionality, testPin will always return an analog value of 0.
-// To see non-zero fake analog values on a fake board, add an analog reader to any other pin.
-var analogTestPin = ""
+// In order to maintain test functionality, testPin will always return the analog value it is set
+// to (defaults to 0 before being set). To see changing fake analog values on a fake board, add an
+// analog reader to any other pin.
+var analogTestPin = "1"
 
 // In order to maintain test functionality, digital interrtups on any pin except nonZeroInterruptPin
 // will always return a digital interrupt value of 0. To see non-zero fake interrupt values on a fake board,
@@ -228,13 +229,6 @@ func (b *Board) SetPowerMode(ctx context.Context, mode pb.PowerMode, duration *t
 	return grpc.UnimplementedError
 }
 
-// WriteAnalog writes the value to the given pin, which can be read back by adding it to AnalogReaders.
-func (b *Board) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	alg := &Analog{pin: pin, Value: int(value)}
-	b.Analogs[pin] = alg
-	return nil
-}
-
 // StreamTicks starts a stream of digital interrupt ticks.
 func (b *Board) StreamTicks(ctx context.Context, interrupts []board.DigitalInterrupt, ch chan board.Tick,
 	extra map[string]interface{},

diff --git a/components/board/genericlinux/board.go b/components/board/genericlinux/board.go
@@ -487,11 +487,6 @@ func (b *Board) SetPowerMode(
 	return grpc.UnimplementedError
 }
 
-// WriteAnalog writes the value to the given pin.
-func (b *Board) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	return nil
-}
-
 // StreamTicks starts a stream of digital interrupt ticks.
 func (b *Board) StreamTicks(ctx context.Context, interrupts []board.DigitalInterrupt, ch chan board.Tick,
 	extra map[string]interface{},

diff --git a/components/board/hat/pca9685/pca9685.go b/components/board/hat/pca9685/pca9685.go
@@ -157,11 +157,6 @@ func (pca *PCA9685) SetPowerMode(ctx context.Context, mode pb.PowerMode, duratio
 	return grpc.UnimplementedError
 }
 
-// WriteAnalog writes the value to the given pin.
-func (pca *PCA9685) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	return grpc.UnimplementedError
-}
-
 // GPIOPinByName returns a GPIOPin by name.
 func (pca *PCA9685) GPIOPinByName(pin string) (board.GPIOPin, error) {
 	pinInt, err := pca.parsePin(pin)

diff --git a/components/board/numato/board.go b/components/board/numato/board.go
@@ -323,11 +323,6 @@ func (b *numatoBoard) SetPowerMode(ctx context.Context, mode pb.PowerMode, durat
 	return grpc.UnimplementedError
 }
 
-// WriteAnalog writes the value to the given pin.
-func (b *numatoBoard) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	return grpc.UnimplementedError
-}
-
 func (b *numatoBoard) Close(ctx context.Context) error {
 	atomic.AddInt32(&b.closed, 1)
 

diff --git a/components/board/pi/impl/board.go b/components/board/pi/impl/board.go
@@ -676,7 +676,7 @@ func (pi *piPigpio) AnalogByName(name string) (board.Analog, error) {
 	defer pi.mu.Unlock()
 	a, ok := pi.analogReaders[name]
 	if !ok {
-		return nil, errors.Errorf("can't find AnalogReader (%s)", name)
+		return nil, errors.Errorf("can't find Analog pin (%s)", name)
 	}
 	return a, nil
 }
@@ -721,11 +721,6 @@ func (pi *piPigpio) SetPowerMode(ctx context.Context, mode pb.PowerMode, duratio
 	return grpc.UnimplementedError
 }
 
-// WriteAnalog writes the value to the given pin.
-func (pi *piPigpio) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	return grpc.UnimplementedError
-}
-
 // Close attempts to close all parts of the board cleanly.
 func (pi *piPigpio) Close(ctx context.Context) error {
 	var terminate bool

diff --git a/components/board/server.go b/components/board/server.go
@@ -161,7 +161,12 @@ func (s *serviceServer) WriteAnalog(
 		return nil, err
 	}
 
-	err = b.WriteAnalog(ctx, req.Pin, req.Value, req.Extra.AsMap())
+	analog, err := b.AnalogByName(req.Pin)
+	if err != nil {
+		return nil, err
+	}
+
+	err = analog.Write(ctx, int(req.Value), req.Extra.AsMap())
 	if err != nil {
 		return nil, err
 	}

diff --git a/components/board/server_test.go b/components/board/server_test.go
@@ -597,10 +597,15 @@ func TestServerWriteAnalog(t *testing.T) {
 			test.That(t, err, test.ShouldBeNil)
 			var actualExtra map[string]interface{}
 
-			injectBoard.WriteAnalogFunc = func(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
+			injectAnalog := inject.Analog{}
+			injectAnalog.WriteFunc = func(ctx context.Context, value int, extra map[string]interface{}) error {
 				actualExtra = extra
 				return tc.injectErr
 			}
+			injectBoard.AnalogByNameFunc = func(pin string) (board.Analog, error) {
+				return &injectAnalog, nil
+			}
+
 			resp, err := server.WriteAnalog(ctx, tc.req)
 			if tc.expRespErr == "" {
 				test.That(t, err, test.ShouldBeNil)

diff --git a/testutils/inject/board.go b/testutils/inject/board.go
@@ -25,7 +25,6 @@ type Board struct {
 	DigitalInterruptNamesFunc  func() []string
 	CloseFunc                  func(ctx context.Context) error
 	SetPowerModeFunc           func(ctx context.Context, mode boardpb.PowerMode, duration *time.Duration) error
-	WriteAnalogFunc            func(ctx context.Context, pin string, value int32, extra map[string]interface{}) error
 	StreamTicksFunc            func(ctx context.Context,
 		interrupts []board.DigitalInterrupt, ch chan board.Tick, extra map[string]interface{}) error
 }
@@ -139,14 +138,6 @@ func (b *Board) SetPowerMode(ctx context.Context, mode boardpb.PowerMode, durati
 	return b.SetPowerModeFunc(ctx, mode, duration)
 }
 
-// WriteAnalog calls the injected WriteAnalog or the real version.
-func (b *Board) WriteAnalog(ctx context.Context, pin string, value int32, extra map[string]interface{}) error {
-	if b.WriteAnalogFunc == nil {
-		return b.Board.WriteAnalog(ctx, pin, value, extra)
-	}
-	return b.WriteAnalogFunc(ctx, pin, value, extra)
-}
-
 // StreamTicks calls the injected StreamTicks or the real version.
 func (b *Board) StreamTicks(ctx context.Context,
 	interrupts []board.DigitalInterrupt, ch chan board.Tick, extra map[string]interface{},