Motion replanning

cli/data.go

@@ -742,12 +742,12 @@ func (c *viamClient) tabularData(dest string, request *datapb.ExportTabularDataR
 			ctx, cancel := context.WithCancel(context.Background())
 
 			defer func() {
-				writer.Flush()   //nolint:errcheck,gosec
-				dataFile.Close() //nolint:errcheck,gosec
+				writer.Flush()   //nolint:errcheck
+				dataFile.Close() //nolint:errcheck
 				cancel()
 
 				if exportErr != nil {
-					os.Remove(dataFile.Name()) //nolint:errcheck,gosec
+					os.Remove(dataFile.Name()) //nolint:errcheck
 				}
 			}()
 

cli/module_build.go

@@ -235,7 +235,7 @@ func (c *viamClient) moduleBuildListAction(cCtx *cli.Context, args moduleBuildLi
 			job.StartTime.AsTime().Format(time.RFC3339))
 	}
 	// the table is not printed to stdout until the tabwriter is flushed
-	//nolint: errcheck,gosec
+	//nolint:errcheck
 	w.Flush()
 	return nil
 }
@@ -700,7 +700,7 @@ func (c *viamClient) loadGitignorePatterns(repoPath string) (gitignore.Matcher,
 			return nil, errors.Wrap(err, "failed to open .gitignore file")
 		}
 		defer func() {
-			//nolint:errcheck,gosec // Ignore close error for read-only file
+			//nolint:errcheck
 			file.Close()
 		}()
 

components/arm/sim/kinematics.go

@@ -39,6 +39,8 @@ var xarm7JSON []byte
 //go:embed kinematics/lite6.json
 var lite6JSON []byte
 
+// Re-Documenting each public arm API is zero value and onerous. This applies for the whole file.
+// revive:disable:exported
 func modelFromName(model, name string) (referenceframe.Model, error) {
 	switch model {
 	case ur5eModel:
@@ -74,7 +76,7 @@ func buildModel(resName string, conf *Config) (referenceframe.Model, error) {
 	}
 }
 
-func (sa *simulatedArm) Get3DModels(
+func (sa *SimulatedArm) Get3DModels(
 	ctx context.Context, extra map[string]interface{},
 ) (map[string]*commonpb.Mesh, error) {
 	models := make(map[string]*commonpb.Mesh)
@@ -97,14 +99,14 @@ func (sa *simulatedArm) Get3DModels(
 	return models, nil
 }
 
-func (sa *simulatedArm) Kinematics(ctx context.Context) (referenceframe.Model, error) {
+func (sa *SimulatedArm) Kinematics(ctx context.Context) (referenceframe.Model, error) {
 	sa.mu.Lock()
 	defer sa.mu.Unlock()
 
 	return sa.model, nil
 }
 
-func (sa *simulatedArm) Geometries(
+func (sa *SimulatedArm) Geometries(
 	ctx context.Context, extra map[string]interface{},
 ) ([]spatialmath.Geometry, error) {
 	inputs, err := sa.CurrentInputs(ctx)

components/arm/sim/sim.go

@@ -30,12 +30,16 @@ type operation struct {
 	stopped      bool
 }
 
+// Re-Documenting each public arm API is zero value and onerous. This applies for the whole file.
+// revive:disable:exported
 func (op operation) isMoving() bool {
 	// If we have `targetInputs` and we are not done and not stopped
-	return op.targetInputs != nil && !op.done && !op.stopped
+	return !op.done && !op.stopped
 }
 
-type simulatedArm struct {
+// SimulatedArm is a fake arm and exposes an `UpdateForTime` method that can be used to explicitly
+// control the arm position in tests as Move* API calls are made.
+type SimulatedArm struct {
 	resource.Named
 
 	// logical properties
@@ -90,10 +94,8 @@ func init() {
 	})
 }
 
-// Model is the name used to refer to the simulated arm model.
 var Model = resource.DefaultModelFamily.WithModel("simulated")
 
-// Validate ensures all parts of the config are valid.
 func (conf *Config) Validate(path string) ([]string, []string, error) {
 	var err error
 	switch {
@@ -132,9 +134,9 @@ func NewArm(ctx context.Context, deps resource.Dependencies, resConf resource.Co
 func newArm(
 	namedArm resource.Named, modelName string, model referenceframe.Model, speed float64, simulateTime bool,
 	logger logging.Logger,
-) *simulatedArm {
+) *SimulatedArm {
 	ctx, cancel := context.WithCancel(context.Background())
-	ret := &simulatedArm{
+	ret := &SimulatedArm{
 		Named:  namedArm,
 		logger: logger,
 
@@ -146,26 +148,30 @@ func newArm(
 		cancel: cancel,
 
 		currInputs: make([]float64, len(model.DoF())),
+		operation: operation{
+			done: true,
+		},
 	}
 
 	if simulateTime {
 		// When simulating time, avoid ever letting the zero value be visible. Lest the first
 		// movement be unpredictable.
 		ret.lastUpdated = time.Now()
 		ret.timeSimulation = utils.NewStoppableWorkerWithTicker(10*time.Millisecond, func(_ context.Context) {
-			ret.updateForTime(time.Now())
+			ret.UpdateForTime(time.Now())
 		})
 	}
 
 	return ret
 }
 
-// Simulated arms only update their position when `updateForTime` is called. This can be used by
-// tests for deterministic passage of time. Or can be called by a background goroutine to follow a
-// realtime clock.
+// UpdateForTime updates the simulated joint positions as a function of the input time, configured
+// speed and target joint positions from a running `Move*` API call. Simulated arms only update
+// their position when `UpdateForTime` is called. This can be used by tests for deterministic
+// passage of time. Or can be called by a background goroutine to follow a realtime clock.
 //
-// But direct `arm.Arm` API calls will _not_ update the position under the hood.
-func (sa *simulatedArm) updateForTime(now time.Time) {
+// Direct `arm.Arm` API calls will _not_ update the position under the hood.
+func (sa *SimulatedArm) UpdateForTime(now time.Time) {
 	sa.mu.Lock()
 	defer sa.mu.Unlock()
 
@@ -223,7 +229,7 @@ func (sa *simulatedArm) updateForTime(now time.Time) {
 	}
 }
 
-func (sa *simulatedArm) EndPosition(
+func (sa *SimulatedArm) EndPosition(
 	ctx context.Context, extra map[string]interface{},
 ) (spatialmath.Pose, error) {
 	sa.mu.Lock()
@@ -232,17 +238,17 @@ func (sa *simulatedArm) EndPosition(
 	return sa.model.Transform(sa.currInputs)
 }
 
-func (sa *simulatedArm) MoveToPosition(
+func (sa *SimulatedArm) MoveToPosition(
 	ctx context.Context, pose spatialmath.Pose, extra map[string]interface{},
 ) error {
 	return errors.New("unimplemented -- must call with explicit joint positions")
 }
 
-func (sa *simulatedArm) CurrentInputs(ctx context.Context) ([]referenceframe.Input, error) {
+func (sa *SimulatedArm) CurrentInputs(ctx context.Context) ([]referenceframe.Input, error) {
 	return sa.JointPositions(ctx, nil)
 }
 
-func (sa *simulatedArm) JointPositions(
+func (sa *SimulatedArm) JointPositions(
 	ctx context.Context, extra map[string]interface{},
 ) ([]referenceframe.Input, error) {
 	sa.mu.Lock()
@@ -254,7 +260,7 @@ func (sa *simulatedArm) JointPositions(
 	return ret, nil
 }
 
-func (sa *simulatedArm) MoveToJointPositions(
+func (sa *SimulatedArm) MoveToJointPositions(
 	ctx context.Context, target []referenceframe.Input, extra map[string]interface{},
 ) error {
 	if err := arm.CheckDesiredJointPositions(ctx, sa, target); err != nil {
@@ -284,7 +290,6 @@ func (sa *simulatedArm) MoveToJointPositions(
 		default:
 			// Poll for completion:
 			sa.mu.Lock()
-			// Calls to `updateForTime` will nil out `targetInputs` when a movement is completed.
 			done, stopped := sa.operation.done, sa.operation.stopped
 			sa.mu.Unlock()
 
@@ -305,7 +310,7 @@ func (sa *simulatedArm) MoveToJointPositions(
 	}
 }
 
-func (sa *simulatedArm) MoveThroughJointPositions(
+func (sa *SimulatedArm) MoveThroughJointPositions(
 	ctx context.Context,
 	positions [][]referenceframe.Input,
 	_ *arm.MoveOptions,
@@ -320,18 +325,18 @@ func (sa *simulatedArm) MoveThroughJointPositions(
 	return nil
 }
 
-func (sa *simulatedArm) GoToInputs(ctx context.Context, inputSteps ...[]referenceframe.Input) error {
+func (sa *SimulatedArm) GoToInputs(ctx context.Context, inputSteps ...[]referenceframe.Input) error {
 	return sa.MoveThroughJointPositions(ctx, inputSteps, nil, nil)
 }
 
-func (sa *simulatedArm) IsMoving(ctx context.Context) (bool, error) {
+func (sa *SimulatedArm) IsMoving(ctx context.Context) (bool, error) {
 	sa.mu.Lock()
 	defer sa.mu.Unlock()
 
 	return sa.operation.isMoving(), nil
 }
 
-func (sa *simulatedArm) Stop(ctx context.Context, extra map[string]any) error {
+func (sa *SimulatedArm) Stop(ctx context.Context, extra map[string]any) error {
 	sa.mu.Lock()
 	defer sa.mu.Unlock()
 
@@ -346,7 +351,7 @@ func (sa *simulatedArm) Stop(ctx context.Context, extra map[string]any) error {
 	return nil
 }
 
-func (sa *simulatedArm) Close(ctx context.Context) error {
+func (sa *SimulatedArm) Close(ctx context.Context) error {
 	sa.closed.Store(true)
 	sa.cancel()
 	if sa.timeSimulation != nil {

components/arm/sim/sim_test.go

@@ -28,7 +28,7 @@ func TestBasic(t *testing.T) {
 
 	simArmI, err := NewArm(ctx, nil, resConf, logger)
 	test.That(t, err, test.ShouldBeNil)
-	simArm := simArmI.(*simulatedArm)
+	simArm := simArmI.(*SimulatedArm)
 
 	// Assert the starting joint position is all zeroes.
 	currInputs, err := simArm.CurrentInputs(ctx)
@@ -60,7 +60,7 @@ func TestBasic(t *testing.T) {
 	// Advance time by one second.
 	clock := simArm.lastUpdated
 	clock = clock.Add(time.Second)
-	simArm.updateForTime(clock)
+	simArm.UpdateForTime(clock)
 
 	// Assert the joint position for first two joints changed to `1`.
 	currInputs, err = simArm.CurrentInputs(ctx)
@@ -76,7 +76,7 @@ func TestBasic(t *testing.T) {
 	// Advance time by another second. Assert the joint position matches the target. Assert the
 	// operation is considered done.
 	clock = clock.Add(time.Second)
-	simArm.updateForTime(clock)
+	simArm.UpdateForTime(clock)
 
 	currInputs, err = simArm.CurrentInputs(ctx)
 	test.That(t, err, test.ShouldBeNil)
@@ -104,7 +104,7 @@ func TestStop(t *testing.T) {
 
 	simArmI, err := NewArm(ctx, nil, resConf, logger)
 	test.That(t, err, test.ShouldBeNil)
-	simArm := simArmI.(*simulatedArm)
+	simArm := simArmI.(*SimulatedArm)
 
 	// Set up a move that will take "2 seconds" to complete. `MoveToJointPositions` is blocking and
 	// time must be advanced manually. Hence the goroutine.
@@ -158,7 +158,7 @@ func TestTimeSimulation(t *testing.T) {
 
 	simArmI, err := NewArm(ctx, nil, resConf, logger)
 	test.That(t, err, test.ShouldBeNil)
-	simArm := simArmI.(*simulatedArm)
+	simArm := simArmI.(*SimulatedArm)
 	defer func() {
 		// When `SimulateTime` is true, we must call `Close` to wait on background goroutines.
 		err = simArm.Close(ctx)

components/board/genericlinux/buses/i2c.go

@@ -16,7 +16,6 @@ import (
 
 func init() {
 	if _, err := host.Init(); err != nil {
-		//nolint:gosec
 		fmtnolint.Println("Error initializing host:", err)
 	}
 }

etc/.golangci.yaml

@@ -60,6 +60,7 @@ linters:
     - testpackage
     - thelper # false positives
     - tparallel # there are valid reasons to have top-level t.Parallel() but serial subtests
+    - unparam # not useful
     - varnamelen
     - wrapcheck
     - wsl
@@ -81,6 +82,7 @@ linters-settings:
       - composites
   gosec:
     excludes:
+      - G104 # Unhandled errors. These are already linted on, no need to double lint.
       - G115 # TODO(go1.23): maybe reenable
   revive:
     # Unfortunately configuring a single rules disables all other rules, even
@@ -120,6 +122,14 @@ linters-settings:
       - ifElseChain
   gomoddirectives:
     replace-allow-list: [github.com/hashicorp/go-getter]
+  stylecheck:
+    checks:
+      - all
+      - "-ST1005" # Low value complaint. Can remove when gosimple rewrites code in place instead of failing.
+  gosimple:
+    checks:
+      - all
+      - "-S1002" # Low value complaint. Can remove when gosimple rewrites code in place instead of failing.
 issues:
   exclude-rules:
     - path: _test\.go$

ftdc/custom_format.go

@@ -63,7 +63,6 @@ func writeSchema(schema *schema, output io.Writer) error {
 func writeDatum(time int64, prev, curr []float32, output io.Writer) error {
 	numPts := len(curr)
 	if len(prev) != 0 && numPts != len(prev) {
-		//nolint:stylecheck
 		return fmt.Errorf("Bad input sizes. Prev: %v Curr: %v", len(prev), len(curr))
 	}
 
@@ -234,7 +233,7 @@ func flattenMap(mValue reflect.Value) ([]string, []float32, error) {
 			}
 			numbers = append(numbers, subNumbers...)
 		case isNumeric(value.Kind()):
-			//nolint:stylecheck
+
 			return nil, nil, fmt.Errorf("A numeric type was forgotten to be included. Kind: %v", value.Kind())
 		default:
 			// Getting the keys for a structure will ignore these types. Such as the antagonistic
@@ -298,7 +297,7 @@ func flattenStruct(value reflect.Value) ([]string, []float32, error) {
 
 			numbers = append(numbers, subNumbers...)
 		case isNumeric(rField.Kind()):
-			//nolint:stylecheck
+
 			return nil, nil, fmt.Errorf("A numeric type was forgotten to be included. Kind: %v", rField.Kind())
 		default:
 			// Getting the keys for a structure will ignore these types. Such as the antagonistic
@@ -599,7 +598,6 @@ func readDiffBits(reader *bufio.Reader, schema *schema) ([]int, error) {
 // is the post-hydration list and consequently matches the `schema.fieldOrder` size.
 func readData(reader *bufio.Reader, schema *schema, diffedFields []int, prevValues []float32) ([]float32, error) {
 	if prevValues != nil && len(prevValues) != len(schema.fieldOrder) {
-		//nolint
 		return nil, fmt.Errorf("Parser error. Mismatched `prevValues` and schema size. PrevValues: %d Schema: %d",
 			len(prevValues), len(schema.fieldOrder))
 	}

module/module.go

@@ -194,7 +194,7 @@ func NewModule(ctx context.Context, address string, logger logging.Logger) (*Mod
 		if err != nil {
 			return nil, err
 		}
-		//nolint: errcheck, gosec
+		//nolint:errcheck
 		trace.SetProvider(
 			ctx,
 			sdktrace.WithResource(