Skip to content

Start the RealSense IMU pipeline before the video pipeline - #3670

Open
jeff-hykin wants to merge 1 commit into
mainfrom
jeff/fix/realsense_imu_start_order
Open

Start the RealSense IMU pipeline before the video pipeline#3670
jeff-hykin wants to merge 1 commit into
mainfrom
jeff/fix/realsense_imu_start_order

Conversation

@jeff-hykin

Copy link
Copy Markdown
Member

D455: the motion module fails to open as a second rs.pipeline once the video pipeline holds the device, even with enable_device(serial). Reverse order works. So RealSenseCamera.start failed whenever enable_imu=True.

Repro (swap the two pipelines and both stream):

import pyrealsense2 as rs
pipe = rs.pipeline()
cfg = rs.config()
cfg.enable_stream(rs.stream.depth, 848, 480, rs.format.z16, 30)
pipe.start(cfg)
imu = rs.pipeline()
imu_cfg = rs.config()
imu_cfg.enable_stream(rs.stream.accel)
imu_cfg.enable_stream(rs.stream.gyro)
imu.start(imu_cfg)  # RuntimeError: No device connected
  • start() now starts the IMU pipeline first
  • _stream_rates queries the device from rs.context() (filtered by serial_number when set) since the video profile doesn't exist yet

Verified on Alfred's orin-nx-7837:

dimos run alfred-mls-nav --g.rerun-open none

On a D455 the motion module fails to open as a second pipeline with "No
device connected" once the video pipeline holds the device (even with
enable_device(serial)). The reverse order works, so start the IMU
pipeline first and query stream rates from the device context instead of
the not-yet-started video profile.
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
dimos/hardware/sensors/camera/realsense/camera.py 0.00% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3670      +/-   ##
==========================================
+ Coverage   77.30%   77.56%   +0.26%     
==========================================
  Files        1287     1281       -6     
  Lines      123173   122262     -911     
  Branches    10821    10737      -84     
==========================================
- Hits        95216    94832     -384     
+ Misses      24967    24339     -628     
- Partials     2990     3091     +101     
Flag Coverage Δ
OS-ubuntu-24.04-arm 72.48% <0.00%> (+<0.01%) ⬆️
OS-ubuntu-latest 74.35% <0.00%> (-0.01%) ⬇️
Py-3.10 74.34% <0.00%> (-0.01%) ⬇️
Py-3.11 74.34% <0.00%> (+<0.01%) ⬆️
Py-3.12 74.34% <0.00%> (-0.01%) ⬇️
Py-3.13 74.34% <0.00%> (+<0.01%) ⬆️
Py-3.14 74.34% <0.00%> (-0.01%) ⬇️
Py-3.14t 74.34% <0.00%> (-0.01%) ⬇️
SelfHosted-Large 30.04% <0.00%> (+0.01%) ⬆️
SelfHosted-Linux 35.34% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
dimos/hardware/sensors/camera/realsense/camera.py 26.75% <0.00%> (+0.06%) ⬆️

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

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change starts the RealSense IMU stream before video initialization for devices that require motion streams to be opened first. Focused executions reproduced two reliability failures in dimos/hardware/sensors/camera/realsense/camera.py: an unconfigured camera can use IMU rates discovered from a different connected device, and a video-startup error leaves the already-open IMU resource held, preventing a later retry. These issues should be resolved before merging.

Confidence Score: 3/5

Not safe to merge until RealSense startup consistently selects one physical device and releases the IMU resource after a later startup failure.

Focused executable harnesses directly exercised both affected control paths: heterogeneous-device capability discovery selected unsupported rates for the pipeline-selected device, and a video startup exception retained the IMU resource and blocked retry.

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

T-Rex T-Rex Logs

What T-Rex did

  • Created a focused RealSense heterogeneous-device reproduction harness to reproduce the P1 finding.
  • Verified aggregate capability discovery with unbound device selection during setup.
  • Explored the RealSense IMU startup failure path and observed the IMU resource remains open after video startup failure.
  • Validated the general contract behavior, showing a pre-capture gyro200/accel100 on device A and post-capture requests for gyro400/accel200 being rejected as unsupported.
  • Validated cleanup control by stopping the retained IMU pipeline, which clears the resource and allows a retry.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 Unbound IMU startup validates rates across different RealSense devices

    • Bug
      • When no serial number is configured, _stream_rates unions motion capabilities from every connected device. _start_imu accepts a gyro rate and chooses the maximum accelerometer rate from that union, then starts an unbound pipeline which may select a different device. The reproduced two-device case selected device A (gyro 200, accel 100), but configured rates owned only by device B (gyro 400, accel 200), producing startup failure.
    • Cause
      • _stream_rates filters by serial only when configured, while the IMU rs.config also leaves device selection unbound when no serial is configured; therefore capability discovery and pipeline selection are not tied to one device.
    • Fix
      • Bind discovery and both pipelines to one selected serial/device before choosing stream profiles, or reject ambiguous multi-device startup when serial_number is unset. Derive IMU rates from that same selected device.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 IMU pipeline leaks when subsequent camera startup fails

    • Bug
      • start() invokes _start_imu() before starting the video pipeline, but has no rollback for exceptions raised by video startup or later initialization. The focused runtime harness shows the IMU resource stays open and a subsequent start is rejected until it is manually stopped.
    • Cause
      • The IMU pipeline is acquired at lines 200-201 outside a try/except or finally that calls stop() on startup failure.
    • Fix
      • Wrap post-IMU startup initialization in exception cleanup that stops and clears the IMU pipeline (and any video pipeline that was started) before re-raising.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix: start the RealSense IMU pipeline be..." | Re-trigger Greptile

Comment on lines +315 to +317
for device in rs.context().query_devices()
if not self.config.serial_number
or device.get_info(rs.camera_info.serial_number) == self.config.serial_number

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Stream rates mix physical devices

When serial_number is unset, _stream_rates unions IMU capabilities from every connected device while _start_imu leaves its pipeline unbound. On heterogeneous cameras, the pipeline can select a device that does not support the aggregated gyro or accelerometer rate, causing startup to fail; IMU and video can also resolve to different cameras. Select a single device before discovering rates and bind both pipelines to its serial, or reject ambiguous multi-device startup.

Artifacts

Focused RealSense heterogeneous-device reproduction harness

  • This executable harness extracts the affected method bodies and simulates two devices with distinct IMU rates, providing a reproducible demonstration of the mismatch.

Aggregate capability discovery with unbound device selection

  • The executed baseline shows the methods aggregate 400 Hz gyro and 200 Hz accelerometer support across devices even though selected device A only offers 200 Hz and 100 Hz.

Unbound IMU startup rejects cross-device rates

  • The executed startup path passes the aggregate rates into the unbound pipeline and receives an unsupported-stream failure for device A, confirming the bug.

RealSense SDK availability probe

  • The environment probe shows `pyrealsense2` is unavailable, so validation used the focused executable SDK simulation rather than attached hardware.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +200 to +201
if self.config.enable_imu:
self._start_imu()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Failed startup retains IMU pipeline

start() opens the IMU pipeline before video startup but does not roll it back if video startup or subsequent initialization raises. The IMU device remains held, so a retry or another camera consumer fails until it is manually stopped. Stop and clear the IMU pipeline, along with any partially started video pipeline, before re-raising the startup error.

Artifacts

Focused RealSense IMU startup failure harness

  • A mocked RealSense harness executes the actual AST-extracted `RealSenseCamera.start` method with video startup failure and retry scenarios, demonstrating the unhandled resource rollback.

IMU remains open after video startup failure

  • The unmodified start-path run shows a simulated video failure, no IMU stop call, and a retry rejected because the IMU device resource is still open.

Manual IMU cleanup permits retry

  • The cleanup control run manually stops the leaked IMU after the same video failure and then successfully retries startup, confirming the required rollback behavior.

View artifacts

T-Rex Ran code and verified through T-Rex

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant