mujoco sim module by jca0 · Pull Request #1035 · dimensionalOS/dimos

jca0 · 2026-01-16T01:33:59Z

mujoco simulation module

created

mujoco backend to bring up mujoco
mujoco manipulation interface to easily use with other robots
mujoco driver to make it a module/determine topics to publish and subscribe to

mujoco parses XML file to determine robot and environment

command to test: python3 -m dimos.simulation.sim_blueprints

todos:

fix mac mujoco viewer issue

greptile-apps · 2026-01-16T01:39:25Z

Greptile Overview

Greptile Summary

Implemented a comprehensive MuJoCo simulation module that provides a simulation backend for robotic manipulators. The implementation includes:

MuJoCo engine backend (mujoco_engine.py) with threaded simulation loop, proper state synchronization using locks, and support for position/velocity/effort control modes
Simulation-agnostic manipulator interface (sim_manip_interface.py) that wraps the engine and provides a uniform API matching hardware manipulator interfaces
Module wrapper (sim_module.py) with separate control and monitor loops for publishing joint states and robot states via LCM transport
XML parser (xml_parser.py) that extracts joint/actuator mappings from MuJoCo XML files, supporting joints, tendons, and actuators
Blueprint configuration for xarm7 simulation with end-to-end tests

The architecture is clean with clear separation of concerns: engine layer handles physics simulation, interface layer provides hardware abstraction, and module layer handles messaging/transport.

Confidence Score: 4/5

This PR is safe to merge with minor concerns about edge cases
The implementation is well-structured with proper threading, locking, and error handling. The code follows good practices with abstract base classes, clear separation of concerns, and comprehensive tests. Previous review comments identified issues that should be addressed, but the core architecture and implementation are sound.
No files require special attention - the previously identified issues in review threads should be addressed

Important Files Changed

Filename	Overview
dimos/simulation/engines/mujoco_engine.py	Core MuJoCo simulation engine - well-structured with proper threading, locking, and state management
dimos/simulation/engines/base.py	Clean abstract base class defining simulation engine interface
dimos/simulation/manipulators/sim_manip_interface.py	Adapter wrapper providing uniform manipulator API over simulation engine
dimos/simulation/manipulators/sim_module.py	Module wrapper with control/monitor loops and message handling
dimos/simulation/utils/xml_parser.py	Robust XML parser for MuJoCo joint/actuator metadata with proper recursion handling

Sequence Diagram

sequenceDiagram
    participant User
    participant SimModule as SimulationModule
    participant SimManip as SimManipInterface
    participant MujocoEngine as MujocoEngine
    participant MuJoCo as MuJoCo Library

    User->>SimModule: start()
    SimModule->>SimManip: connect()
    SimManip->>MujocoEngine: connect()
    MujocoEngine->>MuJoCo: load XML model
    MuJoCo-->>MujocoEngine: model + data
    MujocoEngine->>MujocoEngine: start sim_loop thread
    MujocoEngine-->>SimManip: connected
    SimManip-->>SimModule: connected
    SimModule->>SimModule: start control_loop thread
    SimModule->>SimModule: start monitor_loop thread

    loop Control Loop
        User->>SimModule: publish JointCommand
        SimModule->>SimModule: _on_joint_position_command
        SimModule->>SimManip: write_joint_positions
        SimManip->>MujocoEngine: write_joint_command
        MujocoEngine->>MujocoEngine: update targets
        MujocoEngine->>MuJoCo: mj_step
        MujocoEngine->>SimManip: read_joint_positions
        SimManip-->>SimModule: positions
        SimModule->>User: publish JointState
    end

    loop Monitor Loop
        SimModule->>SimManip: read_state
        SimManip-->>SimModule: state data
        SimModule->>User: publish RobotState
    end

greptile-apps

_{8 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

dimos/simulation/manipulators/mujoco_manip_interface.py

greptile-apps · 2026-01-16T01:39:25Z

dimos/simulation/manipulators/mujoco_driver.py

+    def _on_joint_velocity_command(self, msg: JointCommand) -> None:
+        with self._command_lock:
+            self._pending_velocities = list(msg.positions)
+            self._pending_positions = None


syntax: JointCommand doesn't have a velocities field - it only has positions. This will cause an AttributeError when velocity commands are received.

Suggested change

def _on_joint_velocity_command(self, msg: JointCommand) -> None:

with self._command_lock:

self._pending_velocities = list(msg.positions)

self._pending_positions = None

def _on_joint_velocity_command(self, msg: JointCommand) -> None:

with self._command_lock:

self._pending_velocities = list(msg.positions)

self._pending_positions = None

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

greptile-apps

_{8 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

dimos/simulation/manipulators/mujoco_manip_interface.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

dimos/simulation/manipulators/sim_manip_interface.py

mustafab0 · 2026-01-22T21:14:07Z

dimos/simulation/plugins/mujoco_xarm.py

This is not a scalable approach. Having to build plugin for each oem manufacturer + sim engine is not maintainable.

The robot arms should not be specific to any SimEngine

mustafab0 · 2026-01-22T21:16:59Z

dimos/simulation/plugins/mujoco_xarm.py

+def _xarm_limits(dof: int) -> JointLimits:
+    if dof == 7:
+        lower_deg = [-360, -118, -360, -233, -360, -97, -360]
+        upper_deg = [360, 118, 360, 11, 360, 180, 360]
+    elif dof == 6:
+        lower_deg = [-360, -118, -225, -11, -360, -97]
+        upper_deg = [360, 118, 11, 225, 360, 180]
+    else:
+        lower_deg = [-360, -118, -225, -97, -360]
+        upper_deg = [360, 118, 11, 180, 360]
+
+    lower_rad = [math.radians(d) for d in lower_deg[:dof]]
+    upper_rad = [math.radians(d) for d in upper_deg[:dof]]
+    max_vel_rad = math.radians(180.0)
+    return JointLimits(
+        position_lower=lower_rad,
+        position_upper=upper_rad,
+        velocity_max=[max_vel_rad] * dof,
+    )


The sim module should get a Robot config that has all its limits, assets etc.

Robot shouldn't need a registry to be loaded, but should be defined in the blueprint

spomichter · 2026-01-22T21:03:38Z

dimos/simulation/engines/base.py

+    model: str | None = None
+
+
+class SimulationEngine(ABC):


spomichter · 2026-01-22T21:04:39Z

dimos/simulation/engines/base.py

+
+
+@dataclass(frozen=True)
+class RobotSpec:


Shouldn't this extend some existing general robot spec @mustafab0 ? Yeah pretty sure this should subclass, and shouldnt this take urdf @jca0

spomichter · 2026-01-22T21:10:47Z

dimos/simulation/engines/mujoco_engine.py

+            for i in range(limit):
+                self._joint_position_targets[i] = float(positions[i])
+
+    def write_joint_velocities(self, velocities: list[float]) -> None:


Needs to be changed to list of some velocity type. I assume Vector3

doesn't make sense to. Vector3 is 3D, Twist is 6D, joints can only go in 2 directions

Ideally a JointState message

Both write_joint_position & write_joint_velocities can be refactored into 1 write_joint_command/etc and take in a standard JointState message that can set Pos/Vel/Effort

spomichter · 2026-01-22T21:24:07Z

dimos/simulation/manipulators/sim_module.py

+            self._pending_positions = None
+
+    def _control_loop(self) -> None:
+        period = 1.0 / max(self._control_rate, 1.0)


The second 1.0 here is the minimum control rate so should be an env var called MIN CONTROL RATE or something

spomichter · 2026-01-22T21:24:19Z

dimos/simulation/manipulators/sim_module.py

+            time.sleep(period)
+
+    def _monitor_loop(self) -> None:
+        period = 1.0 / max(self._monitor_rate, 1.0)


Same as above here

spomichter · 2026-01-22T21:29:53Z

dimos/simulation/manipulators/sim_module.py

+        return [f"{self._joint_prefix}{i + 1}" for i in range(dof)]
+
+
+def get_blueprint() -> dict[str, Any]:


Just return blueprint how we normally do idk if we support returning blueprint via dict like this or if converting this back to .blueprint() type even works.

spomichter · 2026-01-22T21:30:39Z

dimos/simulation/manipulators/sim_module.py

+                    joints=[float(p) for p in positions],
+                )
+            )
+            time.sleep(period)


time.sleep(period) ignores work time; loop runs slower than target.

spomichter · 2026-01-22T21:31:25Z

dimos/simulation/plugins/mujoco_xarm.py

+def _xarm_limits(dof: int) -> JointLimits:
+    if dof == 7:
+        lower_deg = [-360, -118, -360, -233, -360, -97, -360]
+        upper_deg = [360, 118, 360, 11, 360, 180, 360]
+    elif dof == 6:
+        lower_deg = [-360, -118, -225, -11, -360, -97]
+        upper_deg = [360, 118, 11, 225, 360, 180]
+    else:
+        lower_deg = [-360, -118, -225, -97, -360]
+        upper_deg = [360, 118, 11, 180, 360]
+
+    lower_rad = [math.radians(d) for d in lower_deg[:dof]]
+    upper_rad = [math.radians(d) for d in upper_deg[:dof]]
+    max_vel_rad = math.radians(180.0)
+    return JointLimits(
+        position_lower=lower_rad,
+        position_upper=upper_rad,
+        velocity_max=[max_vel_rad] * dof,
+    )
+
+


spomichter · 2026-01-22T21:49:58Z

@jca0 Plus these changes add a blueprint to add_blueprints so devs can easily run, and then need an e2e test that runs everything and puts the arm through some motions, as well as a few unit tests where needed

paul-nechifor · 2026-01-24T05:12:03Z

dimos/simulation/manipulators/sim_module.py

+        if not self.config.engine:
+            raise ValueError("engine is required for SimulationModule")


If engine is required, then its type should be str, not str | None = None.

There's no need to verify it here if you disallow the class from being instantiated with an invalid config.

paul-nechifor · 2026-01-24T05:33:17Z

dimos/simulation/manipulators/sim_module.py

+        engine_key = self.config.engine.lower()
+        if engine_key not in self._ENGINES:
+            raise ValueError(f"Unknown simulation engine: {self.config.engine}")


I think it's better to use the type system.

EngineType = Literal["mujoco", "some_other"] _ENGINES: dict[EngineType, SimulationEngine] = { "mujoco": MujocoEngine, } ... # No need for None check, lower(), or key existence. engine_cls = self._ENGINES[self.config.engine]

paul-nechifor · 2026-01-24T05:42:23Z

dimos/simulation/manipulators/sim_module.py

+        except Exception as exc:
+            import logging
+
+            logging.getLogger(self.__class__.__name__).warning(


We use logger = setup_logger() for logging.

But what exceptions are you expecting here? I would expect just this to work without if and try:

self._disposables.add( Disposable( self.joint_position_command.subscribe(self._on_joint_position_command) ) )

If that doesn't work, it would be good to know why.

paul-nechifor · 2026-01-24T06:04:35Z

dimos/simulation/manipulators/sim_manip_interface.py

+        return self._error_code, self._error_message
+
+    def write_joint_positions(self, positions: list[float], velocity: float = 1.0) -> bool:
+        _ = velocity


Is this to silence the velocity parameter not being used? If so, it's better to rename it to _velocity. I assume it's required in the interface?

Oh, velocity isn't used at all. Just delete it?

paul-nechifor · 2026-01-24T06:15:55Z

dimos/simulation/manipulators/sim_manip_interface.py

+        if hasattr(self._engine, "hold_current_position"):
+            self._engine.hold_current_position()  # type: ignore[attr-defined]
+            return True


Checking if a method exists with hasattr is often a sign that the code isn't ideal.

It looks like hold_current_position exists on MujocoEngine, but not on SimulationEngine and you're using hasattr to break the abstraction.

The better way to accomplish this (adding special handling just for Mujoco) without ignoring types is to check if self._engine is an instance of MujocoEngine .

But isn't it better to just add hold_current_position to SimulationEngine?

paul-nechifor · 2026-01-24T06:35:09Z

dimos/simulation/engines/mujoco_engine.py

+        with self._lock:
+            pos_targets = list(self._joint_position_targets)
+        with self._lock:


Why two? Seems like you're releasing and acquiring unnecessary the second time.

paul-nechifor · 2026-01-24T07:04:52Z

dimos/simulation/engines/mujoco_engine.py

+            limit = min(len(positions), self._num_joints)
+            for i in range(limit):
+                self._joint_position_targets[i] = float(positions[i])


I see this pattern a lot in the code where you're copying values only up to a point.

But I think this just silently ignores errors. Is there a valid reason why write_joint_positions would be sent more positions than can be stored? I think you should let the code throw.

dimos/e2e_tests/test_simulation_module.py

Co-authored-by: Paul Nechifor <paul@nechifor.net>

spomichter · 2026-02-12T14:24:09Z

@greptile

greptile-apps

_{12 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

jca0 added 4 commits January 15, 2026 13:45

add original files

07c18f6

rebased with dev for new controller orchestrator arch

4821255

clean up code + comments

b9adc9b

tests for testing

ac1c647

jca0 requested a review from a team January 16, 2026 01:34

greptile-apps bot reviewed Jan 16, 2026

View reviewed changes

jca0 marked this pull request as draft January 16, 2026 01:59

jca0 and others added 3 commits January 15, 2026 23:41

address race condition

ca898c5

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

fix mypy errors

8116d1f

Merge branch 'jing-sim' of github.com:dimensionalOS/dimos into jing-sim

d7fd12e

jca0 marked this pull request as ready for review January 16, 2026 07:44

greptile-apps bot reviewed Jan 16, 2026

View reviewed changes

dimos/simulation/manipulators/mujoco_manip_interface.py Outdated Show resolved Hide resolved

jca0 and others added 10 commits January 15, 2026 23:50

syntax error :(

3461348

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

CI code cleanup

f46ec17

skeleton for modular sim arch

97508ee

mujoco engine/bringup script

89f462a

manipulation interface that plugs into different sims

6073d80

simulation model that supports any sim engine

328a019

add support for xarm in sim

c153c13

clean up code

6d9e135

more cleanup

86a3c06

mypy error fix

4121640

mustafab0 requested changes Jan 22, 2026

View reviewed changes

spomichter requested changes Jan 22, 2026

View reviewed changes

jca0 added 4 commits January 22, 2026 16:51

delete joint limits

a2b66f7

remove get_blueprint() and set constant

91cd6a3

sim module takes in xml file and parses it for robot properties

bcbe1be

Track xarm7 assets with LFS

f392af0

paul-nechifor reviewed Jan 24, 2026

View reviewed changes

consolidate write commands into one and fix velocity control

7fec092

paul-nechifor reviewed Jan 24, 2026

View reviewed changes

jca0 added 2 commits January 23, 2026 22:46

config_path changed to Path type and mandatory

104c4fe

engine typing

e62a249

paul-nechifor reviewed Jan 24, 2026

View reviewed changes

jca0 and others added 10 commits January 23, 2026 23:17

typing fix, no default so set kw_only=True

22d71ec

add hold_current_position() to base

f06fb46

fix locks and make sure array length is as expected

7a6662d

revise e2e tests to spin up blueprint

bc98dd3

fix sim module tests

499ede5

Merge branch 'dev' into jing-sim

ede93fc

fix mypy errors

2c52669

fetch LFS when module starts

645a5f0

Merge branch 'jing-sim' of github.com:dimensionalOS/dimos into jing-sim

6ced566

clean up

9f5f919

paul-nechifor reviewed Jan 25, 2026

View reviewed changes

dimos/e2e_tests/test_simulation_module.py Show resolved Hide resolved

jca0 and others added 4 commits January 25, 2026 03:16

Update test_simulation_module.py

2245752

Co-authored-by: Paul Nechifor <paul@nechifor.net>

Merge branch 'dev' into jing-sim

d3962cd

remove duplicated blueprint

64f5979

run all blueprints generation test

001a053

spomichter and others added 2 commits February 12, 2026 06:24

Merge branch 'dev' into jing-sim

5d62c4f

CI code cleanup

a527d07

greptile-apps bot reviewed Feb 12, 2026

View reviewed changes

fixed all blueprints

de58166

		return [f"{self._joint_prefix}{i + 1}" for i in range(dof)]


		def get_blueprint() -> dict[str, Any]:

		if not self.config.engine:
		raise ValueError("engine is required for SimulationModule")

Conversation

jca0 commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spomichter commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-nechifor Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-nechifor Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spomichter commented Feb 12, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

jca0 commented Jan 16, 2026 •

edited

Loading

greptile-apps bot commented Jan 16, 2026 •

edited

Loading

paul-nechifor Jan 24, 2026 •

edited

Loading

paul-nechifor Jan 24, 2026 •

edited

Loading