Feat/pim/m20 kronknav - #3776
Conversation
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #3776 +/- ##
==========================================
- Coverage 77.59% 77.50% -0.09%
==========================================
Files 1311 1320 +9
Lines 124205 125152 +947
Branches 10875 11171 +296
==========================================
+ Hits 96375 96999 +624
+ Misses 25059 25020 -39
- Partials 2771 3133 +362
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 116 files with indirect coverage changes 🚀 New features to boost your workflow:
|
github-actions
Bot
added
the
ready-to-merge
Greptile SummaryThis change adds M20 ROS/DDS control, Point-LIO localization, navigation components, camera relays, deployment support, and CPU-affinity handling for native children. A malformed lidar Confidence Score: 3/5Not safe to merge until Point-LIO rejects coordinate fields that extend past a point record. The affected validation and native read path were exercised with a valid control case and a malformed boundary-crossing coordinate field. AddressSanitizer reported the predicted out-of-bounds read for the malformed input. Files Needing Attention: dimos/robot/deeprobotics/m20/pointlio/cpp/main.cpp
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (offsets.timestamp + sizeof(double) > static_cast<std::size_t>(cloud.point_step) || | ||
| offsets.ring + sizeof(uint16_t) > static_cast<std::size_t>(cloud.point_step) || | ||
| offsets.intensity + sizeof(float) > static_cast<std::size_t>(cloud.point_step)) { | ||
| throw std::runtime_error("M20 point fields extend past point_step"); | ||
| } |
There was a problem hiding this comment.
validate_m20_cloud accepts FLOAT32 x, y, and z fields when their offsets are below point_step, but it does not verify that the complete four-byte values fit within the point record. A one-point cloud with point_step = 32, 32 bytes of data, and x.offset = 31 passes validation; the later read_unaligned<float> at lines 544–546 reads four bytes beginning at byte 31. AddressSanitizer confirms that this reads past the allocation. Reject coordinate fields whose offset + sizeof(float) exceeds point_step, just as the timestamp, ring, and intensity fields are checked.
Artifacts
Focused parser reproduction script
- Builds and runs an AddressSanitizer harness that extracts the reviewed validation and read helpers, showing the valid and malformed cases side by side.
Focused parser reproduction source
- Contains the extracted production validation/read code and minimal PointCloud2 inputs, including the malformed FLOAT32 x field at offset 31 of a 32-byte record.
AddressSanitizer harness build log
- Shows the focused C++ harness compiled successfully with AddressSanitizer enabled.
- Runs a valid 32-byte point record with x at offset 0; validation and coordinate reads complete successfully with exit 0.
Malformed coordinate field AddressSanitizer failure
- Runs the accepted malformed record with x at offset 31 and captures AddressSanitizer's four-byte out-of-bounds read past the 32-byte allocation.
Native M20 build environment result
- Attempts the repository's native Point-LIO build script and shows it stops because `/opt/robot/scripts/setup_ros2.sh` is absent.
|
|
||
| config: M20PointLioConfig | ||
|
|
||
| lidar: Out[PointCloud2] |
There was a problem hiding this comment.
Do the point clouds coming from the ray tracer look like they are correct? Since the two sensor clouds are getting merged together the points are getting traced through the base_link instead of the sensor origins I'd expect that ray tracing won't be great in all cases
There was a problem hiding this comment.
I think @jeff-hykin is working on supporting this rn
| debug: bool = False | ||
|
|
||
|
|
||
| class M20PointLio(NativeModule, perception.Lidar): |
There was a problem hiding this comment.
Ideally we have a general PointLio module and then we just need to write drivers for each lidar. Don't expect that here because it's not really refactored for that, but just thought I'd drop that in as an fyi
| logger = setup_logger() | ||
|
|
||
|
|
||
| class _M20CameraRelay(Module): |
There was a problem hiding this comment.
Should this module maybe be moved out of the blueprint?
| throw std::runtime_error("M20 cloud has no finite Point-LIO returns"); | ||
| } | ||
|
|
||
| std::sort(points.begin(), points.end(), |
There was a problem hiding this comment.
Why sort the entire point cloud if we just need min and max timestamps?
2 participants
Adds the M20 integration machinery: native ROS/DDS command bridge, robot lifecycle/control, direct lidar and IMU ingestion into Point-LIO, KronkNav mapping/planning/control, and front/rear camera streams.
Extends NativeModuleConfig with optional Linux CPU affinity so Point-LIO, ray tracing, and MLS can be assigned to appropriate RK3588 cores, this is necessary as otherwise performance becomes severely degraded.
Some notes:
This runs fully on the RK3588, further tuning will be required as the mapper is always running behind and this gets worse as the global map grows, the bottleneck is RayTracingVoxelMap on this device.
Planning on flat terrain works consistently, planning over stairs is hit or miss. Naturally since the map is always behind dynamic obstacle avoidance does not work.
How to run:
On the M20:
Configure permissions:
./dimos/robot/deeprobotics/m20/deploy/setup.sh
Start blueprint:
dimos --transport lcm --rerun-host 0.0.0.0 run deeprobotics-m20-kronknav-control
Attach the control shell:
dimos --transport lcm shell
app.M20Connection.standup()
app.M20Connection.liedown()
On laptop:
dimos-viewer --connect rerun+http://10.21.31.104:9877/proxy
--ws-url ws://10.21.31.104:3030/ws
m20forgithub.mov