Jeff/fix/rosnav8 by jeff-hykin · Pull Request #1791 · dimensionalOS/dimos

jeff-hykin · 2026-04-16T05:16:44Z

Pre-requisite PRs

unity-sim — #1681 (merged)
docker — #1431 (merged)
dimos-viewer (separate repo PR) websockets and --connect
native_rebuild3 — #1599
rconnect2 — #1643

Will be perpetually re-testing hardware after merges/changes to ^

631af1e tested for 2 hours on the g1 (entire battery! instantly responsive after 2hrs)
c54578a044f892fe7f231b468f2f3512adf4282a (later) hardware tested and sim 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

port over the whole nav stack

Breaking Changes

None

How to Test

git checkout jeff/fix/rosnav8

# last hardware tested commit
git checkout c54578a044f892fe7f231b468f2f3512adf4282a

# simulation
uv run dimos run unitree-g1-nav-sim

# real
uv run dimos run unitree-g1-nav-onboard

Contributor License Agreement

I have read and approved the CLA.

…nto jeff/fix/rosnav8

…dimos into jeff/fix/rosnav8

…ative_rebuild2

Twelve fixes from Paul-style code review of #1780: native_module.py: - _maybe_build: use did_change(update=False) + post-success update_cache so failed builds never poison the rebuild cache - delete stray "Source files changed" log line that fired on every call - stop(): per-instance _stop_lock; capture proc/watchdog refs under the lock, do signal/wait/join outside the lock to avoid the watchdog-self- join deadlock; second caller no-ops via _stopping - _read_log_stream: receive pid as a parameter instead of reaching into self._process.pid (TOCTOU with stop()) - _child_preexec_linux + ctypes hoisted to module scope under a sys.platform guard; no more re-import per start() - _clear_nix_executable: gracefully handle cwd=None (skip the walk entirely), use .resolve() comparison so a symlinked cwd still terminates the walk, refuse to walk past cwd's tree change_detect.py: - fold max_file_size into _hash_files digest so different thresholds against the same cache_name can't corrupt each other - new _locked_cache_file context manager — flock the .hash file directly in "a+" mode; no more orphan .lock sidecars accumulating in the cache dir; did_change/update_cache/clear_cache all share the helper Tests: - new test_should_rebuild_true_bypasses_change_check for the explicit "always rebuild" path - new test_failed_build_does_not_mark_cache_clean for the update=False retry semantics Build system: - tiledb darwin overlay: filter patches by name instead of dropping all of them, so a future security patch from nixpkgs survives - livox_sdk_config.hpp: honor $TMPDIR on Darwin instead of hardcoding /tmp Perf script: - compute cache_name inline (using the same inspect-based source_file pattern) instead of calling the inlined _build_cache_name method All 26 tests across test_change_detect.py + test_native_module.py + test_native_rebuild.py pass. ruff + mypy clean. mid360_native and fastlio2_native still build end-to-end on aarch64-darwin. Linux drvPaths for libpqxx/tiledb/pcl/vtk verified unchanged by the overlay.

paul-nechifor · 2026-04-18T04:44:19Z

+        # (roof is at ~3 m+).
+        MAX_ALLOWED_Z = 2.0
+
+        lcm_url = os.environ.get("LCM_DEFAULT_URL", "udpm://239.255.76.67:7667?ttl=0")


Use _DEFAULT_LCM_URL from lcmservice.py. No need to duplicate it here.

paul-nechifor · 2026-04-18T04:44:57Z

+                if robot_z > max_z:
+                    max_z = robot_z
+
+        lc.subscribe(ODOM_TOPIC, _odom_handler)


Need to unsubscribe.

paul-nechifor · 2026-04-18T04:46:51Z

+        MAX_ALLOWED_Z = 2.0
+
+        lcm_url = os.environ.get("LCM_DEFAULT_URL", "udpm://239.255.76.67:7667?ttl=0")
+        lc = lcmlib.LCM(lcm_url)


It's a 20000 line PR. You're not saving disk space by using lc instead of lcm. :)

paul-nechifor · 2026-04-18T04:48:52Z

+            )
+
+        finally:
+            print("\n[test-simple] Stopping blueprint…")


Please add in conventions to not add print() in tests.

paul-nechifor · 2026-04-18T04:51:22Z

+            print(f"[test-simple] Odom online. Robot at ({robot_x:.2f}, {robot_y:.2f})")
+
+            print(f"[test-simple] Warming up for {WARMUP_SEC}s…")
+            time.sleep(WARMUP_SEC)


