Hardware: end-effector attachment on arm page; frame-config step on camera page#4968
Open
shannonbradshaw wants to merge 1 commit intoviamrobotics:mainfrom
Open
Hardware: end-effector attachment on arm page; frame-config step on camera page#4968shannonbradshaw wants to merge 1 commit intoviamrobotics:mainfrom
shannonbradshaw wants to merge 1 commit intoviamrobotics:mainfrom
Conversation
…amera page Applies playbook-reviewer-feedback. Two small changes bounded by one reviewer ask plus one verified structural gap. ### add-an-arm.md — answers the reviewer's ask Reviewer asked: > Can we describe the after arm frame here, the json, urdf, etc? Two paragraphs appended inside the existing "Configure a frame (recommended)" step: 1. Set the attached component's `parent` to the arm's name to attach a gripper, camera, or other tool to the end effector. Links to add-a-gripper's frame step for a worked example (no JSON duplication). 2. Custom arm modules implement the arm's `Kinematics` method to return the kinematic model. Links to /motion-planning/reference/kinematics/ for URDF and SVA JSON formats, and /build-modules/write-a-driver-module/ for the module-code side. ### add-a-camera.md — structural gap surfaced while addressing the ask The camera how-to had no Configure-a-frame step. The arm and gripper how-tos both do, and wrist-mounted cameras on arms are a common setup. Deliberate scope expansion beyond the reviewer's literal comment; consistency fix across peer how-to pages. New step 4 "Configure a frame (recommended)" inserted between the existing attributes step and the existing save step (old steps 4 and 5 renumber to 5 and 6). Three patterns: - Wrist-mounted on an arm (parent = arm name, with JSON example). - Fixed in the workspace (parent = world). - Mounted on a mobile base (parent = base name) — called out as a static offset, not live tracking, because bases do not contribute a dynamic frame. Verified against rdk/robot/impl/local_robot.go line 1219, where the frame system extracts a kinematics model only for arms, gantries, and grippers; bases fall in the default case. Cross-links to /hardware/multi-machine/cross-machine-frames/ for remote-part cameras and /motion-planning/frame-system/ for the full reference. ### Deliberate non-additions - No reference-style description of what frames the arm contributes (belongs on the frame-system reference page, not a how-to). - No Viam app UI references. The flow-document requirement from the playbook says UI claims need a pre-existing flow doc; no applicable doc exists, and the ask does not require one. - No `GetFrameSystemConfig` SDK guidance. The call returns parts with an embedded kinematics model, not a flattened list of link names, so the most useful thing I could have said would have required caveats this PR does not need. ### Verification log Every claim traced to source in Phase 1 of the playbook. Key anchors: rdk/robot/impl/local_robot.go:1219 (arm is a ModelFramer, base is not); rdk/components/arm/client.go:190 (Kinematics method signature); docs/motion-planning/reference/kinematics.md covers URDF and SVA JSON; all link targets confirmed present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for viam-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hey @shannonbradshaw — CI is green and no reviewer is assigned yet. Could you request one when you have a chance? Auto-comment from overwatch. Will not re-nudge for 7 days. |
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
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.

Summary
Applies the reviewer-feedback playbook. Two small changes bounded by one reviewer ask plus one verified structural gap.
Reviewer feedback addressed
On `/hardware/common-components/add-an-arm/`:
Changes
add-an-arm.md— reviewer askTwo paragraphs appended inside the existing "Configure a frame (recommended)" step:
add-a-camera.md— structural gap (deliberate scope expansion)The camera how-to had no Configure-a-frame step. The arm and gripper how-tos both do. Wrist-mounted cameras on arms are a common setup. Flagging as deliberate scope expansion beyond the reviewer's verbatim comment, per the playbook's scope-discipline rule.
New "### 4. Configure a frame (recommended)" inserted between the existing attributes step and save step. Old steps 4 and 5 renumber to 5 and 6. Three patterns:
Cross-links to `/hardware/multi-machine/cross-machine-frames/` for remote-part cameras and `/motion-planning/frame-system/` for the full reference.
Verification (full log from Phase 1)
Deliberately out of scope
Test plan
/hardware/common-components/add-an-arm/— two new paragraphs render inside "Configure a frame"/hardware/common-components/add-a-camera/— new step 4 renders; steps 5 and 6 renumber correctly; JSON block rendersadd-a-gripper/#3-configure-a-frame-recommendedresolves/motion-planning/reference/kinematics/resolves/hardware/multi-machine/cross-machine-frames/resolves🤖 Generated with Claude Code