Skip to content

Robot/Link architecture redesign: generic LinkType, one-Robot-class design, kinematic/scene-graph decoupling #571

Description

@petercorke

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 Robot generic over LinkType. DHLink plays two roles (DH-parameter carrier + compiled Link subclass); because Robot(BaseRobot[Link]) pins LinkType=Link, pyright can't see DH parameters on links inside DHRobot, causing 67+ type errors. Fix: class Robot(BaseRobot[RobotLinkType], RobotKinematicsMixin) generic, pinned per subclass (DHRobot(Robot[DHLink]), PoERobot(Robot[PoELink]), RobotURDF(Robot[Link])). Eliminates links property overrides and # type: ignore[union-attr] workarounds.

  • Evaluate "one Robot class, polymorphic Link.A(q)" design. Core idea: Link is an abstract base with A(q) -> SE3; DHLink/PoELink/ETSLink each implement it their own way (DH formula / exp(S*q) / ETS eval, dispatching to fknm for speed). Robot becomes a single class holding list[Link], with factory constructors (Robot.DH(...), Robot.ETS(...), Robot.PoE(...), Robot.URDF(...)) instead of subclasses. Removes ERobot (currently dead: a 5-line pass-through alias for Robot) and the Generic-iterator problem entirely. Robot is arguably a poor name for what's specifically an ETS-based robot -- ETSRobot would be accurate but is an API break; consider for a major version, keeping Robot as a deprecated alias. Preserve the fknm C-extension batch-FK path (robot._fkine_fknm(q)) as an optimised overload when all links are ETSLink -- 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 _Ts cache. 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 calling self._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 _Ts to 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 of Robot.__init__ for that construction path, or documenting it as a required convention for model authors.

  • Decouple Robot/Link kinematic state from scene-graph/rendering state. Directly related to desiderata.md's "Stateless over stateful" aspiration (.q retained as persistent state, "not yet achieved"). Robot/Link currently carry not just .q but SceneNode/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/Link become 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, plausibly base/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 explicit q (via fkine_all + each geometry's fixed local offset), verified bit-for-bit against the old SceneNode-based path to ~1e-9 precision (test_Robot.py::test_fkine_geometry_matches_scene_graph). Swift's env.add() now returns a handle (AssemblyHandle, generalized from RobotHandle) that owns q/qd and calls through to fkine_geometry -- Swift's hot render path no longer depends on SceneNode mutation. Robot/Link themselves are still unchanged (still carry .q, still have SceneNode); this is the pure alternative living alongside the stateful path, not a replacement.
    • Not yet made pure: gripper joints (fkine_geometry still reads gripper.q as ordinary state, not a parameter).
    • Remaining: actually remove SceneNode from Robot/Link, update PyPlot/teach to the handle-based model, decide base/tool ownership (see below).
  • Resolve base/tool ownership. Some models genuinely need base/tool as part of their kinematic definition (e.g. a fixed pedestal offset); for others it's purely "instance placement," indistinguishable from q. No formalized answer yet -- needed before the handle redesign above can be completed (does the instance handle own base/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 by fkine_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()'s mdh-based misuse guard once the hierarchy above is resolved. Current guard (assert getattr(self, "mdh", True), ...) checks the mdh attribute rather than class identity, because joint-last compliance tracks the DH convention in use (DHLink._to_ets()'s MDH branch), not the DHRobot class itself -- a class-name-based check would wrongly reject a compliant mdh=True DHRobot. If the hierarchy redesign above lands (one Robot class, or DHRobot stops subclassing Robot), this question may become moot or need re-deriving structurally rather than via a runtime attribute check.

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtKnown technical debt / deferred cleanup, not a live bug

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions