Pointcloud partially flickering (incomplete) in rviz.

[batzor]

Describe the bug
PointCloud flickering in RVIZ. ros2_ouster_driver works without problem.

To Reproduce
Steps to reproduce the behavior (steps below are just an example):

1. ROS Foxy
2. Clone ros2-foxy branch and colcon build.
3. ros2 launch ouster_ros sensor.independent.launch.py
4. params:

ouster/os_sensor:
  ros__parameters:
    sensor_hostname: '192.168.2.82'
    udp_dest: '192.168.2.81'
    mtp_dest: ''
    mtp_main: false
    lidar_mode: '512x10'
    timestamp_mode: 'TIME_FROM_ROS_TIME'
    udp_profile_lidar: 'LEGACY'
    metadata: ''
    lidar_port: 7502
    imu_port: 7503
ouster/os_cloud:
  ros__parameters:
    tf_prefix: ''
    timestamp_mode: 'TIME_FROM_ROS_TIME'

Screenshots

Platform (please complete the following information):

• Ouster Sensor? OS-0-64-U13
• Ouster Firmware Version? v2.4.0
• ROS version/distro? Foxy
• Operating System? Ubuntu 20.04
• Machine Architecture? ARM
• git commit hash (if not the latest).

[Samahu]

Do you connect to the sensor directly via an Ethernet cable or do you have a usb-ethernet converter in the middle?

In case you have used our ROS1 driver, have you observed similar behavior or you have only seen this issue when trying to integrate with the ROS2 driver?

I do suspect that this has to do with the ROS2 QoS settings. The driver currently uses SensorData QoS which uses "Best Effort" and has only 5 depth for the history. I do have ticket to make this configurable but if you are familiar with ROS2 and C++ you could try fiddling with them and see for example selecting "Reliable" QoS policy and/or increasing the history depth more than 5 helps.

[grischi]

I am observing this behavior as well (arm64 & amd64, OS-1-64-U02, ros foxy, ubuntu 20.04, firmware v3.0.1+20230209044733). Reminds me of an issue that was fixed some time ago in the ros2_ouster_drivers project: ros-drivers/ros2_ouster_drivers#97

[rjwb1]

I'm also experiencing this issue on a pair of OS1-32 using a Jetson Orin with Ubuntu 20 and a 1Gbt Network switch. I have another system with this configuration that doesn't seem to have this issue. I am running ros 1 noetic.

[Samahu]

Thank you for the input, and thank you @grischi for referencing the similar issue, I do have plans to change processing from timers to threads but I have considered this as an optimization, so it had less priority on my list. We'll try to address this ASAP.

[grischi]

Hi @Samahu I have been able to resolve that issue based on your suggestion of QoS tuning. For the topic "lidar_packets", I have increased the history depth on publisher and subscriber side and that problem disappeared (the full cloud is now visible in rviz). Do you want me to send a PR?

[Samahu]

🎉 That's great to hear!

Yes, by all means please submit a PR, I don't know if I would merge it right away since as I stated earlier, the plan is provide users of the driver with the ability to set QoS options through parameters.yaml. Nonetheless, the settings you have just verified to work will help other users who are facing the same issue and I will use as the default setting once we provide users with the ability to set QoS through parameters.yaml file.

[grischi]

In this case, let me just paste a diff here:

modified   ouster-ros/src/os_cloud_node.cpp
@@ -99,7 +99,7 @@ class OusterCloud : public OusterProcessingNodeBase {
     }
 
     void create_subscriptions() {
-        rclcpp::SensorDataQoS qos;
+        rclcpp::SensorDataQoS qos(rclcpp::KeepLast(20));
 
         using LidarHandlerFunctionType =
             std::function<void(const PacketMsg::ConstSharedPtr)>;
modified   ouster-ros/src/os_sensor_node.cpp
@@ -643,7 +643,7 @@ class OusterSensor : public OusterSensorNodeBase {
     }
 
     void create_publishers() {
-        rclcpp::SensorDataQoS qos;
+        rclcpp::SensorDataQoS qos(rclcpp::KeepLast(20));
         lidar_packet_pub = create_publisher<PacketMsg>("lidar_packets", qos);
         imu_packet_pub = create_publisher<PacketMsg>("imu_packets", qos);
     }

However, just to mention, in my opinion, the "standard" user shall not be aware of the existence of a driver-internal topic like lidar_packets. I would therefore prefer not to have to configure QoS settings for this specific topic in a config file. In fact, I would favor a design that does not include such a topic at all, for performance reasons: The transport of messages of type PacketMsg.msg with a variable-length array cannot be done without creating a copy in cyclonedds/iceoryx (If I understood that correctly: https://github.com/ros2/rmw_cyclonedds/blob/rolling/shared_memory_support.md#zero-copy).

[Samahu]

In this case, let me just paste a diff here

That works too.

However, just to mention, in my opinion, the "standard" user shall not be aware of the existence of a driver-internal topic like lidar_packets. I would therefore prefer not to have to configure QoS settings for this specific topic in a config file. In fact, I would favor a design that does not include such a topic at all, for performance reasons: The transport of messages of type PacketMsg.msg with a variable-length array cannot be done without creating a copy in cyclonedds/iceoryx (If I understood that correctly: https://github.com/ros2/rmw_cyclonedds/blob/rolling/shared_memory_support.md#zero-copy).

Yes, I am working towards not having the raw packets published on the ROS messaging bus. Should be due soon.

[grischi]

Sounds great, looking forward to that (y)

[Nosille]

I find the current method of publishing raw packets convenient for rosbag purposes. Recording raw packets results in smaller bags, and we get to incorporate improvements made to the rest of the nodes on bag playback. We can even vary some settings during playback sessions.

[Samahu]

I find the current method of publishing raw packets convenient for rosbag purposes. Recording raw packets results in smaller bags, and we get to incorporate improvements made to the rest of the nodes on bag playback. We can even vary some settings during playback sessions.

@Nosille the current plan is to make publishing raw packets optional, which would be off by default but enabled during record and replay scenarios. It is for the reasons that you have stated.

[grischi]

Woudn't it be possible to cover the use-case of @Nosille with a network dump?

[Samahu]

Woudn't it be possible to cover the use-case of @Nosille with a network dump?

It would, but bag files are more convenient if you are already using ROS. Plus, recently we have started saving the sensor metadata into the bag file along the raw packets (since #102 and #103), which made it more convenient. With network dump the user would be responsible for maintaining and keeping record of the appropriate metadata file that match the recorded session (which is very error prone; the metadata file is lost or overwritten, ...).

[rjwb1]

@Samahu I have this issue using ROS 1 on the master branch. Would it be possible to fix this issue there?

[Samahu]

@Samahu I have this issue using ROS 1 on the master branch. Would it be possible to fix this issue there?

It is possible but I am prioritizing the fix for ros2 first then will back port it to ros1