RosNav DockerModule

[jeff-hykin]

DRAFT: Docker PR needs to be reviewed first

File change count is so high because g1 blueprints were redone

Problem

G1 parity with Go2

Solution

Breaking Changes

How to Test

Contributor License Agreement

• I have read and approved the CLA.

[greptile-apps]

Greptile Summary

Large PR that achieves G1 parity with Go2 by refactoring the Docker module system, adding new G1 blueprints/effectors, and introducing a Docker-based ROS navigation module. The core Docker deployment pipeline is significantly improved with content-hash rebuilds, container reconnection, per-method RPC timeouts, and structured parallel deployment with cleanup on partial failure.

• Docker module overhaul: DockerModule now implements ModuleProxyProtocol, moves container lifecycle into __init__ with proper cleanup, adds reconnect support (docker_reconnect_container), and uses content-hash-based rebuild detection instead of image-exists checks
• RPC timeout refactor: Removed hardcoded 1200s start timeout from call_sync; timeout resolution is now per-method via rpc_timeouts dict on module classes, with RPCClient.default_rpc_timeout (120s) and RPCClient.start_rpc_timeout (1200s) as defaults
• Parallel deploy with cleanup: New safe_thread_map utility and DockerWorkerManager ensure that when parallel deployments partially fail, successfully-started modules/containers are cleaned up before raising ExceptionGroup
• G1 blueprints reorganized: Old G1 blueprints moved to legacy/ subdirectory; new blueprints added for agentic, basic, and perceptive configurations with MuJoCo and onboard variants
• G1 high-level effectors: New HighLevelG1Spec protocol with two implementations — G1HighLevelDdsSdk (native Unitree SDK2 over DDS for onboard) and G1HighLevelWebRtc (remote control)
• ROS navigation Docker module: New ROSNavModule (~940 lines) encapsulating full ROS2 nav stack in a Docker container with LCM↔ROS bridge, teleop override, and Dockerfile for amd64/arm64
• aarch64/Jetson fixes: Worker entrypoint preloads static-TLS native libs on aarch64 to prevent cannot allocate memory in static TLS block errors; is_jetson() utility added
• Note: PR is marked DRAFT — author mentions needing to remove irrelevant changes (like dot.py)

Confidence Score: 3/5

• This is a large DRAFT PR with solid architecture but a couple of latent issues in the deployment pipeline that should be addressed before merge.
• Score of 3 reflects the PR's large scope (76 files, +6682/-596 lines) with two notable issues: (1) RpcCall's default timeout=0 is a latent trap that will cause instant TimeoutError for any future caller that omits the argument, and (2) deploy_parallel's cross-group cleanup gap where successful worker modules aren't stopped when Docker deployment fails (or vice versa). The PR is also marked DRAFT by the author. The overall design is well-thought-out with good test coverage for the new parallel deployment logic.
• Pay close attention to dimos/core/rpc_client.py (timeout default), dimos/core/module_coordinator.py (cross-group cleanup in deploy_parallel), and dimos/web/websocket_vis/websocket_vis_module.py (debug logging).

Important Files Changed

Filename	Overview
dimos/core/docker_runner.py	Major refactor: DockerModule now implements ModuleProxyProtocol, container lifecycle moved into init with proper cleanup, reconnect support added, build-hash-based rebuild, and per-method RPC timeout resolution. Well-structured with good error handling.
dimos/core/docker_build.py	Added content-hash-based rebuild detection using Docker image labels, configurable build timeout for long aarch64 builds, and extra build args support. Clean implementation.
dimos/core/module_coordinator.py	Docker vs worker module routing added to deploy/deploy_parallel. deploy_parallel has a cross-group cleanup gap where successful modules from one group aren't stopped when the other group fails.
dimos/core/rpc_client.py	Added ModuleProxyProtocol, per-method timeout resolution, and explicit timeout parameter on RpcCall. The default timeout=0 for RpcCall is a latent bug that will cause instant TimeoutError for any future caller that omits the argument.
dimos/utils/safe_thread_map.py	New utility for parallel thread execution with structured error handling and cleanup callbacks. Includes ExceptionGroup polyfill for Python 3.10. Well-designed with good documentation.
dimos/core/worker.py	Added aarch64 static TLS preloading workaround for Jetson devices and improved deploy_module error handling with finally block for slot reservation. The TLS fix is well-documented and targeted.
dimos/core/worker_manager.py	Replaced ThreadPoolExecutor with safe_thread_map for deploy_parallel, adding proper cleanup of successful RPCClients when siblings fail. Good improvement to error handling.
dimos/navigation/rosnav/rosnav_module.py	New 942-line Docker-based ROS navigation module for Unitree robots. Comprehensive ROS topic remapping, teleop override, and stream bridging between DimOS and ROS2 container.
dimos/core/docker_worker_manager.py	New static helper for parallel Docker module deployment with proper cleanup of successful containers when siblings fail.
dimos/protocol/rpc/spec.py	Removed hardcoded 1200s start timeout from call_sync; timeout resolution is now handled by the caller (RpcCall/RPCClient). Clean separation of concerns.
dimos/robot/unitree/g1/effectors/high_level/dds_sdk.py	New G1 high-level control module using native Unitree SDK2 over DDS. Implements HighLevelG1Spec with velocity control, arm gestures, and FSM state management.
dimos/web/websocket_vis/websocket_vis_module.py	Improved error logging for stream subscriptions and moved browser open to a background thread. Contains leftover [DEBUG] log lines that should be cleaned up.
dimos/visualization/vis_module.py	New shared visualization module factory that creates viewer-specific blueprints based on global config (foxglove, rerun, etc.).
dimos/core/module.py	Added rpc_timeouts dict for per-method timeout overrides. Removed configure_stream RPC (replaced by set_transport forwarding in DockerModule). Clean changes.
dimos/agents/agent.py	Added rpc_timeouts override for on_system_modules (180s) and explicit timeout parameter on RpcCall construction.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    MC[ModuleCoordinator.deploy_parallel] --> Split{Is Docker module?}
    Split -->|Yes| DI[DockerWorkerManager.deploy_parallel]
    Split -->|No| WI[WorkerManager.deploy_parallel]
    
    DI --> DM1[DockerModule.__init__]
    DI --> DM2[DockerModule.__init__]
    
    DM1 --> Hash{Build hash changed?}
    Hash -->|Yes| Build[build_image]
    Hash -->|No| Pull{Image exists?}
    Pull -->|No| DockerPull[docker pull]
    Pull -->|Yes| Run
    Build --> Run[docker run -d]
    DockerPull --> Run
    Run --> WaitRPC[_wait_for_rpc]
    WaitRPC --> Ready[Container Ready]
    
    WI --> W[Worker.deploy_module]
    W --> RPC[RPCClient]
    
    Ready --> STM[start_all_modules via safe_thread_map]
    RPC --> STM
    STM --> StartRPC[module.start via RPC]
    STM --> OnSys[on_system_modules]
    
    subgraph Error Handling
        STM -->|Failure| EG[ExceptionGroup]
        DI -->|Partial Failure| Cleanup[Stop successful modules]
        WI -->|Partial Failure| CleanupW[Stop successful RPCClients]
    end

Loading

_{Last reviewed commit: a98cfe3}

[greptile-apps]

Dangerous default timeout of 0

timeout: float = 0 will cause event.wait(0) in spec.py:58 to return immediately (non-blocking), resulting in an instant TimeoutError for any RpcCall that doesn't pass an explicit timeout. While all current call sites do pass an explicit value, this is a dangerous default — any future caller that omits timeout will get a confusing immediate timeout. Consider defaulting to RPCClient.default_rpc_timeout (120.0) to make the safe path the easy path.

Suggested change

	timeout: float = 0,
	timeout: float = 120.0,

[greptile-apps]

Debug logging left in production code

These [DEBUG] log lines appear to be leftover debugging instrumentation. They should be removed or demoted to logger.debug() before merge.

Suggested change

	transport = getattr(self.global_costmap, "_transport", "MISSING")
	logger.info(f"[DEBUG] global_costmap transport before subscribe: {transport}")
	try:
	unsub = self.global_costmap.subscribe(self._on_global_costmap)
	self._disposables.add(Disposable(unsub))
	except Exception:
	...
	logger.info(f"[DEBUG] Subscribed to global_costmap OK, transport={transport}")
	try:
	unsub = self.global_costmap.subscribe(self._on_global_costmap)
	self._disposables.add(Disposable(unsub))
	logger.info("Subscribed to global_costmap")

[greptile-apps]

Partial failure doesn't clean up cross-group deployments

If _deploy_docker fails but _deploy_workers already succeeded, the successfully-deployed worker modules in results are registered via _register() but never cleaned up. Similarly, if _deploy_workers fails but Docker modules already succeeded, those Docker modules are orphaned.

The inner safe_thread_map calls within each deployer have their own cleanup for intra-group failures, but there's no cross-group teardown. When one of the two group functions raises, _on_errors calls _register() and re-raises — leaving the modules from the other (successful) group running without any shutdown path.

Consider stopping all successfully-deployed modules in _on_errors before re-raising:

def _on_errors(
    _outcomes: list[Any], _successes: list[Any], errors: list[Exception]
) -> None:
    # Stop any modules that deployed successfully before the failure
    for module in results:
        if module is not None:
            with suppress(Exception):
                module.stop()
    raise ExceptionGroup("deploy_parallel failed", errors)

[jeff-hykin]

adaptation of existing code, renamed because we now have both Unity and Mujoco Sim