Repository navigation
UrdfManager: urdf_to_usd, graft_usd, and an async get_scene of the whole physical tree - #5102
Open
jeff-hykin wants to merge 74 commits into
Open
jeff-hykin wants to merge 74 commits into
jeff-hykin wants to merge 74 commits into
Conversation
USD is becoming the description format, so this is the bridge while the robots are still authored as URDF. A converted stage is one self-contained .usda string rather than a file: a description already travels as one string through UrdfManager.file_info, and a stage naming external layers would need a second resolution contract on the renderer side. Mesh triangles are therefore baked in by default; --mesh-mode reference leaves the paths for a consumer that fetches files itself. Links nest down the kinematic tree at the URDF zero pose, so a viewer with no physics engine still shows the robot assembled, and the joints are also written as UsdPhysics prims. physics:axis is only ever X/Y/Z, so an off-axis URDF axis is folded into both local rotations and the joint is authored about z. Prim names have to be USD identifiers, so dimos:link and dimos:joint carry the original URDF spelling; that is how a frame_id finds its prim when grafting. graft_usd copies specs rather than referencing layers, for the same reason, and rebases the copied subtree's relationship targets: a copied spec keeps its authored paths, so a subtree joint's physics:body0 would otherwise dangle. pxr is imported lazily and the CLI command imports it inside its body: usd-core is in the scene-cooking extra and ships no linux-aarch64 wheel, so importing this must cost a Jetson nothing.
… the whole tree physical_connections says what is physically bolted to the robot: a frame of its URDF -> the module that owns the subtree hanging off that frame. The manager owns the root URDF directly and asks each of those modules for its own USD, so get_scene answers with the whole tree as one layer. Nothing wires this up yet. The default is empty, tf and file serving are untouched by it, and every existing blueprint behaves exactly as before. get_scene is async and meant to be called before start(): it waits for every module in the map, because a scene missing an arm is worse than one that arrives late. A module that is not serving yet is a retry rather than a failure, capped by scene_timeout_s. The validation runs in __init__ so a blueprint that names a frame the URDF does not have, or a module with no get_usd, fails before deployment. The subtree clients are hand-rolled RPCClients rather than declared module refs: a ref is one annotated attribute resolved by type, and this is a mapping whose keys are only known per robot. That addresses a module by class name, so a module deployed under an instance_name is not reachable this way yet. get_usd takes its own lock, not the tf one: baking meshes into a USD takes seconds, and a renderer connecting must not stall the joint tf stream.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
go2 replay realtime (arm64)
Details
| Benchmark suite | Current: ea4c604 | Previous: 83800d9 | Ratio |
|---|---|---|---|
peak memory |
1765.973 MB |
1745.215 MB |
1.01 |
peak threads |
372 threads |
368 threads |
1.01 |
network (transport) |
2736.294 MB |
2730.954 MB |
1.00 |
disk write |
1.922 MB |
1.863 MB |
1.03 |
instructions |
138.732 G |
138.7 G |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #5102 +/- ##
==========================================
- Coverage 83.69% 82.60% -1.10%
==========================================
Files 1862 1870 +8
Lines 181466 183606 +2140
Branches 14956 15445 +489
==========================================
- Hits 151887 151674 -213
- Misses 25584 27956 +2372
+ Partials 3995 3976 -19
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 49 files with indirect coverage changes 🚀 New features to boost your workflow:
|
urdf_to_usd and graft_usd are the library surface this PR is for; a CLI on top of them is a separate question and does not need answering to land the conversion. dimos/cli/dimos.py goes back to what it was, so the lazy-pxr property the CLI startup tests cover is no longer this PR's to keep.
"Physical connections" said relationship where the field says location: the keys are
mount frames, so mounts={"left_mount": arm} reads as what it is. Same shape, same
behaviour, and nothing outside this branch sets it.
dimos/codebase_checks/test_no_sections.py forbids a comment wrapped in separator dashes: if a file needs sections, the sections should be files. The test names already say what each group covers, so the banner was only decoration.
mustafab0
reviewed
Nov 6, 2026
Comment on lines
+51
to
+53
| _NO_USD = ( | ||
| "URDF to USD needs usd-core (pxr), which is in the 'scene-cooking' extra and ships no " | ||
| "linux-aarch64 wheel. Install it with: uv sync --extra scene-cooking" |
Contributor
There was a problem hiding this comment.
let's put the usd-core dependency in simulation dependencies
Contributor
There was a problem hiding this comment.
Probably will become a core dependency as usd manager becomes a key module
mustafab0
reviewed
Nov 6, 2026
Two things in the layer were ours rather than USD's, and neither needed to be. A prim carried `custom string dimos:link` / `dimos:joint` to keep the original URDF spelling, because prim names are identifiers and "arm-1" has to become "arm_1". USD already has the field for this: displayName, whose whole purpose is a name identifier syntax cannot hold. It survives Sdf.CopySpec, so grafting still works, and it is settable at the layer level. A reader falls back to the prim name when no displayName is authored, since the two agree whenever the URDF name was already an identifier. Link prims are now found by their UsdPhysics.RigidBodyAPI rather than by our attribute, which is both standard and narrower than "any Xform": it excludes the defaultPrim standing for the robot and the joint prims, either of which could otherwise shadow a link of the same name. A `<mesh>` carried `custom asset dimos:meshFile`, or had its triangles baked in by trimesh. Both are gone for a standard reference to the path the URDF named. The baking only existed because I had decided a scene must be self-contained -- but UrdfManager already serves those exact paths over render_file_request, so resolution is the consumer's job and always was, the same as it already is for the URDF. That removes the trimesh dependency from this path, the mesh_mode / MeshMode API, and the usd_mesh_mode config field. Net: a consumer needs nothing of ours to read the layer. A test asserts the output contains no `custom ` and no `dimos:` at all, so this cannot quietly regress.
A mount is a module the blueprint says is physically there, so a mount that is not answering is a thing to wait out, not to give up on: half a scene would hand a renderer a robot with no arm. So there is no deadline, scene_timeout_s and the TimeoutError are gone, and scene_retry_s is the only knob left. Waiting forever silently is its own failure mode, so it warns every 20 asks.
A URDF carries more than a kinematic tree and some geometry: mass and inertia, gazebo blocks, transmissions, sensors, and whatever else a vendor put there. urdf_to_usd reads none of it, which was not a reason to lose it -- converting to USD should not also be a way to lose half a description. So each link and joint prim keeps the children of its element that this converter did not read, and the root prim keeps the whole original document, which covers the parts belonging to no link or joint at all. What is read is not duplicated: a <visual> became a Cube prim, so keeping its XML too would be two sources of truth. That all goes in customData, which is USD's own field for data a schema has no place for. It costs a reader nothing and it is not an attribute, so the standard-schemas-only property holds -- there is a test asserting the layer still contains no `custom `. Also drops this PR's edit to assets/README.md; the file goes back to what main has.
urdf_to_usd asked every joint where to put its child, and a floating or planar joint has no
single position to answer with -- that is the whole point of one, its pose comes from odometry
rather than from the model. So it raised, and any robot that walks was unconvertible:
ValueError: floating joint floating_base_joint has no single position
The g1 is exactly such a robot, which makes this the one most likely to be mounted on.
Its child now sits at the joint's origin, which is the honest stand-in for a pose that is not
in the model, and is also what parent_from_child(0) returns for every other joint type, so the
tree is posed consistently either way. _define_joint already left these joints out of the USD;
only the link placement was wrong.
No test caught it because every toy URDF here is bolted to the ground. There are three now.
An asset path only means something on the machine that wrote it, so a path alone cannot say
whether the file a consumer fetched is the file the description meant. A hash can.
urdf_to_usd computes them: each mesh prim gets assetInfo with USD's own reserved keys,
identifier for the path and version for the hash. Both are standard, so Usd.ModelAPI reads
them back and nothing of ours is needed to understand them. The algorithm is inside the value
("sha256:...") so it can change later without the old values becoming unreadable.
require_asset_hashes refuses a layer whose assets are not identified, which is how a layer
that no converter produced -- written by hand, or baked into a constant -- is held to the same
bar. Rather than only say no, the error carries the hash each asset should have been given,
computed on the spot wherever the file can still be read, so the answer is the fix ready to
paste. Where it cannot be read it says to hash it where the file lives, which is the only
honest thing a consumer can say about a file it does not have.
UsdManager applies it to every subtree it is handed. That one is refused rather than retried:
a subtree whose assets are unidentified will not become identified by asking again.
Cut the multi-line comment blocks and docstrings down to what is not obvious from the code: the long ones in assets/usd.py and urdf_manager.py are one or two lines now, and the header block above the module constants is gone. Also removes the whole-URDF copy on the root prim, which this PR had added and the next one took away again. Same mistake as the get_scene deadline: a thing introduced here belongs removed here, not two PRs later. The per-link and per-joint passthrough stays.
The URDF is this module's input, not its output. A render_file_request for a *.usd or *.usda now gets the whole scene when one has been assembled and this robot's own layer otherwise; a request for the .urdf gets nothing. The files the description names are served as before, since a USD layer resolves a mesh by the path it carries. Out-of-repo renderers ask for the .urdf today and have to change with this.
Collaborator
|
Looks good to me! |
pyproject now lists mujoco.* under ignore_missing_imports, which covers both import-not-found and import-untyped, so the inline ignore never fires and --strict reports it as unused.
github-actions
Bot
added
ready-to-merge
A frame added onto a prim that is not a rigid body is welded to the world: the joint names body1 and deliberately leaves body0 empty, which is the USD convention and what test_a_frame_added_to_something_that_is_not_a_link_is_welded _to_the_world pins. The converter then dropped that joint for 'not naming both bodies', so the office scene's three mount frames came out as three orphan roots and office_555 was never a link for tf_renames to turn into 'world' -- which is exactly what the controller's TF panel reported. A body0-less joint now reads as parented to the stage's default prim, and a body a joint names is admitted as a link even without RigidBodyAPI, which the scene's own furniture joints already relied on (slab_ground_joint: office_555 -> slab_ground). On the office scene the roots go from ['left','main','office_555','right'] to ['office_555'].
…s its fixed edges tf only ever went out from handle_coordinator_joint_state. The office scene's manager has no controller -- dimos_graph shows coordinator_joint_state wired to the three robot managers and nothing else -- so it never published anything, and the viewer saw three robot trees and no 'world' to hang them off. A thread now publishes the joints that cannot move at unchanged_tf_hz, through the same _edges_to_send keepalive. Moving joints are left out: they need a real joint state, and posing them at zero would be a lie.
A joint with no body0 keeps its offset in the child prim's own transform, not in LocalPos0 -- _weld_frame writes LocalPos0 and LocalPos1 as zero on purpose. Reading LocalPos0 for those joints dropped the placement, so the office mounts came out at the origin and the G1s stood 0.755 m into the floor with their lidar scanning from the wrong height. _welds_of already read the prim's placement for the same reason.
A converted asset often keeps its shader networks as root-level prims beside its default prim, and the prims inside reach them by absolute path -- g1.usda binds its materials to </Flattened_Prototype_1> and </Flattened_Prototype_2>. graft_usd copied the default prim alone, so those arcs went on pointing at whatever owned that path in the scene, and office_555 has root-level prototypes of its own: the grafted robot came out wearing the room's materials. Carry the prims a subtree reaches outside itself, under names the scene has not taken, and repoint its arcs at the copies. Arcs that point back inside the subtree move with it, which they did not before either. Rebase relationship targets by prim rather than by text while here, so that a sibling named arm_sub_2 is no longer caught by the prefix for arm_sub.
github-actions
Bot
added
the
ready-to-merge
main's Piper work added with_mesh_paths_replaced to RobotModel in the same place this branch added to_usd, so the two land next to each other; everything else merged on its own.
github-actions
Bot
removed
the
ready-to-merge
graft_usd walks the arcs of what it carries, so a shader network the subtree reaches through a second prim comes over too. Nothing exercised that second hop, and the scene owning prims by the same names is exactly when it matters.
The guard already made the string replace safe, but it reintroduced the very pattern the same commit removed from _rebase_relationships one function above. Use Sdf.Path.ReplacePrefix so it is correct on its face, which also makes the source string parameter redundant.
Nabla7
reviewed
Nov 11, 2026
| def start(self) -> None: | ||
| self.tfbuffer # noqa: B018 - Initialize TF before binding input handlers. | ||
| super().start() | ||
| self._stop_static_tf.clear() |
Collaborator
There was a problem hiding this comment.
todo: PR1 shouldn't edit urdf manager (utils only)
Nabla7
reviewed
Nov 11, 2026
| return self._topology[1] | ||
|
|
||
| def get_joint(self, name: str) -> JointDescription | None: | ||
| """Return the named joint when it exists.""" |
Collaborator
There was a problem hiding this comment.
todo: don't delete prexisting comments/docstrings
Pim, on #5102: "PR1 shouldn't edit urdf manager (utils only)". This reverts e51f020 (the static-tf thread) here; the file is byte-identical to main again, and the first PR touches nothing but dimos/robot/assets. The fixed-edge publishing it added is still needed -- the office scene has no controller, so nothing ever calls add_joint_state for it -- so it moves up to #5105, which is the PR that actually owns UrdfManager's behaviour.
Pim, on #5102: "todo: don't delete prexisting comments/docstrings". An earlier pass in this stack stripped comments and docstrings from every file it touched, including the ones it only edited in passing. model.py is pre-existing code, so 31 docstrings and 9 comments that main wrote went with it. Restored from main by qualified name, so only the ones whose code is still here came back; the ones belonging to code this PR replaced stayed out. The same stripping hit pre-existing files on five other branches in the stack and each is being restored on the branch that did it.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
Pim, on #5102: "todo: don't delete prexisting comments/docstrings". An earlier pass in this stack stripped comments and docstrings from every file it touched, including the ones it only edited in passing. model.py is pre-existing code, so 31 docstrings and 9 comments that main wrote went with it. Restored from main by qualified name, so only the ones whose code is still here came back; the ones belonging to code this PR replaced stayed out. The same stripping hit pre-existing files on five other branches in the stack and each is being restored on the branch that did it.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
Pim, on #5102: "don't delete prexisting comments/docstrings". This branch's comment-stripping pass took the module docstring and 15 field comments off Mid360Config -- the ones that say what host_ip=None derives, which fields a live sensor needs, what each point_format puts on the wire. None of that is derivable from the field names.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
Pim, on #5102: "don't delete prexisting comments/docstrings". The rename touched thirteen files that already existed on main, and this branch's stripping pass took their comments with it -- 29 docstrings and 24 comments, none of them about UrdfManager or UsdManager. The xarm teleop ones are the costly kind: which task drives which joint, what an app's twist timeout is for, that drive_joint is open at 0 rad. Restored from main by qualified name, so only the ones whose code is still here came back. One comment the tool could not place was checked by hand and is not lost: the "return every sample" note moved with max_candidates into grasp.py.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
Pim, on #5102: "don't delete prexisting comments/docstrings". This branch's stripping pass took 5 docstrings and 42 comments off a file it edits in passing, and they are the expensive kind to work out again: why LfsPath is lazy (get_data at import time blocks the whole CLI on a multi-GB pull), which geom groups the lidar renders and why, that 0.05 voxel resolution saturates the raytracer on the Orin, that the Jetson collapses a 500 Hz tick to ~90 Hz and starves the policy.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
Pim, on #5102: "don't delete prexisting comments/docstrings". The worst of it is here: this branch's stripping pass took 74 docstrings and 56 comments off ten files it edits in passing, almost all of them dimos/control -- the tick loop, connection_source, hardware_interface. That is the code that decides which task owns a joint and what happens when a robot stops answering, and the comments are the record of it: that a mode conflict at equal priority lets the first task keep the joint, that the tick timer increments from the last loop because macOS oversleeps by up to 25%, that every task value is range-checked before any hold changes because the connection module refuses a whole message. Restored from main by qualified name and by the line each comment sat above. Twelve the tool would not guess at were placed by hand, each checked against main: three in generate_rgba_texture and turbo_image whose code had been rewritten around them, and the rest where the same line of code appears twice in one function. Nothing of main's is left out -- the count is zero across all ten.
github-actions
Bot
added
the
ready-to-merge
This branch was successfully 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.
Converts a URDF into a self-contained USD layer and gives UrdfManager a
mountsmap plusget_usd/get_sceneRPCs so it can serve the USD of the whole physical tree, with every mesh asset required to carry a sha256 in itsassetInfo.