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

Configurable qos and default history depth #229

Open
wants to merge 3 commits into
base: ros2
Choose a base branch
from

Conversation

Aposhian
Copy link

@Aposhian Aposhian commented Oct 12, 2023

Related Issues & PRs

#190
ouster-lidar/ouster-sdk#120

Summary of Changes

Set history depth on lidar packets topic to be something more reasonable.

Allow for configuring qos of the packet topics using parameters. http://design.ros2.org/articles/qos_configurability.html
Note that this requires at least ROS Galactic. It is not compatible with Foxy or earlier. If that backward compatibility is desirable I would like to instead expose a parameter that at least can set the history depth.

This is desirable because neither the system default qos nor the sensor data qos profiles have very large history depths, and the lidar packet topic can be publishing at hundreds of hertz, and I believe it might be dropped packets that are causing missing data in the pointcloud. For example:

os_sensor:
  ros__parameters:
    qos_overrides:
      /lidar_packets:
        publisher:
          depth: 1000
os_cloud:
  ros__parameters:
    qos_overrides:
      /lidar_packets:
        subscription:
          depth: 1000

Note that this also applies to intraprocess communication since the intraprocess buffer is set to the size of the history depth qos.

Validation

Lost packets show up if the os_cloud process is under high load, resulting in a "propeller effect". I can confirm that increasing the history depth eliminates this issue.

@Aposhian Aposhian changed the base branch from master to ros2 October 12, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant