Skip to content

Nav System (Ros and Non-ros) - #1717

Closed
jeff-hykin wants to merge 553 commits into
devfrom
jeff/fix/rosnav5
Closed

jeff-hykin wants to merge 553 commits into
devfrom
jeff/fix/rosnav5

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Mar 31, 2026 •

Copy link
Copy Markdown
Member

Todo:

  • delete the docker ros system (as per our friday discussion)

Pre-requisite PRs

Draft: only becase there was a LOT of merges recently so stuff needs to be re-tested

Problem

Robot blueprints each duplicated viewer backend selection logic (foxglove vs rerun vs rerun-web vs rerun-connect), making it error-prone to add new backends or change behavior. The RerunBridge also lacked a WebSocket server for dimos-viewer keyboard/click events in --connect mode.

Solution

  • Added vis_module() factory that centralizes viewer backend selection into a single function returning a Blueprint
  • Added RerunWebSocketServer module that receives dimos-viewer events (click, twist, stop, heartbeat) over WebSocket and publishes them as DimOS streams
  • Improved RerunBridge: configurable memory_limit, multi-IP connect hints via psutil, headless error handling
  • Migrated all robot blueprints (GO2 basic, G1 primitive, G1 SHM, camera demo) to use vis_module()
  • Added is_jetson() and get_local_ips() utilities

Breaking Changes

  • ViewerBackend enum removed from bridge.py (already moved to global_config)
  • _get_local_ip() removed from bridge (replaced by get_local_ips() in utils)
  • WebsocketVisModule no longer used in G1 primitive blueprint (replaced by RerunWebSocketServer via vis_module)

How to Test

# Unit tests (14 tests)
python -m pytest dimos/visualization/rerun/test_websocket_server.py --noconftest -q

# E2E tests
python -m pytest dimos/visualization/rerun/test_viewer_ws_e2e.py --noconftest -q

# Lint
ruff check dimos/visualization/vis_module.py dimos/visualization/rerun/websocket_server.py dimos/visualization/rerun/bridge.py

Contributor License Agreement

  • I have read and approved the CLA.

jeff-hykin and others added 30 commits March 21, 2026 15:07
Keep both new entries in alphabetical order:
- unitree-go2-webrtc-keyboard-teleop (from dev)
- unity-sim (from this branch)
…rename

After merging dev, the rerun_bridge function was renamed to run_bridge and
changed to a standalone runner. Blueprint should use
RerunBridgeModule.blueprint() like all other blueprints.
…n up code style

- UnityBridgeConfig now inherits properly from ModuleConfig (Pydantic BaseModel)
  after dev changed ModuleConfig from dataclass to Pydantic
- Use pydantic.Field instead of dataclasses.field for default_factory
- Remove empty __init__.py (not allowed per test_no_init_files)
- Remove section marker comments (not allowed per test_no_sections)
- Merge latest dev (LFS assets for piper, xarm6)

This broke after merging dev because ModuleConfig changed from @DataClass to
Pydantic BaseModel. The @DataClass decorator on UnityBridgeConfig was generating
an __init__ that required all fields as positional args, ignoring the Pydantic
field defaults.
test_sim_module.py was replaced by test_sim_adapter.py in PR #1639
(MuJoCo sim support for Manipulation). Accept dev's deletion.
…snav3

# Conflicts:
#	dimos/simulation/manipulators/test_sim_module.py
- native_module: buffer last 50 stderr lines in reader thread so crash
  report actually contains stderr output (was always empty before because
  the reader thread consumed and closed the stream)
- unity module: wrap server socket in try/finally to prevent FD leak if
  bind()/listen() raises
- unity module: drain stale send-queue messages at start of each new
  Unity connection to prevent delivering old-session data
- unity module: read self._x/self._y under _state_lock in _on_terrain
  callback to ensure atomic read of the position pair
- Move _server_ready.set() inside ws_server.serve() context so stop()
  waits for the port to actually bind before sending shutdown signal
- Add /ws path filter to reject non-viewer WebSocket connections
- Add pytest.mark.skipif for dimos-viewer binary test in CI
- Fix import ordering in manipulation/blueprints.py
The default websockets ping_interval=20s + ping_timeout=20s was too
aggressive. Increase both to 30s to give the viewer more time to
respond, especially during brief network hiccups.
Must-fix:
- _running changed from bare bool to threading.Event for proper
  cross-thread visibility (consistent with _unity_ready and halt)
- _sim_loop now holds _state_lock during position reads/writes so
  _on_terrain sees atomic position pairs
- _unity_sender logs exceptions before halting instead of swallowing
- Camera intrinsics unified: _publish_camera_info and rerun_static_pinhole
  now share the same constants (_CAM_FX/FY/CX/CY from 120° HFOV)

Should-fix:
- import signal and import cv2 moved to top-level (no inline imports)
- stop() catches subprocess.TimeoutExpired specifically, logs SIGKILL
  escalation
- Queue drain loop simplified: while True + Empty break (no .empty())
- server_sock wrapped in try/finally from creation (not just the loop)
- unity_host default changed from 0.0.0.0 to 127.0.0.1 (loopback)
- Bridge connection uses 30s read timeout to detect hung Unity sessions

Nits:
- Tests use try/finally for thread cleanup
- Pickle test checks _running.is_set() not bare bool
- Added edge case tests: empty/truncated/garbage pointcloud,
  truncated compressed image, pose_stamped round-trip
- ROS1 deserializers log at DEBUG level on parse failure instead
  of silently returning None
- Add timeout parameter to RpcCall.__init__ and pass through to call_sync
- Replace RPCClient.default_rpc_timeout with DEFAULT_RPC_TIMEOUT constant
- Update all imports of removed blueprint aliases to use ClassName.blueprint
- Fix zero quaternion (0,0,0,0) -> identity (0,0,0,1) in legacy rosnav
- Fix stop_navigation() not unblocking navigate_to() in legacy rosnav
- Remove rosnav.py that shadowed rosnav/ package, add __init__.py
- Regenerate all_blueprints.py
jeff-hykin and others added 29 commits April 2, 2026 23:36
Accept dev's deletions for files moved to dimos/core/coordination/.
Take dev's versions for infrastructure files. PR-specific nav changes
preserved in smart_nav module files.
NativeModule doesn't support __class_getitem__, so
NativeModule[_RebuildConfig] fails at class definition time on
Python 3.12. The default_config class attribute already provides the
config type.
Module and StreamModule don't support __class_getitem__, so
Module[Config] fails at class definition time on Python 3.12.
The default_config class attribute already provides the config type.
- Fix dimos.core.blueprints -> dimos.core.coordination.blueprints imports
- Add config type annotations to NativeModule subclasses
- Add start/stop methods to Module/NativeModule subclasses
- Fix CLI arg test assertions to match cli_name_override mappings
- Update PGO tests for Module (no longer NativeModule)
- Fix TerrainAnalysis test field names
- Add __getstate__/__setstate__ to NativeModule and UnityBridgeModule
- Fix G1 config for new RobotConfig schema
- Add thread leak exclusions in conftest for framework threads
- Remove section markers from config files
- Skip native binary tests when binaries not built
- Handle LCM socket errors in transport tests
- Mark viewer binary test as xfail in headless mode
- Exclude .ignore.enhance from test_no_sections
- Wrap LCM transport test in OSError handler for socket unavailability
- Skip path data test when LFS data not available
- Filter daemon threads from thread leak detection
- Fix duplicate code in test_paths_and_blueprint
Keep tele_cmd_vel topic name from rosnav5 branch while incorporating
speed constant documentation from dev.
@jeff-hykin jeff-hykin closed this Apr 23, 2026
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.

3 participants