Skip to content

Pim Sim2 [1/9]: Reuse ROS joint commands and sensor headers - #5033

Open
Nabla7 wants to merge 2 commits into
jeff/feat/usd_manager_renamefrom
pim/feat/sim2-e-messages
Open

Nabla7 wants to merge 2 commits into
jeff/feat/usd_manager_renamefrom
pim/feat/sim2-e-messages

Conversation

@Nabla7

@Nabla7 Nabla7 commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Integrate Option G codecs directly above Jeff's USD-manager prerequisite (#5111), preserving ROS JointCommand and sensor/TF schemas with the pinned dimos-lcm #31 and native MuJoCo 4.0 USD support.

@codecov

codecov Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@                       Coverage Diff                        @@
##           jeff/feat/usd_manager_rename    #5033      +/-   ##
================================================================
+ Coverage                         82.46%   83.80%   +1.33%     
================================================================
  Files                              1872     1879       +7     
  Lines                            184720   185634     +914     
  Branches                          15505    15389     -116     
================================================================
+ Hits                             152338   155566    +3228     
+ Misses                            28396    26039    -2357     
- Partials                           3986     4029      +43     
Components Coverage Δ
Tests 96.55% <100.00%> (+1.63%) ⬆️
Flag Coverage Δ
OS-macos-latest 78.17% <100.00%> (+0.95%) ⬆️
OS-ubuntu-24.04-arm 76.92% <100.00%> (-0.06%) ⬇️
OS-ubuntu-latest 78.30% <100.00%> (+0.95%) ⬆️
Py-3.10 77.28% <100.00%> (-0.07%) ⬇️
Py-3.11 78.28% <100.00%> (+0.95%) ⬆️
Py-3.12 78.29% <100.00%> (+0.95%) ⬆️
SelfHosted-macOS 40.18% <32.53%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
...ntrol/servo_control/cartesian_motion_controller.py 61.05% <100.00%> (ø)
.../servo_control/test_cartesian_motion_controller.py 100.00% <100.00%> (ø)
...ajectory_controller/joint_trajectory_controller.py 48.19% <100.00%> (ø)
...ory_controller/test_joint_trajectory_controller.py 100.00% <100.00%> (ø)
dimos/msgs/sensor_msgs/Image.py 73.52% <ø> (+4.22%) ⬆️
dimos/msgs/sensor_msgs/PointCloud2.py 78.08% <ø> (ø)
dimos/msgs/sensor_msgs/test_PointCloud2.py 98.85% <100.00%> (+0.03%) ⬆️
dimos/msgs/sensor_msgs/test_image.py 100.00% <100.00%> (ø)
dimos/msgs/test_lcm_definitions.py 100.00% <ø> (ø)

... and 80 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.

@Nabla7 Nabla7 changed the title sim2 [1/9]: Integrate acquisition-stamped messages and Option E codecs sim2 [1/9]: Integrate acquisition-stamped messages and Option F codecs Nov 4, 2026
@Nabla7 Nabla7 changed the title sim2 [1/9]: Integrate acquisition-stamped messages and Option F codecs sim2 [1/9]: Integrate acquisition-stamped messages and Option G codecs Nov 5, 2026
@Nabla7 Nabla7 changed the title sim2 [1/9]: Integrate acquisition-stamped messages and Option G codecs sim2 [1/9]: Reuse standard sensor headers with Option G messages Nov 5, 2026

@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: 467453a Previous: 5bc57a0 Ratio
peak memory 1743.148 MB 1776.91 MB 0.98
peak threads 371 threads 376 threads 0.99
network (transport) 2725.802 MB 2730.972 MB 1.00
disk write 1.828 MB 1.59 MB 1.15
instructions 137.132 G 138.52 G 0.99

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

@Nabla7 Nabla7 changed the title sim2 [1/9]: Reuse standard sensor headers with Option G messages sim2 [1/9]: Reuse ROS joint commands and standard sensor headers Nov 5, 2026
@Nabla7 Nabla7 changed the title sim2 [1/9]: Reuse ROS joint commands and standard sensor headers Pim Sim2 [1/9]: Reuse ROS joint commands and sensor headers Nov 10, 2026
@Nabla7
Nabla7 removed this pull request from stack #5042 November 10, 2026 00:07
@Nabla7
Nabla7 changed the base branch from jeff/feat/shared_config to pim/feat/sim2-usd-shared-config November 10, 2026 00:07
@Nabla7
Nabla7 added this pull request to stack #5154 November 10, 2026 00:07
@Nabla7
Nabla7 marked this pull request as ready for review November 10, 2026 00:27
@greptile-apps

greptile-apps Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes reach manipulator command paths, sensor timestamp decoding, and optional simulation installs.

The reviewed changes appear safe to merge, with a non-blocking fix needed for JPEG tests on machines without libturbojpeg.

Findings

  1. P2 JPEG tests miss library skip ▶

Summary

Replaces the local joint-command type with control_msgs.JointCommand.

  • Manipulator controllers send named ROS joint position commands.
  • Image and point-cloud decoders preserve sub-second timestamps.
  • The optional simulation install includes USD support.

Nabla7 explicitly identifies the draft dimos-lcm Git pin as a pre-merge release dependency. That acknowledged work is not reported as a finding.

Reviews (1) · Last reviewed commit: "Respect USD wheel availability in the si..." · Reviewed by Greptile

Comment on lines +54 to +55
@pytest.mark.parametrize("stamp", [0.0, 0.125])
def test_lcm_round_trip_preserves_zero_and_subsecond_timestamps(stamp):

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.

P2 JPEG tests miss library skip

The new timestamp test calls lcm_jpeg_encode() without @pytest.mark.skipif_no_turbojpeg. If native libturbojpeg is missing on a developer's machine, both cases fail instead of skipping, unlike the existing JPEG test below. Add the same marker, or split the raw and JPEG checks so the raw checks still run.

Suggested change
@pytest.mark.parametrize("stamp", [0.0, 0.125])
def test_lcm_round_trip_preserves_zero_and_subsecond_timestamps(stamp):
@pytest.mark.skipif_no_turbojpeg
@pytest.mark.parametrize("stamp", [0.0, 0.125])
def test_lcm_round_trip_preserves_zero_and_subsecond_timestamps(stamp):

@Nabla7
Nabla7 force-pushed the pim/feat/sim2-e-messages branch from 3bdf8db to 42a0979 Compare November 11, 2026 02:47
@Nabla7
Nabla7 requested a review from bogwi as a code owner November 11, 2026 02:47
@Nabla7
Nabla7 removed this pull request from stack #5154 November 11, 2026 02:48
@Nabla7
Nabla7 changed the base branch from pim/feat/sim2-usd-shared-config to jeff/feat/usd_manager_rename November 11, 2026 02:48
@Nabla7
Nabla7 added this pull request to stack #5185 November 11, 2026 02:49

This branch was successfully deployed

No deployments
cachix — 3bdf8dbd Deployed Nov 10, 2026 by Nabla7 via cachix-build-macos #16658
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant