petercorke / petercorke/robotics-toolbox-python

DHRobot tests fail against SG's new scene-graph cycle detection when sharing link objects across robots

Open
#581 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

tech-debt
Dominant language
C++
Stars
3.5k
Forks
624
Avg merge
2d 4h
Merged PRs (30d)
53

Description

New finding, 2026-08-09, while getting the test suite green again after unvendoring spatialgeometry (see the fix/unvendor-spatialgeometry PR) and bumping the effective spatialgeometry API to SG's current main.

tests/test_DHRobot.py::TestDHRobot::test_isspherical fails against SG's main:

ValueError: Cannot set RevoluteDH(...)'s scene_parent to RevoluteDH(...) --
RevoluteDH(...) is already a descendant of RevoluteDH(...), this would
create a cycle in the scene graph

The test builds several DHRobots that reuse the same RevoluteDH link instances in different combinations/orderings:

l0 = rp.RevoluteDH()
l1 = rp.RevoluteDH(alpha=-np.pi / 2)
l2 = rp.RevoluteDH(alpha=np.pi / 2)
l3 = rp.RevoluteDH()

r0 = rp.DHRobot([l0, l1, l2, l3])
r1 = rp.DHRobot([l0, l1])
r2 = rp.DHRobot([l1, l2, l3, l0])   # <-- fails here

Each DHRobot.__init__ sets link.scene_parent = link.parent for every link (BaseRobot._sort_links). SG recently added cycle-detection to scene_parent's setter (a real safety feature -- an undetected cycle in the scene graph previously caused update()'s root-finding walk to spin forever). Because l0/l1/l2/l3 are shared objects, r0's construction already wired l1.scene_parent = l0 (from the [l0,l1,l2,l3] chain); when r2 later tries l0.scene_parent = l3 in a different order, SG correctly notices l0 is already an ancestor of l3 from r0's wiring and refuses, since these are the same link objects, not fresh copies.

Open question, not yet investigated: is this DHRobot pattern (constructing multiple robots from overlapping/reordered sets of shared link objects) supposed to be supported? If so, DHRobot/BaseRobot needs to stop treating scene_parent as if each link belongs to exactly one robot's scene graph (possibly relevant to the Robot/Link kinematic-vs-scene-graph-state decoupling work tracked elsewhere). If not -- i.e. link objects were never meant to be shared across multiple DHRobot instances -- the fix is on the test side (construct fresh link instances per robot) and the test itself is what should change, not the library.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with tests/test_DHRobot.py::TestDHRobot::test_isspherical and the DHRobot.init path through BaseRobot._sort_links. Determine whether overlapping, reordered shared links across DHRobot instances are supported, using the scene_parent cycle error and the referenced unvendor-spatialgeometry change as context. Done means a documented decision followed by either a library fix or updated test setup.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
robotics, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.