[RSDK-7335] Remove Close() from and Add Write() to Analog interface i…

…n board (#3809)
viamrobotics · Apr 18, 2024 · 8e74013 · 8e74013
1 parent dc2ea5f
commit 8e74013
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 20 deletions.
diff --git a/components/board/board.go b/components/board/board.go
@@ -89,7 +89,9 @@ type Board interface {
 type Analog interface {
 	// Read reads off the current value.
 	Read(ctx context.Context, extra map[string]interface{}) (int, error)
-	Close(ctx context.Context) error
+
+	// Write writes a value to the analog pin.
+	Write(ctx context.Context, value int, extra map[string]interface{}) error
 }
 
 // FromDependencies is a helper for getting the named board from a collection of

diff --git a/components/board/client.go b/components/board/client.go
@@ -205,16 +205,16 @@ type analogClient struct {
 	analogName string
 }
 
-func (arc *analogClient) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
+func (ac *analogClient) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
 	ext, err := protoutils.StructToStructPb(extra)
 	if err != nil {
 		return 0, err
 	}
 	// the api method is named ReadAnalogReader, it is named differenlty than
 	// the board interface functions.
-	resp, err := arc.client.client.ReadAnalogReader(ctx, &pb.ReadAnalogReaderRequest{
-		BoardName:        arc.boardName,
-		AnalogReaderName: arc.analogName,
+	resp, err := ac.client.client.ReadAnalogReader(ctx, &pb.ReadAnalogReaderRequest{
+		BoardName:        ac.boardName,
+		AnalogReaderName: ac.analogName,
 		Extra:            ext,
 	})
 	if err != nil {
@@ -223,6 +223,10 @@ func (arc *analogClient) Read(ctx context.Context, extra map[string]interface{})
 	return int(resp.Value), nil
 }
 
+func (ac *analogClient) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return errors.New("unimplemented")
+}
+
 // digitalInterruptClient satisfies a gRPC based board.DigitalInterrupt. Refer to the
 // interface for descriptions of its methods.
 type digitalInterruptClient struct {

diff --git a/components/board/fake/board.go b/components/board/fake/board.go
@@ -289,6 +289,11 @@ func (a *Analog) Read(ctx context.Context, extra map[string]interface{}) (int, e
 	return a.Value, nil
 }
 
+func (a *Analog) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	a.Set(value)
+	return nil
+}
+
 // Set is used during testing.
 func (a *Analog) Set(value int) {
 	a.Mu.Lock()

diff --git a/components/board/genericlinux/board.go b/components/board/genericlinux/board.go
@@ -375,6 +375,10 @@ func (a *wrappedAnalogReader) reset(ctx context.Context, chipSelect string, read
 	a.chipSelect = chipSelect
 }
 
+func (a *wrappedAnalogReader) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return grpc.UnimplementedError
+}
+
 // Board implements a component for a Linux machine.
 type Board struct {
 	resource.Named

diff --git a/components/board/mcp3008helper/mcp3008.go b/components/board/mcp3008helper/mcp3008.go
@@ -8,6 +8,7 @@ import (
 	"go.uber.org/multierr"
 
 	"go.viam.com/rdk/components/board/genericlinux/buses"
+	"go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/resource"
 )
 
@@ -65,3 +66,7 @@ func (mar *MCP3008AnalogReader) Read(ctx context.Context, extra map[string]inter
 func (mar *MCP3008AnalogReader) Close(ctx context.Context) error {
 	return nil
 }
+
+func (mar *MCP3008AnalogReader) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return grpc.UnimplementedError
+}
diff --git a/components/board/numato/board.go b/components/board/numato/board.go
@@ -104,7 +104,7 @@ type numatoBoard struct {
 	resource.Named
 	resource.AlwaysRebuild
 	pins    int
-	analogs map[string]board.Analog
+	analogs map[string]*pinwrappers.AnalogSmoother
 
 	port   io.ReadWriteCloser
 	closed int32
@@ -363,16 +363,16 @@ type analog struct {
 	pin string
 }
 
-func (ar *analog) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
-	res, err := ar.b.doSendReceive(ctx, fmt.Sprintf("adc read %s", ar.pin))
+func (a *analog) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
+	res, err := a.b.doSendReceive(ctx, fmt.Sprintf("adc read %s", a.pin))
 	if err != nil {
 		return 0, err
 	}
 	return strconv.Atoi(res)
 }
 
-func (ar *analog) Close(ctx context.Context) error {
-	return nil
+func (a *analog) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return grpc.UnimplementedError
 }
 
 func connect(ctx context.Context, name resource.Name, conf *Config, logger logging.Logger) (board.Board, error) {
@@ -407,7 +407,7 @@ func connect(ctx context.Context, name resource.Name, conf *Config, logger loggi
 		logger: logger,
 	}
 
-	b.analogs = map[string]board.Analog{}
+	b.analogs = map[string]*pinwrappers.AnalogSmoother{}
 	for _, c := range conf.Analogs {
 		r := &analog{b, c.Pin}
 		b.analogs[c.Name] = pinwrappers.SmoothAnalogReader(r, c, logger)

diff --git a/components/board/pi/impl/board.go b/components/board/pi/impl/board.go
@@ -96,7 +96,7 @@ type piPigpio struct {
 	cancelFunc    context.CancelFunc
 	duty          int // added for mutex
 	gpioConfigSet map[int]bool
-	analogReaders map[string]board.Analog
+	analogReaders map[string]*pinwrappers.AnalogSmoother
 	// `interrupts` maps interrupt names to the interrupts. `interruptsHW` maps broadcom addresses
 	// to these same values. The two should always have the same set of values.
 	interrupts   map[string]ReconfigurableDigitalInterrupt
@@ -222,7 +222,7 @@ func (pi *piPigpio) StreamTicks(ctx context.Context, interruptNames []string, ch
 
 func (pi *piPigpio) reconfigureAnalogReaders(ctx context.Context, cfg *Config) error {
 	// No need to reconfigure the old analog readers; just throw them out and make new ones.
-	pi.analogReaders = map[string]board.Analog{}
+	pi.analogReaders = map[string]*pinwrappers.AnalogSmoother{}
 	for _, ac := range cfg.AnalogReaders {
 		channel, err := strconv.Atoi(ac.Pin)
 		if err != nil {
@@ -737,7 +737,7 @@ func (pi *piPigpio) Close(ctx context.Context) error {
 	for _, analog := range pi.analogReaders {
 		err = multierr.Combine(err, analog.Close(ctx))
 	}
-	pi.analogReaders = map[string]board.Analog{}
+	pi.analogReaders = map[string]*pinwrappers.AnalogSmoother{}
 
 	for bcom, interrupt := range pi.interruptsHW {
 		err = multierr.Combine(err, interrupt.Close(ctx))

diff --git a/components/board/pinwrappers/analog_smoother_test.go b/components/board/pinwrappers/analog_smoother_test.go
@@ -11,18 +11,19 @@ import (
 	"go.viam.com/utils/testutils"
 
 	"go.viam.com/rdk/components/board"
+	"go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/logging"
 )
 
-type testReader struct {
+type testAnalog struct {
 	mu   sync.Mutex
 	r    *rand.Rand
 	n    int64
 	lim  int64
 	stop bool
 }
 
-func (t *testReader) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
+func (t *testAnalog) Read(ctx context.Context, extra map[string]interface{}) (int, error) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	if t.stop || t.n >= t.lim {
@@ -32,12 +33,16 @@ func (t *testReader) Read(ctx context.Context, extra map[string]interface{}) (in
 	return t.r.Intn(100), nil
 }
 
-func (t *testReader) Close(ctx context.Context) error {
+func (t *testAnalog) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return grpc.UnimplementedError
+}
+
+func (t *testAnalog) Close(ctx context.Context) error {
 	return nil
 }
 
 func TestAnalogSmoother1(t *testing.T) {
-	testReader := testReader{
+	testReader := testAnalog{
 		r:   rand.New(rand.NewSource(11)),
 		lim: 200,
 	}

diff --git a/components/board/pinwrappers/analogs.go b/components/board/pinwrappers/analogs.go
@@ -10,6 +10,7 @@ import (
 	goutils "go.viam.com/utils"
 
 	"go.viam.com/rdk/components/board"
+	"go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/utils"
 )
@@ -140,3 +141,7 @@ func (as *AnalogSmoother) Start(ctx context.Context) {
 		}
 	}, as.activeBackgroundWorkers.Done)
 }
+
+func (as *AnalogSmoother) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	return grpc.UnimplementedError
+}
diff --git a/testutils/inject/analog.go b/testutils/inject/analog.go
@@ -9,8 +9,10 @@ import (
 // Analog is an injected analog pin.
 type Analog struct {
 	board.Analog
-	ReadFunc func(ctx context.Context, extra map[string]interface{}) (int, error)
-	readCap  []interface{}
+	ReadFunc  func(ctx context.Context, extra map[string]interface{}) (int, error)
+	readCap   []interface{}
+	WriteFunc func(ctx context.Context, value int, extra map[string]interface{}) error
+	writeCap  []interface{}
 }
 
 // Read calls the injected Read or the real version.
@@ -30,3 +32,21 @@ func (a *Analog) ReadCap() []interface{} {
 	defer func() { a.readCap = nil }()
 	return a.readCap
 }
+
+// Write calls the injected Write or the real version.
+func (a *Analog) Write(ctx context.Context, value int, extra map[string]interface{}) error {
+	a.writeCap = []interface{}{ctx, value}
+	if a.WriteFunc == nil {
+		return a.Analog.Write(ctx, value, extra)
+	}
+	return a.WriteFunc(ctx, value, extra)
+}
+
+// WriteCap returns the last parameters received by Write, and then clears them.
+func (a *Analog) WriteCap() []interface{} {
+	if a == nil {
+		return nil
+	}
+	defer func() { a.writeCap = nil }()
+	return a.writeCap
+}