Skip to content

dim_slam: cuVSLAM+fusion - #3632

Open
jeff-hykin wants to merge 23 commits into
jeff/feat/cuvslamfrom
jeff/feat/dim_slam
Open

dim_slam: cuVSLAM+fusion #3632
jeff-hykin wants to merge 23 commits into
jeff/feat/cuvslamfrom
jeff/feat/dim_slam

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Aug 24, 2026

Copy link
Copy Markdown
Member

Odom fusion with cuVSLAM

next PR lets us run it on alfred

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds the Rust-backed dimSLAM module, a RealSense stereo demo, odometry-path visualization, and supporting registration. The infrared-only demo can fail to initialize visual odometry on a RealSense profile without a discoverable depth stream because camera-frame transforms are withheld even though the infrared streams are enabled.

Confidence Score: 4/5

The infrared-only RealSense demo is not reliable across supported camera profiles until its transform origin can be selected from an enabled stream.

The camera transform builder unconditionally selects a depth profile as its origin. When that profile is unavailable, it returns without publishing the infrared transforms required by dimSLAM.

Files Needing Attention: dimos/hardware/sensors/camera/realsense/camera.py

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and linked to the review comment.
  • T-Rex produced a proof for another posted P1 finding and linked to the review comment.
  • T-Rex ran the infrared-only RealSense TF validation script and confirmed the expected camera links and frames when depth is exposed; no product code changes were made, and only the validation script and its output were created.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. dimos/hardware/sensors/camera/realsense/camera.py, line 496-499 (link)

    P1 Infrared-only camera transforms require depth

    _build_mount_edges() always selects the depth profile as its transform origin. The dimSLAM demo deliberately disables depth while enabling infrared; on a device/profile without a discoverable depth profile this returns before publishing either infrared mount or optical-frame transform. dimSLAM resolves its stereo-camera extrinsics through TF, so visual odometry cannot initialize. Select an enabled infrared profile as the origin when depth is unavailable.

    Artifacts

    Infrared-only RealSense TF validation script source

    • Authored Python validation parses the current sources and executes the exact RealSense profile and mount-edge methods against mocked device profiles, showing the tested configuration and assertions.

    Infrared-only RealSense TF validation command output

    • Captured output of the validation command from `/home/user/repo` with exit code 0, showing both infrared mount and optical TF edges exist when a depth profile is available.

    View artifacts

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 Infrared-only RealSense TF setup still requires a depth device profile

    • Bug
      • _build_mount_edges() always resolves rs.stream.depth as its transform origin. If an infrared-only configuration runs on a device/profile without a discoverable depth stream profile, it logs that no depth stream exists and returns before publishing any infrared mount or optical TF edges.
    • Cause
      • The hard-coded depth origin lookup at camera.py:496 is unconditional, while the demo deliberately disables the depth stream and DimSlam consumes the remapped infrared camera frames using TF extrinsics against its rig frame.
    • Fix
      • Choose an enabled infrared stream as the origin when depth is disabled or unavailable (and retain depth as the preferred origin when present), then construct the infrared mount and optical edges from that origin.

    T-Rex Ran code and verified through T-Rex

Reviews (20): Last reviewed commit: "dim_slam: as_chunks over chunks_exact fo..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/mapping/dim_slam/dim_slam.py 90.90% 4 Missing ⚠️
dimos/mapping/odometry_path.py 50.00% 1 Missing ⚠️
@@                  Coverage Diff                  @@
##           jeff/feat/cuvslam    #3632      +/-   ##
=====================================================
+ Coverage              77.55%   77.78%   +0.22%     
=====================================================
  Files                   1293     1293              
  Lines                 123802   123829      +27     
  Branches               10850    10851       +1     
=====================================================
+ Hits                   96017    96322     +305     
+ Misses                 24662    24366     -296     
- Partials                3123     3141      +18     
Flag Coverage Δ
OS-ubuntu-24.04-arm 72.77% <89.58%> (+<0.01%) ⬆️
OS-ubuntu-latest 74.61% <89.58%> (-0.01%) ⬇️
Py-3.10 74.61% <89.58%> (+<0.01%) ⬆️
Py-3.11 74.61% <89.58%> (+<0.01%) ⬆️
Py-3.12 74.61% <89.58%> (+<0.01%) ⬆️
Py-3.13 74.60% <89.58%> (-0.01%) ⬇️
Py-3.14 74.61% <89.58%> (-0.01%) ⬇️
Py-3.14t 74.60% <89.58%> (+<0.01%) ⬆️
SelfHosted-Large 30.04% <50.00%> (?)
SelfHosted-Linux 35.25% <50.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
dimos/mapping/dim_slam/demo_cuvslam_realsense.py 76.92% <100.00%> (ø)
dimos/msgs/nav_msgs/Path.py 94.50% <ø> (ø)
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
dimos/mapping/odometry_path.py 97.72% <50.00%> (-2.28%) ⬇️
dimos/mapping/dim_slam/dim_slam.py 61.80% <90.90%> (ø)

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

@jeff-hykin
jeff-hykin marked this pull request as ready for review August 24, 2026 07:50
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Aug 24, 2026
@jeff-hykin jeff-hykin changed the title dim_slam: one module wrapping the merged cuVSLAM+fusion binary dim_slam: cuVSLAM+fusion Aug 24, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 24, 2026
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Aug 24, 2026
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
Comment thread dimos/mapping/dim_slam/demo_cuvslam_realsense.py
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
The Rust filter shipped in the same derivation as cuvslam_odometry but had no
Python side, so nothing in dimos could run it. Every source publishes onto one
sources port and is told apart by header.frame_id, which makes adding a source
a config change rather than a port change.

imu_init_samples carries the weight here: on a 517 s Alfred drive, leaving the
raw gyro bias in costs 19.8 m of final error against 1.6 m with it removed,
which is worse than fusing no IMU at all.
The Rust filter gained a use_imu switch so it can be seeded level from the first
source message and coast between measurements rather than propagating on IMU.
`#[native_config]` supplies no defaults and rejects unknown keys, so the Python
config has to carry the field for the module to start at all.

Off is worth having on Alfred: measured against point-lio over the 517 s drive,
wheel and cuVSLAM fused without the IMU lands at 3.53 m, while the same pair with
the IMU added lands at 71.32 m.
The native module publishes a range-gated depth_cloud and takes the gate from two
config fields. `#[native_config]` supplies no defaults and rejects unknown keys, and
the runtime rejects a module whose declared ports do not match the binary's, so
without both the field pair and the port the process died on startup every run:

    failed to deserialize config: missing field `depth_cloud_min_range`
    topics do not match module ports: missing ["depth_cloud"]

That killed CuvslamOdometry silently in alfred-mls-nav — the stack came up, the
remaining modules ran, and the fusion filter simply never saw a visual source. With
both in place cuvslam publishes odometry at 30 Hz on Alfred.
dimSLAM merged its two binaries into one dim_slam executable that feeds
the tracker's pose into the fusion filter in-process, so the two wrapper
modules collapse into one DimSlam module with the flat union of both
configs. The internal visual odometry stream no longer crosses the wire;
external sources still arrive on the sources input, told apart by
header.frame_id.
k x k block median decimation of the published depth cloud; <= 1 off.
… instead of a pinned rev

(cherry picked from commit 8cc5734)
…efaults

The noise figures are a property of the part, so the module has no sane
default; use_imu now requires all four above zero.
The filter needed four datasheet noise figures at construction or it raised,
so every blueprint without an IMU had to opt out by hand.
The depth scale note claimed rgbd only, which the cloud path does not gate on.
Pairs with dimSLAM's max_position_m, which holds the last pose instead of
publishing a diverged one. Also trims four comments back to the part the
code does not already say.
The dimSLAM repo now exports only the library (cuVSLAM tracking + fusion
filter); the LCM module binary moves here. The crate converts LCM messages
to and from the library's plain types and builds through a flake whose
sdk-<variant> packages come from cu_vslam_rs, so the python build_command
selects the variant the host actually has.
CI clippy-checks every Cargo.lock directory on runners with no SDK;
cu_vslam_rs 0.1.2 compiles as a non-linking stub when CUVSLAM_SDK_DIR
is absent, the re-pinned dim_slam library skips its rpath then, and so
does this build script. Nix builds still set the SDK and are unchanged.
clippy 1.98 (CI stable) denies chunks_exact with a constant size, and
as_chunks yields [f64; 6] directly so the try_into fallibility disappears.
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
use lcm_msgs::std_msgs::Header;

#[derive(Debug, Clone, Default, PartialEq)]
pub struct ImuInfo {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should eventually put this in some kind of shared message thing. For now I'm trying to not modify stuff outside of dim_slam

speed_gate_max_angular: f64,
/// cuVSLAM's own inertial mode, separate from the filter's use_imu.
cuvslam_enable_imu: bool,
depth_units_per_meter: f64,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Config needs to be redone to support multi-camera

}

async fn report(&mut self) {
if let Some(vo) = &self.vo {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer that only fusion reports, will change

@jeff-hykin
jeff-hykin requested a review from aclauer as a code owner August 27, 2026 00:56
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 27, 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.

1 participant