Repository navigation
arm: make the builtin motion service optional - #55
Merged
Merged
Conversation
viam-server's Windows build has no motion service, so requiring "builtin" stopped the arm from constructing at all there -- not just MoveToPosition, but the whole component, and with it the gripper and calibration sensor that depend on it. "builtin" is now an optional dependency, so RDK passes it whenever it exists and nothing changes on platforms that have one. A motion service named in the config stays required, since asking for a specific one and silently not getting it would be worse than failing. When no motion service is available the constructor says so once, and MoveToPosition returns an error instead of dereferencing nil. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nick Hehr (HipsterBrown)
approved these changes
Sep 24, 2026
Nick Hehr (HipsterBrown)
left a comment
Nick Hehr (HipsterBrown)
left a comment
Contributor
There was a problem hiding this comment.
One update to that log line then it should be good to ship.
| ms, err = motion.FromProvider(deps, "builtin") | ||
| if err != nil { | ||
| return nil, err | ||
| logger.Info("builtin motion service unavailable, MoveToPosition disabled") |
Contributor
There was a problem hiding this comment.
This should be a warning rather than Info
Review feedback: without a motion service MoveToPosition is disabled, which is a reduced capability worth surfacing, not routine information. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
viam-server's Windows build has no motion service, so requiring
builtinstops the arm constructing there at all — not justMoveToPosition, but the whole component, and with it the gripper and calibration sensor that depend on it. The Windows binary from #50 therefore can't be used.builtinbecomes an optional dependency, so RDK passes it whenever it exists and nothing changes on platforms that have one. A motion service named explicitly in the config stays required — asking for a specific one and silently not getting it would be worse than failing. When none is available, the constructor logs it once andMoveToPositionreturns an error rather than dereferencing nil.Tested
go test ./...passes (12 packages).TestHardwareArmMoveToPositionSendsGoalCloudcovers the unchanged path: with a motion service injected,MoveToPositionstill works.TestHardwareArmMoveToPositionRequiresMotionService, mirroring the simulated arm's guard test. Removing the guard makes it fail.MoveToPositionstill plans and executes: commanded a 20 mm move on an SO-101 follower arm and it reached the target to within a millimetre, matching the same command on the released 0.9.9 build immediately before the swap.builtin motion service unavailable, MoveToPosition disabled. Teleoperation, motion recording and playback all work.The SO-101: Teleop and Motion Capture course we're launching uses this module, and this change is one of the pieces needed for Windows learners to take it.
🤖 Generated with Claude Code