Repository navigation
Mid360: describe itself as USD over a get_usd RPC - #5109
Open
jeff-hykin wants to merge 92 commits into
Open
jeff-hykin wants to merge 92 commits into
jeff-hykin wants to merge 92 commits into
Conversation
Makes the MID-360 usable as a UrdfManager mount: the manager asks each mounted module for its own subtree and grafts the answer onto the frame it is bolted to. The layer is baked into a constant rather than converted on the fly. usd-core is the scene-cooking extra and has no linux-aarch64 wheel, and this driver's whole job happens on a Jetson, so describing itself must not cost it a USD dependency -- there is a test for exactly that. Its source is mid360.urdf next to the module, committed so the one-off is reproducible and reviewable, with the regeneration recipe in its header. A test regenerates from it and compares, because a generated constant drifts from its source silently otherwise. That test earned its place immediately by catching an invalid XML comment in the URDF. The frame names are substituted at call time from frame_id / imu_frame_id: a layer naming frames nobody publishes is useless, and mid360_for_pointlio already overrides frame_id. The placeholders sit only in the dimos:link / dimos:joint attributes, which are the names that travel (graft_usd matches a mount frame on dimos:link); prim names stay fixed USD identifiers, so nothing here has to sanitise a name and nothing needs pxr. What the geometry is worth: the 65 x 65 x 60 mm shell and the 265 g are from the Livox specs, and the IMU offset is PointLioConfig.extrinsic_t negated, so that stays in step with the in-repo source of truth. The shell's -20 mm z offset is inferred from the IMU offset and the body height and is marked PLACEHOLDER in the URDF; Livox only publishes the origin in a dimensioned drawing. It moves the drawn box and nothing measured, so no test asserts it.
![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: ece3371 | Previous: 5bc57a0 | Ratio |
|---|---|---|---|
peak memory |
1752.105 MB |
1776.91 MB |
0.99 |
peak threads |
371 threads |
376 threads |
0.99 |
network (transport) |
2723.967 MB |
2730.972 MB |
1.00 |
disk write |
1.82 MB |
1.59 MB |
1.14 |
instructions |
137.949 G |
138.52 G |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## jeff/feat/usd_manager_namespace #5109 +/- ##
================================================================
Coverage 82.46% 82.47%
================================================================
Files 1872 1872
Lines 184719 184719
Branches 15505 15505
================================================================
+ Hits 152333 152343 +10
+ Misses 28400 28391 -9
+ Partials 3986 3985 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Follows the base branches off custom attributes: the constant now carries its frame names on displayName, so the placeholders moved there too, and the stand-in root in the tests grew the RigidBodyAPI that marks a prim as a link. Regenerated from mid360.urdf rather than edited by hand, which the drift-guard test confirms. A new test asserts the baked layer contains no `custom ` and no `dimos:` either, matching the one on urdf_to_usd's own output.
codecov flagged the one uncovered line in module.py: get_usd's body. The tests went through mid360_usd directly to stay off the NativeModule's rust build, which left the rpc untested. A sync @rpc is tagged in place rather than wrapped, so it can be called with anything carrying a config. That covers the line and, more usefully, pins the argument order: both frames are strings, so swapping them would be silent. codecov's other 23 "missing" lines are in test_usd.py itself, and they are the pxr-guarded tests that importorskip on the CI legs without the scene-cooking extra. That is the lazy import working as intended, so there is nothing to fix there.
Follows the base branch's new shape: [asset_root, usd]. The root is resolved at call time from this module's own directory, which is where mid360.urdf and anything it might name live. It is deliberately not baked into the constant: that is written on a build machine, whose paths mean nothing on a robot. Today the layer names no files, so the root buys nothing. It is still the honest answer, and it is the one that stays right if the description grows a mesh.
mustafab0
reviewed
Nov 7, 2026
Review asked for it (mustafab0 on #5109) and that matches what "manually, one-off" meant: don't commit the intermediate. The constant was always the thing being shipped; the URDF beside it was a second description of the same sensor that could drift out of step with it. Its provenance moves into the comment above the constant, which is now where the record lives: the shell dimensions and their Livox source, the IMU offset and the PointLioConfig.extrinsic_t it is derived from, which figure is a placeholder and why, and that the root link is the point cloud origin rather than the mounting face. That costs the drift-guard test, since the source it compared against is gone. Also drops the assertion that the asset root contains mid360.urdf -- the root is still this module's directory, which is the only one the driver can name and be right about wherever it is installed.
Nabla7
requested review from
Dreamsorcerer,
TomCC7,
aclauer,
paul-nechifor and
spomichter
as code owners
November 10, 2026 02:54
Contributor
 [Medium impact] The PR appears safe to merge. SummaryAdds a
No actionable issues found. Tests were reviewed but not run. Reviews (1) · Last reviewed commit: "Merge branch 'jeff/feat/usd_manager_name..." · Reviewed by Greptile |
github-actions
Bot
added
the
ready-to-merge
github-actions
Bot
added
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
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: "Please make PR 5109 empty (do all lidar work in a later PR)". This branch now has a zero-file diff against usd_manager_namespace. `mid360_usd()` and the `get_usd` RPC go to #4911 `sim2_lidar`, which is where they are first read: `dimos/sim2/livox.py` there does `from dimos.hardware.sensors.lidar.livox.module import mid360_usd`, and it is the only thing in the stack that does. `mid360_usd` does not exist below here, and usd_manager_rename, shared_config, sim2, sim2_physics and sim2_camera have no reference to it outside the livox directory, so nothing between the two branches goes without it. The simulated blueprints mount `LivoxSim`, never the hardware `Mid360`, which is why nothing wanted it earlier.
jeff-hykin
added a commit
that referenced
this pull request
Nov 11, 2026
…ve up Jeff asked for #5109 to be empty, with the lidar work in a later PR. This is the PR that reads it: `dimos/sim2/livox.py` here imports `mid360_usd`, and without these two files this branch does not import at all (`ImportError: cannot import name 'mid360_usd'`). The files arrive renamed, since this branch sits above #5111: the mount-contract test now imports `dimos.robot.usd_manager` and is named for it.
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.
Gives the Livox Mid360 a
get_usdRPC returning an inline USD description of the sensor, so it can be mounted onto a robot tree.