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

SW-5466: Support Velodyne and other point types in ouster-ros driver #216

Merged
merged 19 commits into from
Nov 13, 2023

Conversation

Samahu
Copy link
Contributor

@Samahu Samahu commented Sep 13, 2023

Related Issues & PRs

Summary of Changes

  • added the ability to customize the published point clouds(s) to velodyne point cloud format and
    other common pcl point types.
  • made ouster_image_component operate separately from ouster_cloud_component

Validation

  • Run the driver with default parameters and verify no observable behavioral changes.
  • Change point_type parameter to native and verify that it reflects the same point cloud format that we get from the sensor.
  • Change point_type parameter to xyz and verify that the published point cloud format only has xyz fields, regardless of the underlying sensor format.
  • Change point_type parameter to xyzi and verify that the published point cloud format only has xyz fields plus intensity, regardless of the underlying sensor format (you should get a warning when the low data mode is selected for this mode).
  • Change point_type parameter to xyzir and verify that the published point cloud format only has xyz fields plus intensity plus ring, regardless of the underlying sensor format (you should get a warning when the low data mode is selected for this mode).

@Samahu Samahu added the enhancement New feature or request label Sep 13, 2023
@Samahu Samahu self-assigned this Sep 13, 2023
@Samahu Samahu changed the title Sw 5466 support velodyne point type in the ros driver SW-5466: Support Velodyne and other point types in ouster-ros driver Sep 13, 2023
@Samahu Samahu requested a review from pallenouster October 12, 2023 19:32
Copy link

@mtswisher mtswisher left a comment

Choose a reason for hiding this comment

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

Just made some typo changes :) How do you decide whether to designate a function as inline? I haven't seen the factory pattern in quite awhile so it was cool to see that solve this problem. Will probably need to dig into some more to really understand what is going on but LGTM :)

ouster-ros/config/driver_params.yaml Outdated Show resolved Hide resolved
ouster-ros/include/ouster_ros/sensor_point_types.h Outdated Show resolved Hide resolved
ouster-ros/include/ouster_ros/sensor_point_types.h Outdated Show resolved Hide resolved
@Samahu
Copy link
Contributor Author

Samahu commented Oct 13, 2023

Just made some typo changes :)

Addressed!

How do you decide whether to designate a function as inline?

Typically for short one liner methods this could minimally increase the code size but avoid having to invoke the method which is always faster .. With modern compilers the inline keyword only serves as a suggestion, compilers could inline methods automatically .. and could also ignore the suggestion if the method body is too long (in this case one needs to use __forceinline).

I haven't seen the factory pattern in quite awhile so it was cool to see that solve this problem. Will probably need to dig into some more to really understand what is going on but LGTM :)

Yeah, the factory pattern is definitely needed here. I will add more comments to the PR to explain what's going on and how could developers use existing code to implement other point types that are not included in this PR.

@Samahu Samahu marked this pull request as ready for review October 18, 2023 15:05
@Samahu Samahu merged commit c828a20 into ros2 Nov 13, 2023
3 checks passed
@Samahu Samahu deleted the SW-5466-support-velodyne-point-type-in-the-ros-driver branch November 14, 2023 16:15
AchmadFathoni pushed a commit to AchmadFathoni/ouster-ros that referenced this pull request Feb 16, 2024
…uster-lidar#216)

* Quick protoype of Velodyne point type
* Add PointXYZIR point type + other major pcl point types
* Include point meta functions and point transform by the ouster_ros namesapce
* Wrap point meta functions with a namespace and use shorter names for the functions +
* Add a seed test module for the point_cloud_compose methods +
 Add description for the point_cloud_compose methods + refactor code and add compile time checks.
* Propagate error state, warn about potential incompatible profile, propagate error state
* Add minimal documentation about the new `point_type` parameter.
EirikKolas added a commit to vortexntnu/ouster-lidar-ros2-driver that referenced this pull request Mar 23, 2024
* SW-5607: rename package ouster_msgs to avoid package name conflict in ros index (ouster-lidar#244)

* rename package ouster_msgs to avoid conflict name conflict in ros index
* set ouster_sensor_msgs version number to match with ouster_ros package

* SW-5466: Support Velodyne and other point types in ouster-ros driver (ouster-lidar#216)

* Quick protoype of Velodyne point type
* Add PointXYZIR point type + other major pcl point types
* Include point meta functions and point transform by the ouster_ros namesapce
* Wrap point meta functions with a namespace and use shorter names for the functions +
* Add a seed test module for the point_cloud_compose methods +
 Add description for the point_cloud_compose methods + refactor code and add compile time checks.
* Propagate error state, warn about potential incompatible profile, propagate error state
* Add minimal documentation about the new `point_type` parameter.

* SW-5466: Support Velodyne point type in the ROS driver amendments (ouster-lidar#254)

* Add support to control point_type through launch.xml files +
* Add a note to CHANGELOG about the breaking change for ptp/utc time offset

* SW-5623: Bump up ouster_client to 20231031 release (ouster-lidar#262)

* Bump ouster-client to 2023103 release
* fix: gracefully stop the driver when shutdown is requested.

* ROS-196: laser scan from ros driver is not properly aligned with point cloud [humble] (ouster-lidar#203)

* Apply destagger to laser scan + Add laser to RVIZ
* Align LaserScan with the PointCloud
* Apply proper pixel shift
* Resolve the issue of zeroed laserscan on dual mode
* Address an issue where LaserScan appeared different on FW prior to 2.4
* Fix the issue for odd numbers
* List selected sensors on the main page + Update RVIZ config to highlight the 2D LaserScan.

* Use timeout when waiting for packets to be proceed in case they don't come (ouster-lidar#293)

* docs: fix spelling mistakes (ouster-lidar#296)

---------

Co-authored-by: Ussama Naal <[email protected]>
Co-authored-by: Andre Nguyen <[email protected]>
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
Development

Successfully merging this pull request may close these issues.

3 participants