Skip to content

UrdfManager: URDF self-masking and joint tf - #4612

Open
jeff-hykin wants to merge 6 commits into
mainfrom
jeff/feat/urdf_manager
Open

jeff-hykin wants to merge 6 commits into
mainfrom
jeff/feat/urdf_manager

Conversation

@jeff-hykin

Copy link
Copy Markdown
Member

UrdfManager poses a URDF from joint state, publishes its joints on tf and cuts the robot out of point clouds, replacing RoboPlanPointCloudSelfFilter (split out of jeff/feat/r1pro_nav).

…joints on tf, and cuts the robot out of point clouds (self-masking); replaces RoboPlanPointCloudSelfFilter (the xArm grasp blueprint uses it with its joint tf off), serves the URDF and its meshes on request (MeshFile), and logs a self-filter summary

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go2 replay realtime (arm64)

Details
Benchmark suite Current: 177b741 Previous: 4bee180 Ratio
peak memory 1580.176 MB 1566.871 MB 1.01
peak threads 355 threads 364 threads 0.98
network (transport) 2724.77 MB 2730.605 MB 1.00
disk write 1.461 MB 1.449 MB 1.01
instructions 115.497 G 115.402 G 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.50646% with 29 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/robot/urdf_manager.py 87.65% 20 Missing and 9 partials ⚠️
@@            Coverage Diff             @@
##             main    #4612      +/-   ##
==========================================
+ Coverage   81.21%   81.75%   +0.53%     
==========================================
  Files        1651     1641      -10     
  Lines      154436   153716     -720     
  Branches    13017    13024       +7     
==========================================
+ Hits       125431   125676     +245     
+ Misses      25424    24463     -961     
+ Partials     3581     3577       -4     
Components Coverage Δ
Tests 96.82% <100.00%> (+0.76%) ⬆️
Flag Coverage Δ
OS-macos-latest 75.74% <92.50%> (+0.04%) ⬆️
OS-ubuntu-24.04-arm 75.50% <92.48%> (+0.04%) ⬆️
OS-ubuntu-latest 75.86% <92.50%> (+0.04%) ⬆️
Py-3.10 75.85% <92.50%> (+0.04%) ⬆️
Py-3.11 75.84% <92.50%> (+0.04%) ⬆️
Py-3.12 75.86% <92.50%> (+0.04%) ⬆️
SelfHosted-Large 31.94% <29.19%> (-0.18%) ⬇️
SelfHosted-Linux 40.58% <53.22%> (+0.55%) ⬆️
SelfHosted-macOS 40.88% <53.48%> (+0.54%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/msgs/shape_msgs/MeshFile.py 100.00% <100.00%> (ø)
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
dimos/robot/galaxea/r1pro/test_joint_tf.py 100.00% <100.00%> (ø)
dimos/robot/manipulators/xarm/blueprints/grasp.py 85.71% <100.00%> (ø)
dimos/robot/test_urdf_manager.py 99.15% <100.00%> (ø)
dimos/robot/urdf_manager.py 87.65% <87.65%> (ø)

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jeff-hykin
jeff-hykin marked this pull request as ready for review October 16, 2026 15:05
…pture-time joint state and tf, point fields kept, late captures dropped) folds into it and point_cloud_self_filter.py goes; masking is optional (self_filter, on by default)
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[High impact] Renames and refactors the point cloud self-filter module.

Not safe to merge until mesh requests are processed without silently dropping distinct requested files.

Findings

  1. P1 Mesh requests disappear during bursts ▶

Summary

This PR replaces RoboPlanPointCloudSelfFilter with UrdfManager.

  • UrdfManager publishes model joint transforms from joint states.
  • The xArm map uses UrdfManager to filter robot returns.
  • URDF viewers can fetch meshes by package URI.

Fix the mesh-request delivery failure before merging: clients can remain waiting for files needed to complete the robot view.

Reviews (1) · Last reviewed commit: "UrdfManager: one module poses a URDF fro..." · Reviewed by Greptile

Comment thread dimos/robot/urdf_manager.py Outdated
Comment on lines +98 to +104
async def handle_mesh_request(self, request: String) -> None:
# Only meshes the URDF names: a request is never a path into the filesystem.
path = self._mesh_paths.get(request.data)
if path is None:
logger.warning(f"mesh_request for {request.data!r}, which is not a mesh in urdf_info")
return
self.mesh_info.publish(MeshFile(request.data, await asyncio.to_thread(path.read_bytes)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Mesh requests disappear during bursts

When a client requests several robot meshes, the automatically connected handler keeps only the latest waiting request. While this handler awaits a file read, requests for other meshes overwrite each other and never receive a mesh_info response. A check through the actual stream sent four distinct requests while the first read was blocked; all four reached the input, but only the first and last files were returned. A client waiting for every mesh can therefore stall or display an incomplete robot.

This needs to be fixed before merging. Process mesh requests through a lossless queue rather than the latest-only dispatcher, and add a burst regression test through the actual stream wiring.

Artifacts

Executed mesh-request burst test

  • The authored test loads snapshot source and exercises real Module stream and dispatcher plumbing with a gated file read, making the dropped-request check reproducible.

Mesh service before PR #4612

  • Started the base snapshot's predecessor module and inspected its live stream declarations, showing that the mesh endpoint did not exist before this PR.

Mesh-request loss at PR #4612 head

  • Ran sequential controls followed by four distinct requests during a blocked read, showing that all requests arrived but only the first and last files were returned.

Mesh-request loss at PR #4379 head

Mesh-request loss at PR #4613 head

  • Executed the identical burst test against the supplied PR r1pro-nav: arm planning #4613 snapshot, showing that it still drops the two intermediate requests.

Executed source and higher-stack comparison

  • Captured git source, relevant blob identities, mesh-reference searches, and unchanged checkout refs, showing that neither higher-stack snapshot replaces the lossy implementation or adds a resolving caller.

View artifacts

T-Rex Ran code and verified through T-Rex

…erwise only at unchanged_tf_hz (4 Hz), not the whole tree at 30 Hz; the keepalive stays under twice the tightest tf lookup tolerance of its readers
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Oct 16, 2026
… and a mesh_request whose ?hash= is already current gets no reply, so a client can cache meshes and only fetch stale ones
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Oct 18, 2026
… that requests the name urdf_info gives (with its ?hash=) still gets the file
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Oct 18, 2026
…very 0.2 s), so a client that just connected doesn't wait out the 5 s period
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Oct 18, 2026

This branch was successfully deployed

1 active (outdated) deployment
cachix — 3e692857 Deployed Oct 18, 2026 by jeff-hykin via cachix-build #13253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant