APP-4940 - Update fake encoder config to have ticks_per_sec parameter

components/encoder/fake/encoder.go

@@ -12,6 +12,7 @@ import (
 	"go.viam.com/rdk/components/encoder"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/resource"
+	rdkutils "go.viam.com/rdk/utils"
 )
 
 var fakeModel = resource.DefaultModelFamily.WithModel("fake")
@@ -44,8 +45,8 @@ func NewEncoder(
 	if err := e.Reconfigure(ctx, nil, cfg); err != nil {
 		return nil, err
 	}
+	e.workers = rdkutils.NewStoppableWorkers(e.start)
 
-	e.start(ctx)
 	return e, nil
 }
 
@@ -59,6 +60,7 @@ func (e *fakeEncoder) Reconfigure(
 		return err
 	}
 	e.mu.Lock()
+
 	e.updateRate = newConf.UpdateRate
 	if e.updateRate == 0 {
 		e.updateRate = 100
@@ -80,16 +82,14 @@ func (cfg *Config) Validate(path string) ([]string, error) {
 // fakeEncoder keeps track of a fake motor position.
 type fakeEncoder struct {
 	resource.Named
-	resource.TriviallyCloseable
 
-	positionType            encoder.PositionType
-	activeBackgroundWorkers sync.WaitGroup
-	logger                  logging.Logger
+	positionType encoder.PositionType
+	logger       logging.Logger
 
 	mu         sync.RWMutex
+	workers    rdkutils.StoppableWorkers
 	position   int64
-	speed      float64 // ticks per minute
-	updateRate int64   // update position in start every updateRate ms
+	updateRate int64 // update position in start every updateRate ms
 }
 
 // Position returns the current position in terms of ticks or
@@ -109,35 +109,32 @@ func (e *fakeEncoder) Position(
 
 // Start starts a background thread to run the encoder.
 func (e *fakeEncoder) start(cancelCtx context.Context) {
-	e.activeBackgroundWorkers.Add(1)
-	utils.ManagedGo(func() {
-		for {
-			select {
-			case <-cancelCtx.Done():
-				return
-			default:
-			}
-
-			e.mu.RLock()
-			updateRate := e.updateRate
-			e.mu.RUnlock()
-			if !utils.SelectContextOrWait(cancelCtx, time.Duration(updateRate)*time.Millisecond) {
-				return
-			}
-
-			e.mu.Lock()
-			e.position += int64(e.speed / float64(60*1000/updateRate))
-			e.mu.Unlock()
+	for {
+		select {
+		case <-cancelCtx.Done():
+			return
+		default:
+		}
+
+		e.mu.RLock()
+		updateRate := e.updateRate
+		e.mu.RUnlock()
+		if !utils.SelectContextOrWait(cancelCtx, time.Duration(updateRate)*time.Millisecond) {
+			return
 		}
-	}, e.activeBackgroundWorkers.Done)
+
+		e.mu.Lock()
+		e.position++
+		e.mu.Unlock()
+	}
 }
 
 // ResetPosition sets the current position of the motor (adjusted by a given offset)
 // to be its new zero position.
 func (e *fakeEncoder) ResetPosition(ctx context.Context, extra map[string]interface{}) error {
 	e.mu.Lock()
 	defer e.mu.Unlock()
-	e.position = int64(0)
+	e.position = 0
 	return nil
 }
 
@@ -149,21 +146,20 @@ func (e *fakeEncoder) Properties(ctx context.Context, extra map[string]interface
 	}, nil
 }
 
+// Close safely shuts down the fake encoder.
+func (e *fakeEncoder) Close(ctx context.Context) error {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	e.workers.Stop()
+	return nil
+}
+
 // Encoder is a fake encoder used for testing.
 type Encoder interface {
 	encoder.Encoder
-	SetSpeed(ctx context.Context, speed float64) error
 	SetPosition(ctx context.Context, position int64) error
 }
 
-// SetSpeed sets the speed of the fake motor the encoder is measuring.
-func (e *fakeEncoder) SetSpeed(ctx context.Context, speed float64) error {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	e.speed = speed
-	return nil
-}
-
 // SetPosition sets the position of the encoder.
 func (e *fakeEncoder) SetPosition(ctx context.Context, position int64) error {
 	e.mu.Lock()

components/encoder/fake/encoder_test.go

@@ -21,6 +21,8 @@ func TestEncoder(t *testing.T) {
 	logger := logging.NewTestLogger(t)
 	e, _ := NewEncoder(ctx, cfg, logger)
 
+	t.Cleanup(func() { e.Close(context.Background()) })
+
 	// Get and set position
 	t.Run("get and set position", func(t *testing.T) {
 		pos, positionType, err := e.Position(ctx, encoder.PositionTypeUnspecified, nil)
@@ -67,28 +69,15 @@ func TestEncoder(t *testing.T) {
 		})
 	})
 
-	// Set Speed
-	t.Run("set speed", func(t *testing.T) {
-		e1 := e.(*fakeEncoder)
-		err := e1.SetSpeed(ctx, 1)
-		test.That(t, err, test.ShouldBeNil)
-		test.That(t, e1.speed, test.ShouldEqual, 1)
-	})
-
 	// Start with default update rate
 	t.Run("start default update rate", func(t *testing.T) {
 		e1 := e.(*fakeEncoder)
-		err := e1.SetSpeed(ctx, 0)
-		test.That(t, err, test.ShouldBeNil)
 
 		testutils.WaitForAssertion(t, func(tb testing.TB) {
 			tb.Helper()
 			test.That(t, e1.updateRate, test.ShouldEqual, 100)
 		})
 
-		err = e1.SetSpeed(ctx, 600)
-		test.That(t, err, test.ShouldBeNil)
-
 		testutils.WaitForAssertion(t, func(tb testing.TB) {
 			tb.Helper()
 			pos, _, err := e.Position(ctx, encoder.PositionTypeUnspecified, nil)

components/motor/fake/motor.go

@@ -209,17 +209,6 @@ func (m *Motor) SetPower(ctx context.Context, powerPct float64, extra map[string
 	m.Logger.CDebugf(ctx, "Motor SetPower %f", powerPct)
 	m.setPowerPct(powerPct)
 
-	if m.Encoder != nil {
-		if m.TicksPerRotation <= 0 {
-			return errors.New("need positive nonzero TicksPerRotation")
-		}
-
-		newSpeed := (m.MaxRPM * m.powerPct) * float64(m.TicksPerRotation)
-		err := m.Encoder.SetSpeed(ctx, newSpeed)
-		if err != nil {
-			return err
-		}
-	}
 	return nil
 }
 
@@ -373,12 +362,6 @@ func (m *Motor) Stop(ctx context.Context, extra map[string]interface{}) error {
 
 	m.Logger.CDebug(ctx, "Motor Stopped")
 	m.setPowerPct(0.0)
-	if m.Encoder != nil {
-		err := m.Encoder.SetSpeed(ctx, 0.0)
-		if err != nil {
-			return errors.Wrapf(err, "error in Stop from motor (%s)", m.Name())
-		}
-	}
 	return nil
 }