Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NOETIC] Further reduce overhead in the ros driver #363

Merged
merged 21 commits into from
Sep 19, 2024

Conversation

Samahu
Copy link
Contributor

@Samahu Samahu commented Sep 4, 2024

Related Issues & PRs

Summary of Changes

  • Add Jazzy to the build status
  • Update to SDK client 0.11.1
  • Switch to using LidarPacket and ImuPacket
    • Get's rid of the deprecation warnings
  • Publish raw packets for the os_driver
  • Add a flag to control the publish of organized point cloud vs non-organized
  • Add a flag to control the publish of destaggerd point cloud vs staggered
  • Add parameter to set the min/max lidar range on generated point clouds
  • Add a new flag to proc_mask to optionally publish the raw packets when using os_driver

TODO(s)

  • optionally publish raw packets in os_driver
  • publicize the use of OpenMP in cartesianT & point_cloud_compose methods

Validation

  • Run the driver in all different modes of operations
  • Verify that the newly added features:
    • When destagger is set to false verify the driver doesn't destagger the published point cloud
    • When organized is set to false verify that the published point cloud isn't 2D
    • Adjust min_range and max_range and observe the point cloud shrink accordingly.
  • Verify that /ouster/imu_packets and /ouster/lidar_packet appear when RAW flag is passed to os_driver

@Samahu Samahu added the enhancement New feature or request label Sep 4, 2024
@Samahu Samahu self-assigned this Sep 4, 2024
Copy link
Collaborator

@matthew-lidar matthew-lidar left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just a few notes. How does performance compare with the same settings?

src/os_driver_nodelet.cpp Outdated Show resolved Hide resolved
src/point_cloud_processor.h Show resolved Hide resolved
launch/sensor.launch Outdated Show resolved Hide resolved
if (lidar_packet_handler) {
// TODO[UN]: this is not ideal since we can't reuse the msg buffer
// Need to redefine the Packet object and allow use of array_views
sensor::LidarPacket lidar_packet(msg->buf.size());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can actually get ROS to decode a message into a custom data type to avoid this copy (probably).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not familiar with this option, I need to look into it to see if it can help with the situation here

@Samahu Samahu changed the title Further reduce overhead in the ros driver [NOETIC] Further reduce overhead in the ros driver Sep 17, 2024
@Samahu Samahu requested a review from diego-guridi September 17, 2024 15:17
@Samahu Samahu merged commit 646beba into master Sep 19, 2024
1 check passed
@Samahu Samahu deleted the further-reduce-overhead-in-the-ros-driver branch October 3, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants