fix: legacy robot.q/qd mutation never wrote handle's integration back - #129
Merged
petercorke merged 1 commit intoAug 21, 2026
Merged
Conversation
_sync_legacy() only pulled robot.q/qd -> handle.q/qd (detecting the deprecated direct-mutation style and adopting it), but never pushed Swift's own per-step integration (velocity-mode step_v()) back into robot.q/qd. A control loop that reads robot.q back mid-loop -- the normal pattern, e.g. RTB's own README p_servo example -- saw a permanently stale configuration: fkine()/jacobe() never advanced, so the computed qd never changed either, and the robot just ran the same joint velocity forever without ever converging or stopping. Reproduced RTB's exact README example headlessly: without this fix it hits a 500-step cap never arriving, with panda.q frozen at the initial value the entire time; with the fix, converges in 43 steps. Add _push_legacy(), the mirror of _sync_legacy(): once a handle is confirmed to be in legacy mode (_warned), write handle.q/qd back into robot._q/_qd after each step_v() integration, keeping the snapshot in sync so this doesn't retrigger the warning. Gated on _warned (not unconditional) so a handle never driven the legacy way -- the intended case, including several handles sharing one plain robot model -- is untouched, preserving the actual design goal of the refactor.
Open
1 task
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
_sync_legacy()only pulledrobot.q/robot.qd->handle.q/handle.qd(detecting the deprecated direct-mutation style and adopting it), but never pushed Swift's own per-step integration (velocity-modestep_v()) back intorobot.q/robot.qd.A control loop that reads
robot.qback mid-loop -- the normal pattern, and RTB's own documented README example -- saw a permanently stale configuration:fkine()/jacobe()never advanced, so the computedqdnever changed either, and the robot just ran the same joint velocity forever without converging or stopping. Not just a deprecation-warning nuisance -- a silent functional break with no error.Reproduced RTB's exact README p_servo example headlessly:
panda.qfrozen at the initial value the entire time.Fix
Added
_push_legacy(), the mirror of_sync_legacy(): once a handle is confirmed to be in legacy mode (_warned), writehandle.q/handle.qdback intorobot._q/robot._qdafter eachstep_v()integration, keeping the snapshot in sync so this doesn't retrigger the warning.Gated on
_warned(not unconditional) so a handle never driven the legacy way -- the intended case, including several handles sharing one plain robot model -- is untouched, preserving the actual design goal of the refactor.Companion RTB PRs:
docs: update Swift RRMC example to the AssemblyHandle API(README +examples/RRMC_swift.py)fix: Robot._to_dict() uses set_alpha(), deprecated by spatialgeometryTest plan
test_legacy_qd_mutation_updates_robot_q_after_step: drivespanda.qddirectly across twoenv.step()calls, assertspanda.qadvances correctly and only warns oncertb-marked; one pre-existing, unrelatedspatialgeometryPolyline gap from a stale local install, not this change)