fix: add the payload to the end-effector link instead of overwriting it - #699
Open
SulimanAbdulrazzaq wants to merge 1 commit into
Open
SulimanAbdulrazzaq wants to merge 1 commit into
SulimanAbdulrazzaq wants to merge 1 commit into
Conversation
payload(m, p) replaced the last link's mass and centre of mass with the payload's, so payload(0), documented as removing the payload, zeroed the link's own mass and centre of mass and changed the inverse dynamics (petercorke#638). The point mass is now combined with the link's own mass, centre of mass and inertia (parallel axis theorem). The link's own parameters are kept, so payload(0) restores them and a new payload replaces the previous one. The payload now goes on the end-effector link when the robot has exactly one. For a DH robot that is links[n - 1], as before, but a URDF robot's links list includes static links, so links[n - 1] can be a link in the middle of the chain, eg. UR5's wrist_1_link rather than wrist_3_link. Robots with several end-effectors keep links[n - 1].
Author
|
@petercorke when you have a moment, could you approve the workflow runs and take a look? |
Owner
|
Thanks for this fix. It's a long standing issue. It's raised a couple of issues which I'm still thinking about like to how to generalise to a multi-EE arm and if that even makes sense. |
3 tasks
petercorke
added a commit
that referenced
this pull request
Sep 27, 2026
Mesh.to_dict()["filename"] now normalizes to forward slashes (SG's jhavl/swift#152 fix for Swift's JS mesh loader on Windows paths), which broke test_mesh_filename's native-separator comparison on every Windows CI job (confirmed on both #698 and #699, unrelated to either PR's own changes) since SG 1.4.1 released 2026-09-21. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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
payload(m, p)overwrote the last link's mass and centre of mass with the payload's (lastlink.m = m,lastlink.r = p), the quick hack from the MATLAB version mentioned in #134. The docstring says "payload(0) removes added payload", butpayload(0)actually zeroed the link's own mass and centre of mass, so the dynamics changed. With the reproduction from #638, Puma560rne(q, 0, 0)goes from[-0, 16.36782, 4.59518, -0.00517, -0.00394, 0]to[-0, 16.02365, 4.38141, 0, 0, 0].The signature is unchanged. The function now does what its docstring says:
payload(0)restores them and a new payload replaces the previous one instead of accumulating.It also changes which link gets the payload.
links[n - 1]is the end-effector link of a DH robot, but a URDF robot'slinksincludes static links, so there it can be a link in the middle of the chain (URDF UR5:wrist_1_linkwhileee_linksis[wrist_3_link]; URDF Panda:panda_link6whileee_linksis[panda_link8]). The payload now goes onee_links[0]when the robot has exactly one end-effector. Robots with several end-effectors keeplinks[n - 1], so nothing that runs today starts failing.RobotProtogains theee_linksproperty thatpayload()now reads.The existing
test_payloadasserted the overwrite ("payload() overwrites the link's m/r outright"), so it now checks the combined mass and centre of mass and thatpayload(0)restores the link. New tests:test_payload_zero_is_noop: the robot.payload(m=0, p=zeros) is not a no-op -- RNE result changes #638 reproduction;test_payload_point_mass: a physics check that does not reuse the new formula. The joint-space inertia matrix grows bym Jp' Jpand the gravity load by-m Jp' g, whereJpis the Jacobian of the payload point;test_payload_replaces_previous;test_payload_end_effector_linkandtest_payload_several_end_effectorsintest_ERobot.py.One limitation is unchanged from before:
copy()rebuilds links from their current parameters, so a copy made while a payload is attached carries the payload as part of the link's own parameters.Related issue
Fixes #638
Checklist
type: description)pytest):pytest tests/ --ignore=tests/test_blocks.py --timeout=50 --timeout_method=thread -qgives 821 passed, 12 skipped (Python 3.12, Linux)payload()signature is unchanged)