Skip to content

feat: per-point timing support for PointCloud2#3103

Draft
jeff-hykin wants to merge 10 commits into
mainfrom
jeff/feat/pointcloud_timing
Draft

feat: per-point timing support for PointCloud2#3103
jeff-hykin wants to merge 10 commits into
mainfrom
jeff/feat/pointcloud_timing

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Jul 20, 2026

Copy link
Copy Markdown
Member

Needed for fastlivo (and would be good for pointlio/fastlio)

Partly does the "TODO: work with full spectrum of pointcloud2".

Test with

dimos run mid360

@jeff-hykin
jeff-hykin force-pushed the jeff/feat/pointcloud_timing branch from 1fa46bf to a61f4a4 Compare July 20, 2026 20:42
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds per-point timing metadata to Livox PointCloud2 messages. The main changes are:

  • Publish offset_time, tag, and line fields from the Mid-360 driver.
  • Encode and decode these optional attributes in Python PointCloud2 messages.
  • Add an LCM round-trip test for timing metadata.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
dimos/hardware/sensors/lidar/livox/cpp/main.cpp Adds packed per-point timing and tag fields to the Mid-360 PointCloud2 publisher.
dimos/msgs/sensor_msgs/PointCloud2.py Adds optional timing, tag, and line attributes to PointCloud2 encoding and decoding.
dimos/msgs/sensor_msgs/test_PointCloud2.py Adds round-trip coverage for the new per-point metadata fields.

Reviews (2): Last reviewed commit: "review: vendor-neutral PointCloud2 docs/..." | Re-trigger Greptile

Comment thread dimos/hardware/sensors/lidar/livox/cpp/main.cpp Outdated
Comment thread dimos/hardware/sensors/lidar/livox/cpp/main.cpp Outdated
Comment thread dimos/msgs/sensor_msgs/PointCloud2.py
@jeff-hykin
jeff-hykin marked this pull request as draft July 20, 2026 20:47
…ng config (default on)

Consumers that never read offset_time/tag/line no longer pay for extracting
them (0.50 -> 0.35 ms/frame decode; plain layout 0.21). Mid360Config.per_point_timing=False
restores the legacy 16-byte wire layout entirely.
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
dimos/msgs/sensor_msgs/PointCloud2.py 93.15% 2 Missing and 3 partials ⚠️
@@            Coverage Diff             @@
##             main    #3103      +/-   ##
==========================================
+ Coverage   72.36%   72.38%   +0.02%     
==========================================
  Files        1024     1024              
  Lines       92038    92137      +99     
  Branches     8420     8439      +19     
==========================================
+ Hits        66600    66692      +92     
- Misses      23167    23172       +5     
- Partials     2271     2273       +2     
Flag Coverage Δ
OS-ubuntu-24.04-arm 65.43% <95.04%> (+0.03%) ⬆️
OS-ubuntu-latest 67.77% <95.04%> (+0.02%) ⬆️
Py-3.10 67.77% <95.04%> (+0.03%) ⬆️
Py-3.11 67.76% <95.04%> (+0.02%) ⬆️
Py-3.12 67.76% <95.04%> (+0.02%) ⬆️
Py-3.13 67.76% <95.04%> (+0.02%) ⬆️
Py-3.14 67.77% <95.04%> (+0.02%) ⬆️
Py-3.14t 67.76% <95.04%> (+0.02%) ⬆️
SelfHosted-Large 30.10% <5.05%> (-0.02%) ⬇️
SelfHosted-Linux 37.34% <20.20%> (-0.03%) ⬇️
SelfHosted-macOS 36.21% <20.20%> (-0.03%) ⬇️

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

Files with missing lines Coverage Δ
dimos/hardware/sensors/lidar/livox/module.py 93.18% <100.00%> (+0.15%) ⬆️
dimos/msgs/sensor_msgs/test_PointCloud2.py 98.14% <100.00%> (+0.58%) ⬆️
dimos/msgs/sensor_msgs/PointCloud2.py 64.50% <93.15%> (+5.73%) ⬆️

... and 3 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 added the backport:skip Skip creating a backport to any release branches label Jul 20, 2026
pc.fields[3] = make_field("intensity", 12, sensor_msgs::PointField::FLOAT32);
if (timing) {
pc.fields[4] =
make_field("offset_time", OFFSET_TIME_OFFSET, sensor_msgs::PointField::UINT32);

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.

ROS version of livox follows the same general pattern

…fset_time) / legacy (x,y,z,intensity)

Replaces the per_point_timing bool. No LIO in the stack reads intensity
(passthrough for viz only), so minimal mode carries just geometry + the
per-point timing that scan undistortion needs, at the legacy 16 B/point.
@jeff-hykin jeff-hykin changed the title feat: per-point timing (offset_time/tag/line) in PointCloud2 + Mid-360 driver feat: per-point timing support for PointCloud2 Jul 20, 2026
…e wire default, full-format clouds imply a consumer that reads them
// minimal (default) x,y,z,offset_time — 16 B/point
// full x,y,z,intensity,offset_time,tag,line — 22 B/point
// legacy x,y,z,intensity — 16 B/point
// offset_time is uint32 ns since the header stamp; tag/line are the Livox

@jeff-hykin jeff-hykin Jul 20, 2026

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.

main livox change: publish timing info

# Parse field offsets. The message is self-describing; a known field is
# honored only when its advertised datatype matches what we read it as,
# otherwise it is treated as absent rather than misread.
_expected_datatype = {

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.

note: this is generic, PointField is a ROS struct, and Pointcloud2 supports all these fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip Skip creating a backport to any release branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant