petercorke / petercorke/robotics-toolbox-python
Robot/Link architecture redesign: generic LinkType, one-Robot-class design, kinematic/scene-graph decoupling
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3.5k
- Forks
- 624
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 53
Description
Migrated from tech-debt.md (deleted, see repo history via git log -- tech-debt.md). Groups everything under the long-running "Robot/Link architecture" direction -- see the "Roadmap by Peter (2026-07-31)" note that was at the end of the old file: RTB 1.5.0 plans a "major renovation of Swift backend and detangle of SG from RTB" and RTB 2.0.0 plans to "revisit the top-level robot classes. Explicit ETSRobot, Robot -> base". These subitems are the concrete groundwork for that.
Check items off as they land; close this issue once all are done (or split a subitem into its own issue if it grows a life of its own).
-
Make
Robotgeneric overLinkType.DHLinkplays two roles (DH-parameter carrier + compiledLinksubclass); becauseRobot(BaseRobot[Link])pinsLinkType=Link, pyright can't see DH parameters on links insideDHRobot, causing 67+ type errors. Fix:class Robot(BaseRobot[RobotLinkType], RobotKinematicsMixin)generic, pinned per subclass (DHRobot(Robot[DHLink]),PoERobot(Robot[PoELink]),RobotURDF(Robot[Link])). Eliminateslinksproperty overrides and# type: ignore[union-attr]workarounds. -
Evaluate "one Robot class, polymorphic
Link.A(q)" design. Core idea:Linkis an abstract base withA(q) -> SE3;DHLink/PoELink/ETSLinkeach implement it their own way (DH formula /exp(S*q)/ ETS eval, dispatching to fknm for speed).Robotbecomes a single class holdinglist[Link], with factory constructors (Robot.DH(...),Robot.ETS(...),Robot.PoE(...),Robot.URDF(...)) instead of subclasses. RemovesERobot(currently dead: a 5-line pass-through alias forRobot) and the Generic-iterator problem entirely.Robotis arguably a poor name for what's specifically an ETS-based robot --ETSRobotwould be accurate but is an API break; consider for a major version, keepingRobotas a deprecated alias. Preserve the fknm C-extension batch-FK path (robot._fkine_fknm(q)) as an optimised overload when all links areETSLink-- it's a real perf win, not a design constraint. -
Delete
ERobot(dead code, no functionality) once the above lands. -
Simplify
Link.A(q): drop the_Tscache.Link.A(q)(Link.py:1509-1516) caches the link's constant (non-joint) ETS prefix as one pre-multiplied matrix to avoid re-multiplying it every call. Benchmarked against just callingself._ets.eval(q)(200k calls, real Panda link): the cached version is ~15-18% slower (1.677us vs 1.421us/call) -- the Python-level overhead of a second method call + a separate@dispatch for a 4x4 array costs more than the cheap Eigen constant-multiply it's avoiding. Root structural reason: URDF-derived links are at most 2 ETs (one constant, one joint) by construction, so there's nothing meaningful for_Tsto save in the common case. Not yet benchmarked: a link with a genuinely long uncompiled constant-ET prefix (hand-authored ETS models that never call.compile()) -- check that regime before removing the cache outright. -
Wire
ETS.compile()into non-URDF model construction.ETS.compile()(constant-folds consecutive constant ETs) is correct but never called anywhere in the repo, including the hand-authored model constructors that would benefit (models.ETS.Panda()and similar). Worth calling automatically at the end ofRobot.__init__for that construction path, or documenting it as a required convention for model authors. -
Decouple
Robot/Linkkinematic state from scene-graph/rendering state. Directly related todesiderata.md's "Stateless over stateful" aspiration (.qretained as persistent state, "not yet achieved").Robot/Linkcurrently carry not just.qbutSceneNode/update()(was_propogate_scene_tree()) machinery -- world-transform bookkeeping that exists purely to support rendering (Swift, PyPlot), mixed into the kinematic model classes. Agreed direction:Robot/Linkbecome a pure kinematic model (links, DH/ETS/PoE params, joint limits, geometry attachments -- no live world-transform state); a separate viz-owned "instance handle" owns mutable per-simulation state (q, plausiblybase/tool) and computes part poses via the pure FK path.- Partial progress already shipped:
Robot.fkine_geometry(q, robot_alpha, collision_alpha)computes every geometry part's world pose purely from an explicitq(viafkine_all+ each geometry's fixed local offset), verified bit-for-bit against the oldSceneNode-based path to ~1e-9 precision (test_Robot.py::test_fkine_geometry_matches_scene_graph). Swift'senv.add()now returns a handle (AssemblyHandle, generalized fromRobotHandle) that ownsq/qdand calls through tofkine_geometry-- Swift's hot render path no longer depends onSceneNodemutation.Robot/Linkthemselves are still unchanged (still carry.q, still haveSceneNode); this is the pure alternative living alongside the stateful path, not a replacement. - Not yet made pure: gripper joints (
fkine_geometrystill readsgripper.qas ordinary state, not a parameter). - Remaining: actually remove
SceneNodefromRobot/Link, update PyPlot/teach to the handle-based model, decidebase/toolownership (see below).
- Partial progress already shipped:
-
Resolve
base/toolownership. Some models genuinely needbase/toolas part of their kinematic definition (e.g. a fixed pedestal offset); for others it's purely "instance placement," indistinguishable fromq. No formalized answer yet -- needed before the handle redesign above can be completed (does the instance handle ownbase/tool, or does the model?). -
Remove
Robot._fk_dict()(dead code). Walks every link's geometry/collision and reads each shape's cached_wT/_wq(populated by the old_update_link_tf()+ scene-graph-propagate pass). Its only real caller was Swift's old_draw_all(), replaced byfkine_geometry()-- grepped, no remaining callers anywhere in this repo.Shape.fk_dict()(the per-shape method it calls) is still alive and used directly by Swift for plain shapes -- don't touch that. -
Revisit
Robot.rne()'smdh-based misuse guard once the hierarchy above is resolved. Current guard (assert getattr(self, "mdh", True), ...) checks themdhattribute rather than class identity, because joint-last compliance tracks the DH convention in use (DHLink._to_ets()'s MDH branch), not theDHRobotclass itself -- a class-name-based check would wrongly reject a compliantmdh=TrueDHRobot. If the hierarchy redesign above lands (oneRobotclass, orDHRobotstops subclassingRobot), this question may become moot or need re-deriving structurally rather than via a runtime attribute check.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
This is a multi-part architecture effort rather than one newcomer-sized change. Start by splitting out a concrete subitem, then read Link.py:1509-1516 and test_Robot.py::test_fkine_geometry_matches_scene_graph where relevant. Done means one scoped redesign item is implemented, its existing behavior is covered, and this checklist is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 28/100