Sometimes sleep is unavoidable, but is there no way to pool for a condition than to wait 15 seconds every time?

actually yeah, if your module start() has returned your module is assumed ready

paul-nechifor · 2026-04-18T05:04:37Z

+Test sequence:
+  p0  (-0.3,  2.5) — open corridor speed test
+  p1  (11.2, -1.8) — navigate with furniture
+  p2  ( 3.3, -4.9) — intermediate waypoint near doorway (explore lower area)
+  p3  ( 7.0, -5.0) — through the doorway into the right room
+  p4  (11.3, -5.6) — explore right room
+  p4→p1 (11.2, -1.8) — CRITICAL: must route through doorway, NOT wall


Suggested change

Test sequence:

p0 (-0.3, 2.5) — open corridor speed test

p1 (11.2, -1.8) — navigate with furniture

p2 ( 3.3, -4.9) — intermediate waypoint near doorway (explore lower area)

p3 ( 7.0, -5.0) — through the doorway into the right room

p4 (11.3, -5.6) — explore right room

p4→p1 (11.2, -1.8) — CRITICAL: must route through doorway, NOT wall

paul-nechifor · 2026-04-18T05:05:57Z

+import lcm as lcmlib
+import pytest
+
+os.environ.setdefault("DISPLAY", ":1")


Please do not change the system after the test. Why is this even needed?

paul-nechifor · 2026-04-18T05:07:35Z

+        from dimos.visualization.vis_module import vis_module
+
+        # -- Clear stale nav paths from previous runs -------------------------
+        paths_dir = Path(__file__).resolve().parents[3] / "data" / "smart_nav_paths"


Is this get_data("smart_nav_paths")? Why are you removing it? Everything in data should be static.

paul-nechifor · 2026-04-18T05:08:57Z

+            for f in paths_dir.iterdir():
+                f.unlink(missing_ok=True)
+
+        # -- Build blueprint (same composition as unitree_g1_nav_sim) ----------


This does look pretty identical. So why not use it instead of copying it here?

paul-nechifor · 2026-04-18T05:14:44Z

+pytestmark = [pytest.mark.slow]
+
+
+class TestCrossWallPlanning:


This code is pretty much identical to dimos/navigation/smart_nav/tests/test_cross_wall_planning_simple.py . Please extract to common parts (like the waypoint following) into fixtures.

I already have something similar as follow_points in dimos/e2e_tests/conftest.py. Can that not be used here?

Also, it might be better to move these tests to dimos/e2e_tests.

paul-nechifor · 2026-04-18T05:18:10Z

+    ("result-tare-planner", "tare_planner"),
+]
+_HAS_BINARIES = all((_NATIVE_DIR / d / "bin" / name).exists() for d, name in _REQUIRED_BINARIES)
+_IS_LINUX_X86 = platform.system() == "Linux" and platform.machine() in ("x86_64", "AMD64")


You use _IS_LINUX_X86 in several places. Please move it to dimos/conftest.py as skipif_macos. But I don't understand why we can't build on Mac OS.

paul-nechifor · 2026-04-18T05:27:07Z

+    registered_scan: Out[PointCloud2]
+    odometry: Out[Odometry]
+
+    def __init__(self, **kwargs):  # type: ignore[no-untyped-def]


Suggested change

def __init__(self, **kwargs): # type: ignore[no-untyped-def]

def __init__(self, **kwargs):

no typechecking in tests.

paul-nechifor · 2026-04-18T05:27:57Z

+    def __getstate__(self) -> dict[str, Any]:
+        state = super().__getstate__()
+        state.pop("_cmd_lock", None)
+        state.pop("_sensor_thread", None)
+        state.pop("_sim_thread", None)
+        return state
+
+    def __setstate__(self, state: dict[str, Any]) -> None:
+        super().__setstate__(state)
+        self._cmd_lock = threading.Lock()
+        self._sensor_thread = None
+        self._sim_thread = None


Are you sure this is needed?

paul-nechifor · 2026-04-18T05:28:09Z

+    @rpc
+    def start(self) -> None:
+        super().start()
+        self.cmd_vel._transport.subscribe(self._on_cmd_vel)


please unsub

paul-nechifor · 2026-04-18T05:28:46Z

+
+            now = time.time()
+            quat = Quaternion.from_euler(Vector3(0.0, 0.0, self._yaw))
+            self.odometry._transport.publish(


Why _transport?

paul-nechifor · 2026-04-18T05:31:19Z

+    lock: threading.Lock = field(default_factory=threading.Lock)
+
+
+# Test


Suggested change

# Test

paul-nechifor · 2026-04-18T05:31:31Z

+    from dimos.core.coordination.blueprints import autoconnect
+    from dimos.core.coordination.module_coordinator import ModuleCoordinator
+    from dimos.msgs.geometry_msgs.PointStamped import PointStamped
+    from dimos.msgs.nav_msgs.Path import Path as NavPath
+    from dimos.navigation.smart_nav.modules.local_planner.local_planner import LocalPlanner
+    from dimos.navigation.smart_nav.modules.path_follower.path_follower import PathFollower
+    from dimos.navigation.smart_nav.modules.tare_planner.tare_planner import TarePlanner
+    from dimos.navigation.smart_nav.modules.terrain_analysis.terrain_analysis import TerrainAnalysis


Move to top.

paul-nechifor · 2026-04-18T05:37:19Z

+    tare = coordinator.get_instance(TarePlanner)
+    planner = coordinator.get_instance(LocalPlanner)
+    follower = coordinator.get_instance(PathFollower)
+    coordinator.get_instance(MockVehicle)
+    terrain = coordinator.get_instance(TerrainAnalysis)
+
+    def _on_wp(msg: PointStamped) -> None:
+        with collector.lock:
+            collector.waypoints.append((msg.x, msg.y, msg.z))
+
+    def _on_terrain(msg: PointCloud2) -> None:
+        with collector.lock:
+            collector.terrain_maps.append(True)
+
+    def _on_path(msg: NavPath) -> None:
+        with collector.lock:
+            collector.paths.append(msg)
+
+    def _on_cmd(msg: Twist) -> None:
+        with collector.lock:
+            collector.cmd_vels.append((msg.linear.x, msg.linear.y, msg.angular.z))
+
+    tare.way_point._transport.subscribe(_on_wp)
+    planner.path._transport.subscribe(_on_path)
+    follower.cmd_vel._transport.subscribe(_on_cmd)
+    terrain.terrain_map._transport.subscribe(_on_terrain)


This is the same pattern you're using in dimos/navigation/smart_nav/tests/test_waypoint_nav.py except you're using lock.acquire() instead of with lock.

You can have a fixture which starts the blueprint and records data. The test should use the fixture and assert that the data is what you expect.

leshy · 2026-04-21T03:47:21Z

+
+
+class PathFollower(NativeModule):
+    """Pure pursuit path follower with PID yaw control.


we shouldn't use pure pursuit for holonomic robots, just for an issue for later

leshy · 2026-04-21T03:48:37Z

+
+    def _query_pose(self) -> tuple[float, float, float]:
+        """Return (x, y, z) from the TF tree, falling back to cached values."""
+        for parent, child in self._TF_POSE_QUERIES:


why aren't you simply using self.tf.get ? what is this for? I see cache mentioned but tf service does offer built in caching

leshy · 2026-04-21T03:50:30Z

+        return len(self._key_poses)
+
+
+class PGO(Module):


why do we have DIY python only PGO? aren't we using some fastlio pgo stuff?

leshy · 2026-04-21T03:56:19Z

+    return [s, int((ts - s) * 1_000_000_000)]
+
+
+class GraphNodes3D(Timestamped):


dimos.msgs structure 100% matches ros msgs, you shouldn't add new custom classes to this nav_msgs dir but create a dimos.msgs.something subdir for your custom messages.

they can later be upgraded to be a part of our actual dimos-lcm repo for multi lang support if we choose to, but shouldn't pollute ROS namespace

paul-nechifor · 2026-04-21T04:41:57Z

+        "rerun",
+        rerun_config={
+            "visual_override": {
+                "world/lidar": lambda grid: grid.to_rerun(voxel_size=voxel_size, mode="boxes"),


You cannot use lambdas in blueprints because they can't be pickled to be sent to modules. Are these values not sent to modules?

You have to use functions with names because they can be serialized. (This worked in dask because dask serializes functions by bytecode.)

paul-nechifor · 2026-04-21T04:42:40Z

+        pass
+    # Also grab addresses via DGRAM trick for interfaces without DNS
+    try:
+        import subprocess


Move to top.

paul-nechifor · 2026-04-21T04:44:53Z

-        super().stop()
+        # Subscribe to our own odometry output so we can mirror each
+        # pose update into the TF tree as an odom→body transform.
+        self.odometry.transport.subscribe(self._on_odom_for_tf, self.odometry)


Unsubscribe.

paul-nechifor · 2026-04-21T05:01:15Z

+
+
+class TestFarPlannerConfig:
+    """Test FarPlanner configuration."""


More tests for values. Please remove.

I don't see any code in dimos/navigation/smart_nav/modules/far_planner/far_planner.py . There's nothing to test.

paul-nechifor · 2026-04-21T05:13:42Z

@@ -0,0 +1,102 @@
+# Copyright 2026 Dimensional Inc.


Please remove this file too.

paul-nechifor · 2026-04-21T05:24:35Z

+                    logger.debug(
+                        "Global map published",
+                        points=len(cloud_np),
+                        keyframes=pgo.num_key_poses,
+                    )


Suggested change

logger.debug(

"Global map published",

points=len(cloud_np),

keyframes=pgo.num_key_poses,

)

paul-nechifor · 2026-04-21T05:25:28Z

+        self.odometry.subscribe(self._on_odom)
+        self.registered_scan.subscribe(self._on_scan)


paul-nechifor · 2026-04-21T05:30:46Z

 import uuid

+
+def get_local_ips() -> list[tuple[str, str]]:


There's another _get_local_ips in dimos/hardware/sensors/lidar/fastlio2/module.py. Should this replace that? Also move the imports to the top.

jeff-hykin added 30 commits April 11, 2026 19:36

native rebuild changes

817ddfd

fix: ruff format and update uv.lock

b07cefb

minor fixes

bf80a6b

delete docker nav

a5ff2ca

all vis changes

9423ae4

Merge branch 'jeff/fix/rconnect2' of github.com:dimensionalOS/dimos i…

cf099d8

…nto jeff/fix/rosnav8

-

3e0cdef

fixup

6d6151c

add a conventions.md to dev docs

ac62445

fixup cmd_vel timer edgecase leaks

36492b1

switch cmd_vel_scaling to be on rerun

306bd2b

add convention

f7277d9

fixup

75bf186

Merge branch 'jeff/fix/rconnect2' of github.com:dimensionalOS/dimos i…

9237a97

…nto jeff/fix/rosnav8

Merge branch 'jeff/feat/native_rebuild2' of github.com:dimensionalOS/…

78aba2d

…dimos into jeff/fix/rosnav8

fixup signal handling (looks worse but in practice behaves better)

b8b9981

cleanup

2718e73

make streaming

7641683

cleaner cache building logic (update file cache on check time)

46df237

make dependencies work on MacOS

c1743c9

add nix performance check

efc5aa9

get build working on macos

8ed96a6

add rebuild_on_change

3e38209

add perf test

bb69f20

limit the slow rebuild to macos

2a56733

missed edgecase

29875f8

simplify logging

446871a

add max_file_size

0dc5152

Merge branch 'dev' of github.com:dimensionalOS/dimos into jeff/feat/n…

417796a

…ative_rebuild2

paul-nechifor reviewed Apr 18, 2026

View reviewed changes

leshy reviewed Apr 21, 2026

View reviewed changes

paul-nechifor reviewed Apr 21, 2026

View reviewed changes

	def __init__(self, **kwargs): # type: ignore[no-untyped-def]
	def __init__(self, **kwargs):

		lock: threading.Lock = field(default_factory=threading.Lock)


		# Test



		class PathFollower(NativeModule):
		"""Pure pursuit path follower with PID yaw control.

		return [s, int((ts - s) * 1_000_000_000)]


		class GraphNodes3D(Timestamped):



		class TestFarPlannerConfig:
		"""Test FarPlanner configuration."""

		self.odometry.subscribe(self._on_odom)
		self.registered_scan.subscribe(self._on_scan)

Conversation

jeff-hykin commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-requisite PRs

Problem

Solution

Breaking Changes

How to Test

Contributor License Agreement

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!

paul-nechifor Apr 18, 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!

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!

leshy Apr 21, 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!

leshy Apr 21, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-nechifor Apr 21, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-hykin commented Apr 16, 2026 •

edited

Loading

paul-nechifor Apr 18, 2026 •

edited

Loading

leshy Apr 21, 2026 •

edited

Loading

leshy Apr 21, 2026 •

edited

Loading

paul-nechifor Apr 21, 2026 •

edited

Loading