Conversation
Adds ORB-SLAM3 visual SLAM as a NativeModule under perception/slam/, following the same pattern as FastLIO2. Phase 1: nix build from source, binary initializes System and blocks — no LCM I/O yet. - Python module with OrbSlam3Config and perception.Odometry protocol - C++ wrapper using dimos::NativeModule for CLI arg parsing - Nix flake that builds ORB-SLAM3 from github:thuvasooriya/orb-slam3 - Default RealSense D435i camera settings - Vocab auto-resolved from nix store path
|
So we've got a problem (I'm having this with stuff in the nav stack too) ... ORB SLAM3 is copy left, GPLv3. We can't really include it in our codebase. I think for a lot of our native modules we're going to need a way to keep it in a separate codebase and then pull it in at runtime if the user opts to use the module. |
True, despite this not including orb-slam3 code, main.cpp links against orb-slam3. so we might want to put this as a separate GPL3 repo - edit - done |
ORB-SLAM3 is GPL-3.0, incompatible with our Apache 2.0 license. Moved C++ native module (main.cpp, CMakeLists, flake.nix) and ORB-SLAM3 configs to dimensionalOS/dimos-orb-slam3. The Python wrapper now builds from the external flake via `nix build github:dimensionalOS/dimos-orb-slam3` into a local cache directory. IPC boundary via LCM is unchanged.
Add SensorMode enum, remove model_post_init, use module directory for cwd, pin build to dimos-orb-slam3/v0.1.0. Gitignore **/result.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
if you add a comment about that weirdness (basically brokenness) you were showing me last night, then it looks good to me |
Documents the known transform mismatch where reconstructed trajectory diverges from ground-truth poses. Needs investigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
done |
…odule # Conflicts: # dimos/robot/all_blueprints.py
There was a problem hiding this comment.
Pull request overview
Adds an ORB-SLAM3-based SLAM capability to the perception stack via a thin NativeModule wrapper, plus a runnable webcam blueprint, as part of the monocular navigation experiment work.
Changes:
- Register new ORB-SLAM3 module and webcam blueprint in the global blueprint/module registries.
- Add
OrbSlam3native-module wrapper + webcam autoconnect blueprint + README. - Improve
Webcamcamera_info defaults and update repo.gitignoreto ignore Nix buildresultoutputs.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
dimos/robot/all_blueprints.py |
Registers orbslam3-webcam blueprint and orbslam3-module module entrypoints. |
dimos/perception/slam/orbslam3/module.py |
Introduces OrbSlam3 NativeModule wrapper and configuration for ORB-SLAM3 subprocess integration. |
dimos/perception/slam/orbslam3/blueprints/webcam.py |
Adds a simple autoconnect blueprint wiring webcam → ORB-SLAM3 → visualization. |
dimos/perception/slam/orbslam3/README.md |
Documents the new native module and current known issues. |
dimos/hardware/sensors/camera/webcam.py |
Switches camera_info default to pydantic.Field(default_factory=...) and fills width/height when unset. |
.gitignore |
Ignores Nix build result directories repository-wide. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from dimos.visualization.rerun.bridge import _resolve_viewer_mode, rerun_bridge | ||
|
|
||
| orbslam3_webcam = autoconnect( | ||
| rerun_bridge(viewer_mode=_resolve_viewer_mode()), |
There was a problem hiding this comment.
dimos.visualization.rerun.bridge doesn’t define rerun_bridge (only RerunBridgeModule / run_bridge exist), so this import will raise at runtime when loading the orbslam3-webcam blueprint. Switch to RerunBridgeModule.blueprint(...) (as done in other blueprints) or add/rename the intended helper in the bridge module.
| from dimos.visualization.rerun.bridge import _resolve_viewer_mode, rerun_bridge | |
| orbslam3_webcam = autoconnect( | |
| rerun_bridge(viewer_mode=_resolve_viewer_mode()), | |
| from dimos.visualization.rerun.bridge import _resolve_viewer_mode, RerunBridgeModule | |
| orbslam3_webcam = autoconnect( | |
| RerunBridgeModule.blueprint(viewer_mode=_resolve_viewer_mode()), |
| class SensorMode(enum.StrEnum): | ||
| MONOCULAR = "MONOCULAR" | ||
| STEREO = "STEREO" | ||
| RGBD = "RGBD" | ||
| IMU_MONOCULAR = "IMU_MONOCULAR" | ||
| IMU_STEREO = "IMU_STEREO" | ||
| IMU_RGBD = "IMU_RGBD" | ||
|
|
There was a problem hiding this comment.
enum.StrEnum is only available on Python 3.11+, but this repo declares/supports Python 3.10 (and CI runs mypy under py3.10). Importing this module on Python 3.10 will crash. Use a py3.10-compatible enum definition (e.g., class SensorMode(str, enum.Enum): ...) or gate StrEnum behind a version check / typing_extensions fallback.
| settings_path: str = str( | ||
| _MODULE_DIR / "result" / "share" / "orbslam3" / "config" / "RealSense_D435i.yaml" | ||
| ) |
There was a problem hiding this comment.
The default settings_path points at a RealSense D435i calibration YAML, but the provided orbslam3-webcam blueprint wires a generic Webcam via CameraModule and doesn’t override settings_path. That likely makes the runnable blueprint produce incorrect poses by default. Consider providing a webcam-appropriate default (or a minimal pinhole config), or explicitly override settings_path in the webcam blueprint / make it a required config parameter for that blueprint.
| settings_path: str = str( | |
| _MODULE_DIR / "result" / "share" / "orbslam3" / "config" / "RealSense_D435i.yaml" | |
| ) | |
| settings_path: str |
| if info.width == 0 or info.height == 0: | ||
| info.width = self.config.width |
There was a problem hiding this comment.
This mutates the shared CameraInfo instance in config and also overwrites both dimensions when only one is unset. If a caller provides (say) height but leaves width=0, this will clobber the provided height. Prefer filling width and height independently only when each is 0 (and consider avoiding in-place mutation by constructing a new CameraInfo if feasible).
| if info.width == 0 or info.height == 0: | |
| info.width = self.config.width | |
| if info.width == 0: | |
| info.width = self.config.width | |
| if info.height == 0: |
|
|
||
| ## Known Issues | ||
|
|
||
| - **Transform / trajectory reconstruction mismatch**: The reconstructed trajectory does not match ground-truth poses. There is a suspected coordinate-frame or transform-composition issue causing output to diverge from base truth. Needs investigation. |
There was a problem hiding this comment.
Spelling: “base truth” should be “ground truth” (or just “ground-truth”).
| - **Transform / trajectory reconstruction mismatch**: The reconstructed trajectory does not match ground-truth poses. There is a suspected coordinate-frame or transform-composition issue causing output to diverge from base truth. Needs investigation. | |
| - **Transform / trajectory reconstruction mismatch**: The reconstructed trajectory does not match ground-truth poses. There is a suspected coordinate-frame or transform-composition issue causing output to diverge from ground truth. Needs investigation. |
|
Right now I've got a voxel slam branch, a lt-mapper, a khronos-slam, better-fastlio2, etc. I'm kinda thinking after rosnav we add a handful of loop-closure benchmarks (lidar and visual) and dynamic map change benchmarks to DimOS, make sure stuff like ORB-SLAM3 is benchmarking how its supposed to, then let openclaw go crazy porting every major slam system to DimOS with validation |
part of monocular nav experiment #1596 but ORB-SLAM3 is a good thing to have around in general
otherwise extremely light native-module
TBH I don't understand rerun transforms (I keep forgetting and re-reading the docs) and not sure that our bridge API in this context is most convinient, so will review, this doesn't give you a nice moving camera frame, just odom and image messages, I will implement this later, it's a rerun config issue