[RSDK-1963] servo digital interrupts are only in the raspberry pi code (viamrobotics#3369)

I had this much, much cleaner, where the only part left in the `board` package was the interface, and all the implementation details were left up to the boards. but then it turned out that a bunch of unit tests were using these as well, and I had to put a bunch of stuff back.

Main changes:
- The `PostProcessor` type is gone. The `Formula` field in the interrupt config is gone.
- There is a copy of all the digital interrupt stuff (including the configs) in board/pi/impl/ now. That component uses this new copy.
- The main (non-pi) version of digital interrupts no longer has a `ServoDigitalInterrupt` implementation.
- The main (non-pi) version of digital interrupt configs no longer takes a Type.

Author: penguinland

components/board/board.go

@@ -88,11 +88,6 @@ type AnalogReader interface {
 	Close(ctx context.Context) error
 }
 
-// A PostProcessor takes a raw input and transforms it into a new value.
-// Multiple post processors can be stacked on each other. This is currently
-// only used in DigitalInterrupt readings.
-type PostProcessor func(raw int64) int64
-
 // FromDependencies is a helper for getting the named board from a collection of
 // dependencies.
 func FromDependencies(deps resource.Dependencies, name string) (Board, error) {

components/board/client.go

@@ -253,10 +253,6 @@ func (dic *digitalInterruptClient) RemoveCallback(c chan Tick) {
 	panic(errUnimplemented)
 }
 
-func (dic *digitalInterruptClient) AddPostProcessor(pp PostProcessor) {
-	panic(errUnimplemented)
-}
-
 // gpioPinClient satisfies a gRPC based board.GPIOPin. Refer to the interface
 // for descriptions of its methods.
 type gpioPinClient struct {

components/board/config.go

@@ -51,10 +51,8 @@ func (config *AnalogReaderConfig) Validate(path string) error {
 
 // DigitalInterruptConfig describes the configuration of digital interrupt for a board.
 type DigitalInterruptConfig struct {
-	Name    string `json:"name"`
-	Pin     string `json:"pin"`
-	Type    string `json:"type,omitempty"` // e.g. basic, servo
-	Formula string `json:"formula,omitempty"`
+	Name string `json:"name"`
+	Pin  string `json:"pin"`
 }
 
 // Validate ensures all parts of the config are valid.

components/board/digital_interrupts.go

@@ -5,10 +5,7 @@ import (
 	"sync"
 	"sync/atomic"
 
-	"github.com/erh/scheme"
 	"github.com/pkg/errors"
-
-	"go.viam.com/rdk/utils"
 )
 
 // ServoRollingAverageWindow is how many entries to average over for
@@ -27,8 +24,7 @@ type Tick struct {
 }
 
 // A DigitalInterrupt represents a configured interrupt on the board that
-// when interrupted, calls the added callbacks. Post processors can also
-// be added to modify what Value ultimately returns.
+// when interrupted, calls the added callbacks.
 type DigitalInterrupt interface {
 	// Value returns the current value of the interrupt which is
 	// based on the type of interrupt.
@@ -44,10 +40,6 @@ type DigitalInterrupt interface {
 	// happens.
 	AddCallback(c chan Tick)
 
-	// AddPostProcessor adds a post processor that should be used to modify
-	// what is returned by Value.
-	AddPostProcessor(pp PostProcessor)
-
 	// RemoveCallback removes a listener for interrupts
 	RemoveCallback(c chan Tick)
 
@@ -64,19 +56,7 @@ type ReconfigurableDigitalInterrupt interface {
 // CreateDigitalInterrupt is a factory method for creating a specific DigitalInterrupt based
 // on the given config. If no type is specified, a BasicDigitalInterrupt is returned.
 func CreateDigitalInterrupt(cfg DigitalInterruptConfig) (ReconfigurableDigitalInterrupt, error) {
-	if cfg.Type == "" {
-		cfg.Type = "basic"
-	}
-
-	var i ReconfigurableDigitalInterrupt
-	switch cfg.Type {
-	case "basic":
-		i = &BasicDigitalInterrupt{}
-	case "servo":
-		i = &ServoDigitalInterrupt{ra: utils.NewRollingAverage(ServoRollingAverageWindow)}
-	default:
-		panic(errors.Errorf("unknown interrupt type (%s)", cfg.Type))
-	}
+	i := &BasicDigitalInterrupt{}
 
 	if err := i.Reconfigure(cfg); err != nil {
 		return nil, err
@@ -93,24 +73,13 @@ type BasicDigitalInterrupt struct {
 
 	mu  sync.RWMutex
 	cfg DigitalInterruptConfig
-	pp  PostProcessor
-}
-
-// Config returns the config used to create this interrupt.
-func (i *BasicDigitalInterrupt) Config(ctx context.Context) (DigitalInterruptConfig, error) {
-	i.mu.RLock()
-	defer i.mu.RUnlock()
-	return i.cfg, nil
 }
 
 // Value returns the amount of ticks that have occurred.
 func (i *BasicDigitalInterrupt) Value(ctx context.Context, extra map[string]interface{}) (int64, error) {
 	i.mu.RLock()
 	defer i.mu.RUnlock()
 	count := atomic.LoadInt64(&i.count)
-	if i.pp != nil {
-		return i.pp(count), nil
-	}
 	return count, nil
 }
 
@@ -165,162 +134,16 @@ func (i *BasicDigitalInterrupt) RemoveCallback(c chan Tick) {
 	}
 }
 
-// AddPostProcessor sets the post processor that will modify the value that
-// Value returns.
-func (i *BasicDigitalInterrupt) AddPostProcessor(pp PostProcessor) {
-	i.mu.Lock()
-	defer i.mu.Unlock()
-	i.pp = pp
-}
-
 // Close does nothing.
 func (i *BasicDigitalInterrupt) Close(ctx context.Context) error {
 	return nil
 }
 
-func processFormula(oldFormula, newFormula, name string) (func(raw int64) int64, bool, error) {
-	if oldFormula == newFormula {
-		return nil, false, nil
-	}
-	x, err := scheme.Parse(newFormula)
-	if err != nil {
-		return nil, false, errors.Wrapf(err, "couldn't parse formula for %s", name)
-	}
-
-	testScope := scheme.Scope{}
-	num := 1.0
-	testScope["raw"] = &scheme.Value{Float: &num}
-	_, err = scheme.Eval(x, testScope)
-	if err != nil {
-		return nil, false, errors.Wrapf(err, "test exec failed for %s", name)
-	}
-
-	return func(raw int64) int64 {
-		scope := scheme.Scope{}
-		rr := float64(raw) // TODO(erh): fix
-		scope["raw"] = &scheme.Value{Float: &rr}
-		res, err := scheme.Eval(x, scope)
-		if err != nil {
-			panic(err)
-		}
-		f, err := res.ToFloat()
-		if err != nil {
-			panic(err)
-		}
-		return int64(f)
-	}, true, nil
-}
-
 // Reconfigure reconfigures this digital interrupt with a new formula.
 func (i *BasicDigitalInterrupt) Reconfigure(conf DigitalInterruptConfig) error {
 	i.mu.Lock()
 	defer i.mu.Unlock()
 
-	newFormula, isNew, err := processFormula(i.cfg.Formula, conf.Formula, conf.Name)
-	if err != nil {
-		return err
-	}
-	if !isNew {
-		return nil
-	}
-	i.pp = newFormula
 	i.cfg = conf
 	return nil
 }
-
-// A ServoDigitalInterrupt is an interrupt associated with a servo in order to
-// track the amount of time that has passed between low signals (pulse width). Post processors
-// make meaning of these widths.
-type ServoDigitalInterrupt struct {
-	last uint64
-	ra   *utils.RollingAverage
-
-	mu  sync.RWMutex
-	cfg DigitalInterruptConfig
-	pp  PostProcessor
-}
-
-// Config returns the config the interrupt was created with.
-func (i *ServoDigitalInterrupt) Config(ctx context.Context) (DigitalInterruptConfig, error) {
-	i.mu.RLock()
-	defer i.mu.RUnlock()
-	return i.cfg, nil
-}
-
-// Value will return the window averaged value followed by its post processed
-// result.
-func (i *ServoDigitalInterrupt) Value(ctx context.Context, extra map[string]interface{}) (int64, error) {
-	i.mu.RLock()
-	defer i.mu.RUnlock()
-	v := int64(i.ra.Average())
-	if i.pp != nil {
-		return i.pp(v), nil
-	}
-
-	return v, nil
-}
-
-// Tick records the time between two successive low signals (pulse width). How it is
-// interpreted is based off the consumer of Value.
-func (i *ServoDigitalInterrupt) Tick(ctx context.Context, high bool, now uint64) error {
-	i.mu.RLock()
-	defer i.mu.RUnlock()
-	diff := now - i.last
-	i.last = now
-
-	if i.last == 0 {
-		return nil
-	}
-
-	if high {
-		// this is time between signals, ignore
-		return nil
-	}
-
-	i.ra.Add(int(diff / 1000))
-	return nil
-}
-
-// AddCallback currently panics.
-func (i *ServoDigitalInterrupt) AddCallback(c chan Tick) {
-	i.mu.Lock()
-	defer i.mu.Unlock()
-	panic("servos can't have callback")
-}
-
-// RemoveCallback currently panics.
-func (i *ServoDigitalInterrupt) RemoveCallback(c chan Tick) {
-	i.mu.Lock()
-	defer i.mu.Unlock()
-	panic("servos can't have callback")
-}
-
-// AddPostProcessor sets the post processor that will modify the value that
-// Value returns.
-func (i *ServoDigitalInterrupt) AddPostProcessor(pp PostProcessor) {
-	i.mu.Lock()
-	defer i.mu.Unlock()
-	i.pp = pp
-}
-
-// Reconfigure reconfigures this digital interrupt with a new formula.
-func (i *ServoDigitalInterrupt) Reconfigure(conf DigitalInterruptConfig) error {
-	i.mu.Lock()
-	defer i.mu.Unlock()
-
-	newFormula, isNew, err := processFormula(i.cfg.Formula, conf.Formula, conf.Name)
-	if err != nil {
-		return err
-	}
-	if !isNew {
-		return nil
-	}
-	i.pp = newFormula
-	i.cfg = conf
-	return nil
-}
-
-// Close does nothing.
-func (i *ServoDigitalInterrupt) Close(ctx context.Context) error {
-	return nil
-}

components/board/digital_interrupts_test.go

@@ -15,24 +15,23 @@ func nowNanosecondsTest() uint64 {
 
 func TestBasicDigitalInterrupt1(t *testing.T) {
 	config := DigitalInterruptConfig{
-		Name:    "i1",
-		Formula: "(+ 1 raw)",
+		Name: "i1",
 	}
 
 	i, err := CreateDigitalInterrupt(config)
 	test.That(t, err, test.ShouldBeNil)
 
 	intVal, err := i.Value(context.Background(), nil)
 	test.That(t, err, test.ShouldBeNil)
-	test.That(t, intVal, test.ShouldEqual, int64(1))
+	test.That(t, intVal, test.ShouldEqual, int64(0))
 	test.That(t, i.Tick(context.Background(), true, nowNanosecondsTest()), test.ShouldBeNil)
 	intVal, err = i.Value(context.Background(), nil)
 	test.That(t, err, test.ShouldBeNil)
-	test.That(t, intVal, test.ShouldEqual, int64(2))
+	test.That(t, intVal, test.ShouldEqual, int64(1))
 	test.That(t, i.Tick(context.Background(), false, nowNanosecondsTest()), test.ShouldBeNil)
 	intVal, err = i.Value(context.Background(), nil)
 	test.That(t, err, test.ShouldBeNil)
-	test.That(t, intVal, test.ShouldEqual, int64(2))
+	test.That(t, intVal, test.ShouldEqual, int64(1))
 
 	c := make(chan Tick)
 	i.AddCallback(c)
@@ -149,48 +148,3 @@ func TestRemoveCallbackDigitalInterrupt(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, intVal, test.ShouldEqual, int64(3))
 }
-
-func TestServoInterrupt(t *testing.T) {
-	config := DigitalInterruptConfig{
-		Name: "s1",
-		Type: "servo",
-	}
-
-	s, err := CreateDigitalInterrupt(config)
-	test.That(t, err, test.ShouldBeNil)
-
-	now := uint64(0)
-	for i := 0; i < 20; i++ {
-		test.That(t, s.Tick(context.Background(), true, now), test.ShouldBeNil)
-		now += 1500 * 1000 // this is what we measure
-		test.That(t, s.Tick(context.Background(), false, now), test.ShouldBeNil)
-		now += 1000 * 1000 * 1000 // this is between measurements
-	}
-
-	intVal, err := s.Value(context.Background(), nil)
-	test.That(t, err, test.ShouldBeNil)
-	test.That(t, intVal, test.ShouldEqual, int64(1500))
-}
-
-func TestServoInterruptWithPP(t *testing.T) {
-	config := DigitalInterruptConfig{
-		Name:    "s1",
-		Type:    "servo",
-		Formula: "(+ 1 raw)",
-	}
-
-	s, err := CreateDigitalInterrupt(config)
-	test.That(t, err, test.ShouldBeNil)
-
-	now := uint64(0)
-	for i := 0; i < 20; i++ {
-		test.That(t, s.Tick(context.Background(), true, now), test.ShouldBeNil)
-		now += 1500 * 1000 // this is what we measure
-		test.That(t, s.Tick(context.Background(), false, now), test.ShouldBeNil)
-		now += 1000 * 1000 * 1000 // this is between measurements
-	}
-
-	intVal, err := s.Value(context.Background(), nil)
-	test.That(t, err, test.ShouldBeNil)
-	test.That(t, intVal, test.ShouldEqual, int64(1501))
-}

components/board/fake/board.go

@@ -349,7 +349,6 @@ type DigitalInterruptWrapper struct {
 	di        board.DigitalInterrupt
 	conf      board.DigitalInterruptConfig
 	callbacks map[chan board.Tick]struct{}
-	pps       []board.PostProcessor
 }
 
 // NewDigitalInterruptWrapper returns a new digital interrupt to be used for testing.
@@ -380,9 +379,6 @@ func (s *DigitalInterruptWrapper) reset(conf board.DigitalInterruptConfig) error
 		for c := range s.callbacks {
 			s.di.AddCallback(c)
 		}
-		for _, pp := range s.pps {
-			s.di.AddPostProcessor(pp)
-		}
 		return nil
 	}
 	// reconf
@@ -420,15 +416,6 @@ func (s *DigitalInterruptWrapper) AddCallback(c chan board.Tick) {
 	s.di.AddCallback(c)
 }
 
-// AddPostProcessor adds a post processor that should be used to modify
-// what is returned by Value.
-func (s *DigitalInterruptWrapper) AddPostProcessor(pp board.PostProcessor) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	s.pps = append(s.pps, pp)
-	s.di.AddPostProcessor(pp)
-}
-
 // RemoveCallback removes a listener for interrupts.
 func (s *DigitalInterruptWrapper) RemoveCallback(c chan board.Tick) {
 	s.mu.Lock()

components/board/fake/board_test.go

@@ -19,7 +19,7 @@ func TestFakeBoard(t *testing.T) {
 		},
 		DigitalInterrupts: []board.DigitalInterruptConfig{
 			{Name: "i1", Pin: "35"},
-			{Name: "i2", Pin: "31", Type: "servo"},
+			{Name: "i2", Pin: "31"},
 			{Name: "a", Pin: "38"},
 			{Name: "b", Pin: "40"},
 		},