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

ROS2 compatibility mode #146

Merged
merged 38 commits into from
Jun 23, 2023
Merged

ROS2 compatibility mode #146

merged 38 commits into from
Jun 23, 2023

Conversation

Samahu
Copy link
Contributor

@Samahu Samahu commented May 31, 2023

Related Issues & PRs

Summary of Changes

  • Build compatibility mode with Steve Macenski's implementation of Ouster driver for ROS2
  • Combine the functionality of os_sensor and os_cloud into a single node/component named os_driver
  • Retain the os_sensor and os_cloud for backward compatibility and also still be able to record the raw packets
  • Add unit tests around the ThreadSafeRingBuffer
  • Add an option to select the frame of reference when publishing point cloud and use lidar_frame by default
  • Provide topic mapping that is compatible with Steve Macenski's Ouster ROS2 driver
    • Provide support for parsing Steve's Ouster ROS2 yaml format
      • Partially support proc_mask handling (all flags supported excluding the IMG)
    • Create and utilize an RVIZ config file that immediately maps to the topics of the community driver
  • Add the ability to publish sensor_msgs::msg::LaserScan
    • Added a scan_ring parameter to enable picking a specific ring.
  • Add ability to override QoS settings
  • Factored out the functionality of OusterImage and integrate into OusterDriver same as os_cloud

Performance Comparison

Still in the process of running a full benchmark but in general the current data shows the following:

  • Improved overall performance over the current mainline driver when using the new os_driver node (a reduction in CPU utilization by 30%)
  • Improved overall performance and stability over the community driver when using os_driver node (a reduction in CPU utilization by 15-20%)

TODOs

  • Provide topic mapping that is compatible with Steve Macenski's Ouster ROS2 driver
  • Provide support for parsing Steve's Ouster ROS2 yaml format
    • Support the proc_mask parameter
  • Add an option to use select the frame_id for published point clouds between os_lidar and os_sensor.
  • Hook up the implemented ThreadSafeRingBuffer to minimize packets drop in case the processing thread falls behind.
    • Add units around the ThreadSafeRingBuffer
    • Reduce the number of synchronization objects needed
  • Add the ability to publish sensor_msgs::msg::LaserScan
    • Configurable ring (De-scoped)
  • Configurable QoS (allow system default qos)
  • Factor out the functionality of OusterImage and integrate into OusterDriver same as os_cloud (De-scoped)

Validation

Old Interface Maintained

The three launch commands should be fully preserved under the new changes

# regular launch
ros2 launch ouster_ros sensor.launch.xml    \
    sensor_hostname:=<sensor host name>
# record
ros2 launch ouster_ros record.launch.xml    \
    sensor_hostname:=<sensor host name>     \
    bag_file:=<optional bag file name>      \
    metadata:=<json file name>              # optional
# replay
ros2 launch ouster_ros replay.launch.xml    \
    bag_file:=<path to rosbag file>         \
    metadata:=<json file name>              # optional if bag file has /metadata topic

Single Node (os_sensor+os_cloud+os_image)

To use the new combined node os_driver run the launch file driver.launch.py as shown below but please find the driver_config.yaml and adjust to match your sensor settings:

ros2 launch ouster_ros driver.launch.py

Single Node (Compatibility Mode)

To have the same topic names as the community driver use driver_launch.py launch file instead, this launch file points to the community_driver_config.yaml and it should work seamlessly except for the IMG option of proc_mask parameter (all flags should work as expected)

ros2 launch ouster_ros driver_launch.py

proc_mask flags

  • When running in a single node test enabling \ disabling individual flags for the proc_mask.
    • For example enable only the IMU, SCAN, IMG and PCL.
    • Watch that only the specific message of the enabled message type actually gets published

Alternate QoS settings

  • Test enabling \ disabling the use_system_default_qos
    Note that when setting use_system_default_qos to true RViz won't be able to display the point cloud by default. You would need to select /ouster/points topic and adjust the policy to be reliable.

Alternate PointCloud frame

  • Test altering point_cloud_frame between os_lidar and os_sensor
    Altering between the two should be clearly reflected in the the rendered point cloud in RVIZ

Sensor Lifecycle Management

  • Connect the sensor, then invoke the /ouster/reset service
    ros2 service call /ouster/reset std_srvs/srv/Empty
    • Watch point cloud update pauses for couple of second while the sensor restarts then the node resumes normally.
  • Connect to the sensor, open the sensor configuration change and apply any change except the udp_lidar_profile.
    • Watch the os_driver node detects the change and re-adapts accordingly.
  • Connect to the sensor, open the sensor configuration change and change the udp_lidar_profile.
    • Watch the sensor, generates few errors and then performs successfully re-activates itself.

Samahu added 2 commits May 26, 2023 11:24
Added required launch files +
Better abstraction of classes +
Simplified threading logic +
Added thread safe implemention of ring buffer (not hooked yet)
@Samahu Samahu added the enhancement New feature or request label May 31, 2023
@Samahu Samahu self-assigned this May 31, 2023
@Samahu Samahu requested a review from celentes June 8, 2023 23:32
ouster-ros/src/thread_safe_ring_buffer.h Outdated Show resolved Hide resolved
ouster-ros/src/os_sensor_node.h Outdated Show resolved Hide resolved
@@ -1,7 +1,7 @@
# Official ROS driver for Ouster sensors

[ROS1 (melodic/noetic)](https://github.com/ouster-lidar/ouster-ros/tree/master) |
[ROS2 (rolling/humble)](https://github.com/ouster-lidar/ouster-ros/tree/ros2) |
[ROS2 (rolling/humble/iron)](https://github.com/ouster-lidar/ouster-ros/tree/ros2) |
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

ouster-ros/launch/driver.launch.py Outdated Show resolved Hide resolved
@Samahu Samahu merged commit 7191da6 into ros2 Jun 23, 2023
@Samahu Samahu deleted the ros2-compatibility-mode branch June 23, 2023 16:51
stephen-derosa referenced this pull request Aug 14, 2023
* Add thread_safe_ring_buffer + Switch to using threads for packet processing in os_sensor node +
Add new wreset service + Add metadata service to os_replay
* implemented the refactored os_cloud_nodelet + adding relevant processors
* Backport os_image_nodelet + add image_processor + Add a switch between dynamic and static broadcast
* Refactoring and fixes for the os_cloud_nodelet + Only create publisher when processors are enabled
* Add a new os_driver nodelet + Add a new launch file to utilize os_driver +
Made it is possible to set custom tf frames + Made it possible to set proc_mask and scan_ring
* Add ring_buffer_test + enable building and running within docker
* Adjust the tests run command
* Source the workspace before running tests
* Enable bash as the shell
* Rename nodelets_os namesapce to ouster_ros + Rename nodelets_os.xml to ouster_ros_nodelet.xml +
Wrap all classes with ouster_ros + always default no_bond + Fix Dockerfile build
* Update CHANGELOG.txt
* Update rviz color scheme and changelog
* Update README.md to direct people to use driver.launch instead of sensor.launch
* Add a section describing how to obtain detailed description of launch args
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.

4 participants