Skip to content

feat(recording): go2_mid360 + mid360_realsense_30 recorders#2588

Open
jeff-hykin wants to merge 5 commits into
mainfrom
jeff/feat/mid360_recorders
Open

feat(recording): go2_mid360 + mid360_realsense_30 recorders#2588
jeff-hykin wants to merge 5 commits into
mainfrom
jeff/feat/mid360_recorders

Conversation

@jeff-hykin

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

Copy link
Copy Markdown
Member

go2_mid360 and mid360_realsense recording blueprints

Add memory2-based record blueprints for the Go2+Mid-360 and RealSense
D435i+Mid-360 rigs:

- PointlioPoseRecorder: shared Recorder base stamping each lidar frame
  with the latest odometry pose.
- StaticTfPublisher: republishes a rig's static mount frames onto /tf on
  an interval (PubSubTF has no latched static tf), so they're captured in
  the recording's tf stream.
- Go2Mid360Recorder / Mid360RealsenseRecorder + their static-transform
  trees and record blueprints (unitree-go2-mid360-record,
  mid360-realsense-record). Pygame WASD teleop on the go2 rig.
- Raw Livox capture is opt-in via RECORD_PCAP=1 (reuses the existing
  Mid360PcapRecorder); default off.
- Recording doc moved to experimental/docs/nav/map_recording/go2_mid360.md
  (post-processing stripped).

Regenerated all_blueprints.py.
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds two new map-recording blueprints — unitree-go2-mid360-record for the Go2 + Mid-360 rig and mid360-realsense-record / mid360-realsense-record-with-pcap for a standalone Mid-360 + RealSense D435i rig — along with a shared PointlioPoseRecorder base class and a new StaticTfPublisher infrastructure module.

  • PointlioPoseRecorder: new base recorder that stamps each lidar frame with the latest odometry pose from Point-LIO, with a staleness guard (_POSE_MATCH_TOL = 0.1 s) so frames whose odometry has dropped out are stored unposed and map-skipped rather than silently registered at a wrong location.
  • StaticTfPublisher: re-publishes fixed mount-frame transforms at a configurable Hz via a background loop so the rig geometry is captured in the tf stream throughout a recording; subclassed by both Go2Mid360StaticTf and Mid360RealsenseStaticTf with their respective physical extrinsics.
  • Registry entries: all new blueprints and modules are wired into all_blueprints.py in correct alphabetical order.

Confidence Score: 4/5

Safe to merge; the only open issue is that StaticTfPublisher._publish_loop has no exception handler around tf.publish(), so a single transient error permanently terminates the mount-frame stream for the rest of a recording run.

The _publish_loop in StaticTfPublisher calls self.tf.publish() bare inside a while self._running loop with no try/except. A transient tf-channel error would propagate out of the coroutine, end the background task permanently, and silently drop all mount-frame transforms for the remainder of the recording session — corrupting spatial anchoring of any post-processing tool that needs those frames. This was noted in the previous review round and remains unaddressed.

dimos/protocol/tf/static_tf_publisher.py — the _publish_loop method needs exception handling around the tf.publish() call to survive transient errors.

Important Files Changed

Filename Overview
dimos/hardware/sensors/lidar/pointlio/pose_recorder.py New base recorder that stamps lidar frames with odometry pose; correctly implements the staleness guard (_POSE_MATCH_TOL = 0.1 s) that was requested in previous review feedback.
dimos/protocol/tf/static_tf_publisher.py New module that re-publishes static tf transforms on a fixed interval; _publish_loop has no exception handler around tf.publish(), so a single publish failure permanently terminates the loop (flagged in a previous review).
dimos/robot/unitree/go2/blueprints/basic/unitree_go2_mid360_record.py Go2 + Mid-360 drive-and-record blueprint; PST timezone issue from previous review is now correctly resolved using datetime.now().astimezone().strftime("%Z").
dimos/hardware/sensors/lidar/mid360_realsense_30/blueprints.py Clean blueprint definitions for the RealSense D435i + Mid-360 rig with proper remappings and optional pcap variant.
dimos/hardware/sensors/lidar/mid360_realsense_30/static_transforms.py Static frame tree for D435i + Mid-360 rig, sourced from the official realsense2_description xacro and Livox extrinsics; well-commented and correct.
experimental/docs/nav/map_recording/go2_mid360.md New user-facing guide for Go2 + Mid-360 recording; clear and accurate except for one stale example path that still shows a hardcoded -PST timezone suffix.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Mid360
    participant PointLio
    participant PointlioPoseRecorder
    participant StaticTfPublisher
    participant Memory2DB

    Mid360->>PointLio: livox_lidar / livox_imu
    PointLio->>PointlioPoseRecorder: pointlio_odometry
    PointlioPoseRecorder->>PointlioPoseRecorder: _odom_pose() stores pose + raw_ts
    PointLio->>PointlioPoseRecorder: pointlio_lidar
    PointlioPoseRecorder->>PointlioPoseRecorder: _lidar_pose() checks staleness (≤0.1s)
    PointlioPoseRecorder->>Memory2DB: frame + pose (or unposed if stale)

    loop every 1/publish_hz seconds
        StaticTfPublisher->>StaticTfPublisher: stamp transforms with time.now()
        StaticTfPublisher->>Memory2DB: tf.publish(mount frames)
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Mid360
    participant PointLio
    participant PointlioPoseRecorder
    participant StaticTfPublisher
    participant Memory2DB

    Mid360->>PointLio: livox_lidar / livox_imu
    PointLio->>PointlioPoseRecorder: pointlio_odometry
    PointlioPoseRecorder->>PointlioPoseRecorder: _odom_pose() stores pose + raw_ts
    PointLio->>PointlioPoseRecorder: pointlio_lidar
    PointlioPoseRecorder->>PointlioPoseRecorder: _lidar_pose() checks staleness (≤0.1s)
    PointlioPoseRecorder->>Memory2DB: frame + pose (or unposed if stale)

    loop every 1/publish_hz seconds
        StaticTfPublisher->>StaticTfPublisher: stamp transforms with time.now()
        StaticTfPublisher->>Memory2DB: tf.publish(mount frames)
    end
Loading

Reviews (4): Last reviewed commit: "refactor(recording): mid360_realsense as..." | Re-trigger Greptile

Comment on lines +43 to +47
config: PointlioPoseRecorderConfig

pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]

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 No staleness guard on _lidar_pose — stale odometry silently mis-stamps frames

_lidar_pose returns _last_odom_pose unconditionally, with no check on how old that pose is. If Point-LIO temporarily drops its odometry output (degenerate geometry, topic lag, process hiccup), every subsequent lidar frame will be stamped with the last known pose rather than None. None causes the frame to be map-skipped, which is the correct fallback; a stale pose causes it to be registered at the wrong location, silently corrupting the map.

The existing PointlioRecorder uses _POSE_MATCH_TOL = 0.1 s on the raw sensor timestamps (abs(raw_ts - self._last_odom_raw_ts) <= _POSE_MATCH_TOL) to detect exactly this case. The same guard — tracking _last_odom_raw_ts and comparing it against the lidar frame's raw ts — should be applied here so the behavior is consistent.

Comment on lines +34 to +37
class Go2Mid360Recorder(PointlioPoseRecorder):
pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]
go2_lidar: In[PointCloud2]

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.

P2 These two ports are already declared on PointlioPoseRecorder; re-declaring them here is redundant and could confuse readers into thinking the subclass owns them. The same applies to Mid360RealsenseRecorder.

Suggested change
class Go2Mid360Recorder(PointlioPoseRecorder):
pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]
go2_lidar: In[PointCloud2]
class Go2Mid360Recorder(PointlioPoseRecorder):
go2_lidar: In[PointCloud2]

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


def _default_recording_dir() -> Path:
now = datetime.now()
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower() + "-PST"

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.

P2 The -PST suffix is hardcoded regardless of the machine's actual timezone. datetime.now() returns local time, so on a machine set to UTC or any other zone the label is misleading. The same issue exists in mid360_realsense_30/mid360_realsense_record.py at the same line. The simplest fix is to drop the suffix, or use datetime.now().astimezone().strftime("%Z") to pick up the real zone abbreviation.

Suggested change
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower() + "-PST"
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower()

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
dimos/protocol/tf/static_tf_publisher.py 51.02% 24 Missing ⚠️
...s/hardware/sensors/lidar/pointlio/pose_recorder.py 62.50% 12 Missing ⚠️
.../go2/blueprints/basic/unitree_go2_mid360_record.py 67.56% 10 Missing and 2 partials ⚠️
...ors/lidar/mid360_realsense_30/static_transforms.py 96.15% 1 Missing ⚠️
.../robot/unitree/go2/go2_mid360_static_transforms.py 90.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main    #2588      +/-   ##
==========================================
+ Coverage   69.61%   70.76%   +1.15%     
==========================================
  Files         878      875       -3     
  Lines       79326    77630    -1696     
  Branches     7126     6893     -233     
==========================================
- Hits        55220    54936     -284     
+ Misses      22301    20890    -1411     
+ Partials     1805     1804       -1     
Flag Coverage Δ
OS-ubuntu-24.04-arm 62.92% <73.54%> (-0.08%) ⬇️
OS-ubuntu-latest 65.73% <73.54%> (-0.10%) ⬇️
Py-3.10 65.73% <73.54%> (-0.10%) ⬇️
Py-3.11 65.73% <73.54%> (-0.09%) ⬇️
Py-3.12 65.73% <73.54%> (-0.09%) ⬇️
Py-3.13 65.73% <73.54%> (-0.10%) ⬇️
Py-3.14 65.74% <73.54%> (-0.10%) ⬇️
Py-3.14t 65.73% <73.54%> (-0.09%) ⬇️
SelfHosted-Linux 37.80% <ø> (+<0.01%) ⬆️
SelfHosted-macOS 36.59% <ø> (ø)

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

Files with missing lines Coverage Δ
...re/sensors/lidar/mid360_realsense_30/blueprints.py 100.00% <100.00%> (ø)
...ware/sensors/lidar/mid360_realsense_30/recorder.py 100.00% <100.00%> (ø)
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
dimos/robot/unitree/go2/go2_mid360_recorder.py 100.00% <100.00%> (ø)
...ors/lidar/mid360_realsense_30/static_transforms.py 96.15% <96.15%> (ø)
.../robot/unitree/go2/go2_mid360_static_transforms.py 90.00% <90.00%> (ø)
...s/hardware/sensors/lidar/pointlio/pose_recorder.py 62.50% <62.50%> (ø)
.../go2/blueprints/basic/unitree_go2_mid360_record.py 67.56% <67.56%> (ø)
dimos/protocol/tf/static_tf_publisher.py 51.02% <51.02%> (ø)

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

…z label

Address Greptile review on PR #2588:
- pose_recorder: guard pointlio_lidar pose-stamping with _POSE_MATCH_TOL (0.1s)
  on raw odom ts, matching PointlioRecorder. Stale odometry now yields an
  unposed (map-skipped) frame instead of mis-stamping at the last pose.
- go2/realsense recorders: drop redundant pointlio_odometry/pointlio_lidar
  port re-declarations (inherited from PointlioPoseRecorder).
- record blueprints: use datetime.now().astimezone() + %Z for the recordings
  dir label instead of a hardcoded -PST suffix.
mid360_realsense_30/__init__.py was license-header only; the repo forbids
__init__.py under dimos/. Namespace-package import still works.
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 24, 2026
Rename mid360_realsense_record.py -> blueprints.py, drop the __main__ runner,
and expose two blueprints instead of the RECORD_PCAP env toggle:
- mid360_realsense_record (db only)
- mid360_realsense_record_with_pcap (db + raw Mid-360 pcap)

Matches the repo's blueprints.py convention (e.g. virtual_mid360). Regenerated
all_blueprints.py.
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Jun 24, 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