Merge branch 'main' into add-foreign-service-docs

components/arm/arm.go

@@ -17,6 +17,7 @@ import (
 	"go.viam.com/rdk/referenceframe"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/robot"
+	"go.viam.com/rdk/robot/framesystem"
 	"go.viam.com/rdk/spatialmath"
 )
 
@@ -98,7 +99,7 @@ type Arm interface {
 	referenceframe.ModelFramer
 	resource.Shaped
 	resource.Actuator
-	referenceframe.InputEnabled
+	framesystem.InputEnabled
 
 	// EndPosition returns the current position of the arm.
 	EndPosition(ctx context.Context, extra map[string]interface{}) (spatialmath.Pose, error)

components/base/kinematicbase/kinematics.go

@@ -13,14 +13,15 @@ import (
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/motionplan"
 	"go.viam.com/rdk/referenceframe"
+	"go.viam.com/rdk/robot/framesystem"
 	"go.viam.com/rdk/services/motion"
 )
 
 // KinematicBase is an interface for Bases that also satisfy the ModelFramer and InputEnabled interfaces.
 type KinematicBase interface {
 	base.Base
 	motion.Localizer
-	referenceframe.InputEnabled
+	framesystem.InputEnabled
 
 	Kinematics() referenceframe.Frame
 	LocalizationFrame() referenceframe.Frame

components/gantry/gantry.go

@@ -13,6 +13,7 @@ import (
 	"go.viam.com/rdk/referenceframe"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/robot"
+	"go.viam.com/rdk/robot/framesystem"
 )
 
 func init() {
@@ -85,7 +86,7 @@ type Gantry interface {
 	resource.Resource
 	resource.Actuator
 	referenceframe.ModelFramer
-	referenceframe.InputEnabled
+	framesystem.InputEnabled
 
 	// Position returns the position in meters.
 	Position(ctx context.Context, extra map[string]interface{}) ([]float64, error)

components/gantry/singleaxis/singleaxis.go

@@ -295,7 +295,7 @@ func (g *singleAxis) moveAway(ctx context.Context, pin int) error {
 	if pin != 0 {
 		dir = -1.0
 	}
-	if err := g.motor.GoFor(ctx, dir*g.rpm, 0, nil); err != nil {
+	if err := g.motor.SetRPM(ctx, dir*g.rpm, nil); err != nil {
 		return err
 	}
 	defer utils.UncheckedErrorFunc(func() error {
@@ -420,7 +420,7 @@ func (g *singleAxis) testLimit(ctx context.Context, pin int) (float64, error) {
 		wrongPin = 0
 	}
 
-	err := g.motor.GoFor(ctx, d*g.rpm, 0, nil)
+	err := g.motor.SetRPM(ctx, d*g.rpm, nil)
 	if err != nil {
 		return 0, err
 	}

components/gantry/singleaxis/singleaxis_test.go

@@ -45,7 +45,7 @@ func createFakeMotor() motor.Motor {
 		},
 		ResetZeroPositionFunc: func(ctx context.Context, offset float64, extra map[string]interface{}) error { return nil },
 		GoToFunc:              func(ctx context.Context, rpm, position float64, extra map[string]interface{}) error { return nil },
-		GoForFunc:             func(ctx context.Context, rpm, revolutions float64, extra map[string]interface{}) error { return nil },
+		SetRPMFunc:            func(ctx context.Context, rpm float64, extra map[string]interface{}) error { return nil },
 		StopFunc:              func(ctx context.Context, extra map[string]interface{}) error { return nil },
 		SetPowerFunc:          func(ctx context.Context, powerPct float64, extra map[string]interface{}) error { return nil },
 	}
@@ -287,16 +287,16 @@ func TestHome(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, homed, test.ShouldBeTrue)
 
-	goForErr := errors.New("GoFor failed")
+	setRPMErr := errors.New("SetRPM failed")
 	posErr := errors.New("Position fail")
 	fakeMotor := &inject.Motor{
 		PropertiesFunc: func(ctx context.Context, extra map[string]interface{}) (motor.Properties, error) {
 			return motor.Properties{
 				PositionReporting: false,
 			}, nil
 		},
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
-			return goForErr
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
+			return setRPMErr
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error {
 			return nil
@@ -368,7 +368,7 @@ func TestHomeLimitSwitch(t *testing.T) {
 
 	getPosErr := errors.New("failed to get position")
 	fakegantry.motor = &inject.Motor{
-		GoForFunc:    func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error { return nil },
+		SetRPMFunc:   func(ctx context.Context, rpm float64, extra map[string]interface{}) error { return nil },
 		StopFunc:     func(ctx context.Context, extra map[string]interface{}) error { return nil },
 		PositionFunc: func(ctx context.Context, extra map[string]interface{}) (float64, error) { return 0, getPosErr },
 	}
@@ -381,7 +381,7 @@ func TestHomeLimitSwitch(t *testing.T) {
 				PositionReporting: true,
 			}, nil
 		},
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
 			return errors.New("err")
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error { return nil },
@@ -395,7 +395,7 @@ func TestHomeLimitSwitch(t *testing.T) {
 				PositionReporting: true,
 			}, nil
 		},
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
 			return nil
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error { return errors.New("err") },
@@ -409,7 +409,7 @@ func TestHomeLimitSwitch(t *testing.T) {
 				PositionReporting: true,
 			}, nil
 		},
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
 			return errors.New("err")
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error { return nil },
@@ -467,7 +467,7 @@ func TestHomeLimitSwitch2(t *testing.T) {
 
 	getPosErr := errors.New("failed to get position")
 	fakegantry.motor = &inject.Motor{
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
 			return nil
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error {
@@ -486,7 +486,7 @@ func TestHomeLimitSwitch2(t *testing.T) {
 				PositionReporting: true,
 			}, nil
 		},
-		GoForFunc: func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error {
+		SetRPMFunc: func(ctx context.Context, rpm float64, extra map[string]interface{}) error {
 			return errors.New("not supported")
 		},
 		StopFunc: func(ctx context.Context, extra map[string]interface{}) error { return nil },
@@ -513,7 +513,7 @@ func TestHomeEncoder(t *testing.T) {
 
 	resetZeroErr := errors.New("failed to set zero")
 	injMotor := &inject.Motor{
-		GoForFunc:             func(ctx context.Context, rpm, rotations float64, extra map[string]interface{}) error { return nil },
+		SetRPMFunc:            func(ctx context.Context, rpm float64, extra map[string]interface{}) error { return nil },
 		StopFunc:              func(ctx context.Context, extra map[string]interface{}) error { return nil },
 		ResetZeroPositionFunc: func(ctx context.Context, offset float64, extra map[string]interface{}) error { return resetZeroErr },
 	}

module/modmanager/manager.go

@@ -830,6 +830,11 @@ func (mgr *Manager) newOnUnexpectedExitHandler(mod *module) func(exitCode int) b
 		if orphanedResourceNames := mgr.attemptRestart(mgr.restartCtx, mod); orphanedResourceNames != nil {
 			if mgr.removeOrphanedResources != nil {
 				mgr.removeOrphanedResources(mgr.restartCtx, orphanedResourceNames)
+				rNames := make([]string, len(orphanedResourceNames))
+				for idx, rName := range orphanedResourceNames {
+					rNames[idx] = rName.String()
+				}
+				mgr.logger.Debugw("Removed resources after failed module restart", "module", mod.cfg.Name, "resources", rNames)
 			}
 			return false
 		}

module/modmanager/manager_test.go

@@ -530,8 +530,8 @@ func TestModuleReloading(t *testing.T) {
 
 		testutils.WaitForAssertion(t, func(tb testing.TB) {
 			tb.Helper()
-			test.That(tb, logs.FilterMessageSnippet("Error while restarting crashed module").Len(),
-				test.ShouldEqual, 3)
+			test.That(tb, logs.FilterMessageSnippet("Removed resources after failed module restart").Len(),
+				test.ShouldEqual, 1)
 		})
 
 		ok = mgr.IsModularResource(rNameMyHelper)
@@ -546,6 +546,8 @@ func TestModuleReloading(t *testing.T) {
 			test.ShouldEqual, 1)
 		test.That(t, logs.FilterMessageSnippet("Module successfully restarted").Len(),
 			test.ShouldEqual, 0)
+		test.That(t, logs.FilterMessageSnippet("Error while restarting crashed module").Len(),
+			test.ShouldEqual, 3)
 
 		// Assert that RemoveOrphanedResources was called once.
 		test.That(t, dummyRemoveOrphanedResourcesCallCount.Load(), test.ShouldEqual, 1)
@@ -587,7 +589,7 @@ func TestModuleReloading(t *testing.T) {
 
 		testutils.WaitForAssertion(t, func(tb testing.TB) {
 			tb.Helper()
-			test.That(tb, logs.FilterMessageSnippet("Will not attempt to restart crashed module").Len(),
+			test.That(tb, logs.FilterMessageSnippet("Removed resources after failed module restart").Len(),
 				test.ShouldEqual, 1)
 		})
 
@@ -598,11 +600,13 @@ func TestModuleReloading(t *testing.T) {
 		test.That(t, err.Error(), test.ShouldContainSubstring, "not connected")
 
 		// Assert that logs reflect that test-module crashed and was not
-		// successfully restarted.
+		// restarted.
 		test.That(t, logs.FilterMessageSnippet("Module has unexpectedly exited").Len(),
 			test.ShouldEqual, 1)
 		test.That(t, logs.FilterMessageSnippet("Module successfully restarted").Len(),
 			test.ShouldEqual, 0)
+		test.That(t, logs.FilterMessageSnippet("Will not attempt to restart crashed module").Len(),
+			test.ShouldEqual, 1)
 
 		// Assert that RemoveOrphanedResources was called once.
 		test.That(t, dummyRemoveOrphanedResourcesCallCount.Load(), test.ShouldEqual, 1)

motionplan/constraint.go

@@ -600,7 +600,7 @@ func NewConstraints(
 // ConstraintsFromProtobuf converts a protobuf object to a Constraints object.
 func ConstraintsFromProtobuf(pbConstraint *motionpb.Constraints) *Constraints {
 	if pbConstraint == nil {
-		return nil
+		return NewEmptyConstraints()
 	}
 
 	// iterate through all motionpb.LinearConstraint and convert to RDK form

referenceframe/primitives.go

@@ -1,7 +1,6 @@
 package referenceframe
 
 import (
-	"context"
 	"fmt"
 	"math"
 
@@ -56,14 +55,6 @@ func JointPositionsFromRadians(radians []float64) *pb.JointPositions {
 	return &pb.JointPositions{Values: n}
 }
 
-// InputEnabled is a standard interface for all things that interact with the frame system
-// This allows us to figure out where they currently are, and then move them.
-// Input units are always in meters or radians.
-type InputEnabled interface {
-	CurrentInputs(ctx context.Context) ([]Input, error)
-	GoToInputs(context.Context, ...[]Input) error
-}
-
 // interpolateInputs will return a set of inputs that are the specified percent between the two given sets of
 // inputs. For example, setting by to 0.5 will return the inputs halfway between the from/to values, and 0.25 would
 // return one quarter of the way from "from" to "to".

robot/framesystem/framesystem.go

@@ -30,6 +30,14 @@ var API = resource.APINamespaceRDKInternal.WithServiceType(SubtypeName)
 // InternalServiceName is used to refer to/depend on this service internally.
 var InternalServiceName = resource.NewName(API, "builtin")
 
+// InputEnabled is a standard interface for all things that interact with the frame system
+// This allows us to figure out where they currently are, and then move them.
+// Input units are always in meters or radians.
+type InputEnabled interface {
+	CurrentInputs(ctx context.Context) ([]referenceframe.Input, error)
+	GoToInputs(context.Context, ...[]referenceframe.Input) error
+}
+
 // A Service that returns the frame system for a robot.
 //
 // TransformPose example:
@@ -73,7 +81,7 @@ type Service interface {
 
 	// CurrentInputs returns a map of the current inputs for each component of a machine's frame system
 	// and a map of statuses indicating which of the machine's components may be actuated through input values.
-	CurrentInputs(ctx context.Context) (map[string][]referenceframe.Input, map[string]referenceframe.InputEnabled, error)
+	CurrentInputs(ctx context.Context) (map[string][]referenceframe.Input, map[string]InputEnabled, error)
 
 	// FrameSystem returns the frame system of the machine and incorporates any specified additional transformations.
 	FrameSystem(ctx context.Context, additionalTransforms []*referenceframe.LinkInFrame) (referenceframe.FrameSystem, error)
@@ -222,7 +230,7 @@ func (svc *frameSystemService) TransformPose(
 		if !ok {
 			return nil, DependencyNotFoundError(name)
 		}
-		inputEnabled, ok := component.(referenceframe.InputEnabled)
+		inputEnabled, ok := component.(InputEnabled)
 		if !ok {
 			return nil, NotInputEnabledError(component)
 		}
@@ -247,15 +255,15 @@ func (svc *frameSystemService) TransformPose(
 // InputEnabled resources that those inputs came from.
 func (svc *frameSystemService) CurrentInputs(
 	ctx context.Context,
-) (map[string][]referenceframe.Input, map[string]referenceframe.InputEnabled, error) {
+) (map[string][]referenceframe.Input, map[string]InputEnabled, error) {
 	fs, err := svc.FrameSystem(ctx, []*referenceframe.LinkInFrame{})
 	if err != nil {
 		return nil, nil, err
 	}
 	input := referenceframe.StartPositions(fs)
 
 	// build maps of relevant components and inputs from initial inputs
-	resources := map[string]referenceframe.InputEnabled{}
+	resources := map[string]InputEnabled{}
 	for name, original := range input {
 		// skip frames with no input
 		if len(original) == 0 {
@@ -267,7 +275,7 @@ func (svc *frameSystemService) CurrentInputs(
 		if !ok {
 			return nil, nil, DependencyNotFoundError(name)
 		}
-		inputEnabled, ok := component.(referenceframe.InputEnabled)
+		inputEnabled, ok := component.(InputEnabled)
 		if !ok {
 			return nil, nil, NotInputEnabledError(component)
 		}

robot/impl/resource_manager_test.go

@@ -50,7 +50,6 @@ import (
 	"go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/module/modmaninterface"
-	"go.viam.com/rdk/motionplan"
 	"go.viam.com/rdk/operation"
 	"go.viam.com/rdk/pointcloud"
 	"go.viam.com/rdk/referenceframe"
@@ -463,16 +462,6 @@ func TestManagerAdd(t *testing.T) {
 	test.That(t, resource1, test.ShouldEqual, injectBoard)
 
 	injectMotionService := &inject.MotionService{}
-	injectMotionService.MoveFunc = func(
-		ctx context.Context,
-		componentName resource.Name,
-		grabPose *referenceframe.PoseInFrame,
-		worldState *referenceframe.WorldState,
-		constraints *motionplan.Constraints,
-		extra map[string]interface{},
-	) (bool, error) {
-		return false, nil
-	}
 	objectMResName := motion.Named("motion1")
 	manager.resources.AddNode(objectMResName, resource.NewConfiguredGraphNode(resource.Config{}, injectMotionService, unknownModel))
 	motionService, err := manager.ResourceByName(objectMResName)