From 0749c54859e655fc5babc2781b624e9d137643ae Mon Sep 17 00:00:00 2001
From: Ussama Naal <ussama.naal@ouster.io>
Date: Mon, 10 Jul 2023 09:54:44 -0700
Subject: [PATCH 1/2] Initialize point_cloud_frame with an empty string + and
 expose point_cloud_frame through launch xml files + Remove unused/duplicate
 param definitions + Update descriptions

---
 ouster-ros/config/driver_params.yaml          |  4 +--
 ouster-ros/launch/record.composite.launch.xml | 29 +++++++++++------
 ouster-ros/launch/replay.composite.launch.xml | 31 +++++++++++++------
 ouster-ros/launch/rviz.launch.xml             |  4 ---
 ouster-ros/launch/sensor.composite.launch.xml | 28 ++++++++++-------
 .../launch/sensor.independent.launch.xml      | 27 +++++++++-------
 ouster-ros/launch/sensor_mtp.launch.xml       | 16 +++++++---
 .../src/os_static_transforms_broadcaster.h    |  2 +-
 8 files changed, 88 insertions(+), 53 deletions(-)

diff --git a/ouster-ros/config/driver_params.yaml b/ouster-ros/config/driver_params.yaml
index 040c9e72..13edc256 100644
--- a/ouster-ros/config/driver_params.yaml
+++ b/ouster-ros/config/driver_params.yaml
@@ -60,7 +60,7 @@ ouster/os_driver:
     # to select which beam of the LidarScan to use when producing the LaserScan
     # message. Choose a value the range [0, sensor_beams_count).
     scan_ring: 0
-    # use_system_default_qos[optional]: if False, data are published with sensor
+    # use_system_default_qos[optional]: if false, data are published with sensor
     # data QoS. This is preferrable for production but default QoS is needed for
     # rosbag. See: https://github.com/ros2/rosbag2/issues/125
-    use_system_default_qos: False
+    use_system_default_qos: false
diff --git a/ouster-ros/launch/record.composite.launch.xml b/ouster-ros/launch/record.composite.launch.xml
index dbedb9eb..d1716c6e 100644
--- a/ouster-ros/launch/record.composite.launch.xml
+++ b/ouster-ros/launch/record.composite.launch.xml
@@ -42,16 +42,28 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
+  <arg name="sensor_frame" default="os_sensor"
+    description="sets name of choice for the sensor_frame tf frame, value can not be empty"/>
+  <arg name="lidar_frame" default="os_lidar"
+    description="sets name of choice for the os_lidar tf frame, value can not be empty"/>
+  <arg name="imu_frame" default="os_imu"
+      description="sets name of choice for the os_imu tf frame, value can not be empty"/>
+  <arg name="point_cloud_frame" default=""
+    description="which frame to be used when publishing PointCloud2 or LaserScan messages.
+    Choose between the value of sensor_frame or lidar_frame, leaving this value empty
+    would set lidar_frame to be the frame used when publishing these messages."/>
 
-  <arg name="use_system_default_qos" default="true" description="Use the default system QoS settings"/>
+  <arg name="use_system_default_qos" default="true"
+    description="Use the default system QoS settings"/>
+
+  <arg name="proc_mask" default="IMG|PCL|IMU|SCAN" description="
+    The IMG flag here is not supported and does not affect anything,
+    to disable image topics you would need to omit the os_image node
+    from the launch file"/>
 
   <arg name="scan_ring" default="0" description="
     use this parameter in conjunction with the SCAN flag
-    and choose a value the range [0, sensor_beams_count)"
-  />
+    and choose a value the range [0, sensor_beams_count)"/>
 
   <group>
     <push-ros-namespace namespace="$(var ouster_ns)"/>
@@ -73,8 +85,10 @@
         <param name="sensor_frame" value="$(var sensor_frame)"/>
         <param name="lidar_frame" value="$(var lidar_frame)"/>
         <param name="imu_frame" value="$(var imu_frame)"/>
+        <param name="point_cloud_frame" value="$(var point_cloud_frame)"/>
         <param name="timestamp_mode" value="$(var timestamp_mode)"/>
         <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
+        <param name="proc_mask" value="$(var proc_mask)"/>
         <param name="scan_ring" value="$(var scan_ring)"/>
       </composable_node>
       <composable_node pkg="ouster_ros" plugin="ouster_ros::OusterImage" name="os_image">
@@ -93,9 +107,6 @@
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">
     <arg name="ouster_ns" value="$(var ouster_ns)"/>
     <arg name="rviz_config" value="$(var rviz_config)"/>
-    <arg name="sensor_frame" value="$(var sensor_frame)"/>
-    <arg name="lidar_frame" value="$(var lidar_frame)"/>
-    <arg name="imu_frame" value="$(var imu_frame)"/>
   </include>
 
   <let name="_use_bag_file_name" value="$(eval '\'$(var bag_file)\' != \'b\'')"/>
diff --git a/ouster-ros/launch/replay.composite.launch.xml b/ouster-ros/launch/replay.composite.launch.xml
index d9c685e1..73755ebc 100644
--- a/ouster-ros/launch/replay.composite.launch.xml
+++ b/ouster-ros/launch/replay.composite.launch.xml
@@ -19,19 +19,31 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
+  <arg name="sensor_frame" default="os_sensor"
+    description="sets name of choice for the sensor_frame tf frame, value can not be empty"/>
+  <arg name="lidar_frame" default="os_lidar"
+    description="sets name of choice for the os_lidar tf frame, value can not be empty"/>
+  <arg name="imu_frame" default="os_imu"
+      description="sets name of choice for the os_imu tf frame, value can not be empty"/>
+  <arg name="point_cloud_frame" default=""
+    description="which frame to be used when publishing PointCloud2 or LaserScan messages.
+    Choose between the value of sensor_frame or lidar_frame, leaving this value empty
+    would set lidar_frame to be the frame used when publishing these messages."/>
 
   <let name="_use_metadata_file" value="$(eval '\'$(var metadata)\' != \'\'')"/>
 
-  <arg name="use_system_default_qos" default="true" description="Use the default system QoS settings"/>
+  <arg name="use_system_default_qos" default="true"
+    description="Use the default system QoS settings"/>
+
+  <arg name="proc_mask" default="IMG|PCL|IMU|SCAN" description="
+    The IMG flag here is not supported and does not affect anything,
+    to disable image topics you would need to omit the os_image node
+    from the launch file"/>
+
 
   <arg name="scan_ring" default="0" description="
     use this parameter in conjunction with the SCAN flag
-    and choose a value the range [0, sensor_beams_count)"
-  />
-
+    and choose a value the range [0, sensor_beams_count)"/>
 
   <group>
     <push-ros-namespace namespace="$(var ouster_ns)"/>
@@ -44,8 +56,10 @@
         <param name="sensor_frame" value="$(var sensor_frame)"/>
         <param name="lidar_frame" value="$(var lidar_frame)"/>
         <param name="imu_frame" value="$(var imu_frame)"/>
+        <param name="point_cloud_frame" value="$(var point_cloud_frame)"/>
         <param name="timestamp_mode" value="$(var timestamp_mode)"/>
         <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
+        <param name="proc_mask" value="$(var proc_mask)"/>
         <param name="scan_ring" value="$(var scan_ring)"/>
       </composable_node>
       <composable_node pkg="ouster_ros" plugin="ouster_ros::OusterImage" name="os_image">
@@ -66,9 +80,6 @@
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">
     <arg name="ouster_ns" value="$(var ouster_ns)"/>
     <arg name="rviz_config" value="$(var rviz_config)"/>
-    <arg name="sensor_frame" value="$(var sensor_frame)"/>
-    <arg name="lidar_frame" value="$(var lidar_frame)"/>
-    <arg name="imu_frame" value="$(var imu_frame)"/>
   </include>
 
   <let name="_use_bag_file_name" value="$(eval '\'$(var bag_file)\' != \'b\'')"/>
diff --git a/ouster-ros/launch/rviz.launch.xml b/ouster-ros/launch/rviz.launch.xml
index 54b490b7..0c9beb17 100644
--- a/ouster-ros/launch/rviz.launch.xml
+++ b/ouster-ros/launch/rviz.launch.xml
@@ -5,10 +5,6 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
-
   <group>
     <push-ros-namespace namespace="$(var ouster_ns)"/>
     <node pkg="rviz2" exec="rviz2" name="rviz2" output="screen"
diff --git a/ouster-ros/launch/sensor.composite.launch.xml b/ouster-ros/launch/sensor.composite.launch.xml
index fa66dabb..e66fd3d9 100644
--- a/ouster-ros/launch/sensor.composite.launch.xml
+++ b/ouster-ros/launch/sensor.composite.launch.xml
@@ -40,19 +40,26 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
+  <arg name="sensor_frame" default="os_sensor"
+    description="sets name of choice for the sensor_frame tf frame, value can not be empty"/>
+  <arg name="lidar_frame" default="os_lidar"
+    description="sets name of choice for the os_lidar tf frame, value can not be empty"/>
+  <arg name="imu_frame" default="os_imu"
+      description="sets name of choice for the os_imu tf frame, value can not be empty"/>
+  <arg name="point_cloud_frame" default=""
+    description="which frame to be used when publishing PointCloud2 or LaserScan messages.
+    Choose between the value of sensor_frame or lidar_frame, leaving this value empty
+    would set lidar_frame to be the frame used when publishing these messages."/>
 
-  <arg name="use_system_default_qos" default="false" description="Use the default system QoS settings"/>
+  <arg name="use_system_default_qos" default="false"
+    description="Use the default system QoS settings"/>
+  
   <arg name="proc_mask" default="IMG|PCL|IMU|SCAN" description="
-    use any combination of the 4 flags to enable or disable specific processors"
-  />
+    use any combination of the 4 flags to enable or disable specific processors"/>
 
   <arg name="scan_ring" default="0" description="
     use this parameter in conjunction with the SCAN flag
-    and choose a value the range [0, sensor_beams_count)"
-  />
+    and choose a value the range [0, sensor_beams_count)"/>
 
   <group>
     <push-ros-namespace namespace="$(var ouster_ns)"/>
@@ -65,11 +72,11 @@
       <param name="imu_port" value="$(var imu_port)"/>
       <param name="udp_profile_lidar" value="$(var udp_profile_lidar)"/>
       <param name="lidar_mode" value="$(var lidar_mode)"/>
-      <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="metadata" value="$(var metadata)"/>
       <param name="sensor_frame" value="$(var sensor_frame)"/>
       <param name="lidar_frame" value="$(var lidar_frame)"/>
       <param name="imu_frame" value="$(var imu_frame)"/>
+      <param name="point_cloud_frame" value="$(var point_cloud_frame)"/>
       <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
       <param name="proc_mask" value="$(var proc_mask)"/>
@@ -87,9 +94,6 @@
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">
     <arg name="ouster_ns" value="$(var ouster_ns)"/>
     <arg name="rviz_config" value="$(var rviz_config)"/>
-    <arg name="sensor_frame" value="$(var sensor_frame)"/>
-    <arg name="lidar_frame" value="$(var lidar_frame)"/>
-    <arg name="imu_frame" value="$(var imu_frame)"/>
   </include>
 
 </launch>
diff --git a/ouster-ros/launch/sensor.independent.launch.xml b/ouster-ros/launch/sensor.independent.launch.xml
index bcb8683d..4859da5f 100644
--- a/ouster-ros/launch/sensor.independent.launch.xml
+++ b/ouster-ros/launch/sensor.independent.launch.xml
@@ -40,20 +40,28 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
+  <arg name="sensor_frame" default="os_sensor"
+    description="sets name of choice for the sensor_frame tf frame, value can not be empty"/>
+  <arg name="lidar_frame" default="os_lidar"
+    description="sets name of choice for the os_lidar tf frame, value can not be empty"/>
+  <arg name="imu_frame" default="os_imu"
+      description="sets name of choice for the os_imu tf frame, value can not be empty"/>
+  <arg name="point_cloud_frame" default=""
+    description="which frame to be used when publishing PointCloud2 or LaserScan messages.
+    Choose between the value of sensor_frame or lidar_frame, leaving this value empty
+    would set lidar_frame to be the frame used when publishing these messages."/>
+
+  <arg name="use_system_default_qos" default="false"
+    description="Use the default system QoS settings"/>
 
-  <arg name="use_system_default_qos" default="false" description="Use the default system QoS settings"/>
   <arg name="proc_mask" default="IMG|PCL|IMU|SCAN" description="
-    currently IMG is not supported and does not affect anything,
+    The IMG flag here is not supported and does not affect anything,
     to disable image topics you would need to omit the os_image node
     from the launch file"/>
 
   <arg name="scan_ring" default="0" description="
     use this parameter in conjunction with the SCAN flag
-    and choose a value the range [0, sensor_beams_count)"
-  />
+    and choose a value the range [0, sensor_beams_count)"/>
 
   <group>
     <push-ros-namespace namespace="$(var ouster_ns)"/>
@@ -69,12 +77,12 @@
       <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="metadata" value="$(var metadata)"/>
       <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
-      <param name="proc_mask" value="$(var proc_mask)"/>
     </node>
     <node pkg="ouster_ros" exec="os_cloud" name="os_cloud" output="screen">
       <param name="sensor_frame" value="$(var sensor_frame)"/>
       <param name="lidar_frame" value="$(var lidar_frame)"/>
       <param name="imu_frame" value="$(var imu_frame)"/>
+      <param name="point_cloud_frame" value="$(var point_cloud_frame)"/>
       <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
       <param name="proc_mask" value="$(var proc_mask)"/>
@@ -95,9 +103,6 @@
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">
     <arg name="ouster_ns" value="$(var ouster_ns)"/>
     <arg name="rviz_config" value="$(var rviz_config)"/>
-    <arg name="sensor_frame" value="$(var sensor_frame)"/>
-    <arg name="lidar_frame" value="$(var lidar_frame)"/>
-    <arg name="imu_frame" value="$(var imu_frame)"/>
   </include>
 
 </launch>
diff --git a/ouster-ros/launch/sensor_mtp.launch.xml b/ouster-ros/launch/sensor_mtp.launch.xml
index 0096a6cd..66c12986 100644
--- a/ouster-ros/launch/sensor_mtp.launch.xml
+++ b/ouster-ros/launch/sensor_mtp.launch.xml
@@ -48,12 +48,20 @@
   <arg name="rviz_config" default="$(find-pkg-share ouster_ros)/config/viz.rviz"
     description="optional rviz config file"/>
 
-  <arg name="sensor_frame" default="os_sensor" description="value can not be empty"/>
-  <arg name="lidar_frame" default="os_lidar" description="value can not be empty"/>
-  <arg name="imu_frame" default="os_imu" description="value can not be empty"/>
+  <arg name="sensor_frame" default="os_sensor"
+    description="sets name of choice for the sensor_frame tf frame, value can not be empty"/>
+  <arg name="lidar_frame" default="os_lidar"
+    description="sets name of choice for the os_lidar tf frame, value can not be empty"/>
+  <arg name="imu_frame" default="os_imu"
+      description="sets name of choice for the os_imu tf frame, value can not be empty"/>
+  <arg name="point_cloud_frame" default=""
+    description="which frame to be used when publishing PointCloud2 or LaserScan messages.
+    Choose between the value of sensor_frame or lidar_frame, leaving this value empty
+    would set lidar_frame to be the frame used when publishing these messages."/>
 
   <arg name="use_system_default_qos" default="false"
     description="Use the default system QoS settings"/>
+
   <arg name="proc_mask" default="IMG|PCL|IMU|SCAN"
     description="Use any combination of the 4 flags to enable or disable specific processors"/>
 
@@ -72,11 +80,11 @@
       <param name="imu_port" value="$(var imu_port)"/>
       <param name="udp_profile_lidar" value="$(var udp_profile_lidar)"/>
       <param name="lidar_mode" value="$(var lidar_mode)"/>
-      <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="metadata" value="$(var metadata)"/>
       <param name="sensor_frame" value="$(var sensor_frame)"/>
       <param name="lidar_frame" value="$(var lidar_frame)"/>
       <param name="imu_frame" value="$(var imu_frame)"/>
+      <param name="point_cloud_frame" value="$(var point_cloud_frame)"/>
       <param name="timestamp_mode" value="$(var timestamp_mode)"/>
       <param name="use_system_default_qos" value="$(var use_system_default_qos)"/>
       <param name="proc_mask" value="$(var proc_mask)"/>
diff --git a/ouster-ros/src/os_static_transforms_broadcaster.h b/ouster-ros/src/os_static_transforms_broadcaster.h
index 1b368b1e..c600c1f6 100644
--- a/ouster-ros/src/os_static_transforms_broadcaster.h
+++ b/ouster-ros/src/os_static_transforms_broadcaster.h
@@ -22,7 +22,7 @@ class OusterStaticTransformsBroadcaster {
         node->declare_parameter("sensor_frame", "os_sensor");
         node->declare_parameter("lidar_frame", "os_lidar");
         node->declare_parameter("imu_frame", "os_imu");
-        node->declare_parameter("point_cloud_frame", "os_lidar");
+        node->declare_parameter("point_cloud_frame", "");
     }
 
     void parse_parameters() {

From cd659c89738cb9132ef54527772449a1b1efb559 Mon Sep 17 00:00:00 2001
From: Ussama Naal <ussama.naal@ouster.io>
Date: Wed, 12 Jul 2023 15:17:32 -0700
Subject: [PATCH 2/2] Correct the name of the node to be activated for merged
 node

---
 ouster-ros/launch/sensor.composite.launch.xml | 4 ++--
 ouster-ros/launch/sensor_mtp.launch.xml       | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ouster-ros/launch/sensor.composite.launch.xml b/ouster-ros/launch/sensor.composite.launch.xml
index e66fd3d9..9e11b97f 100644
--- a/ouster-ros/launch/sensor.composite.launch.xml
+++ b/ouster-ros/launch/sensor.composite.launch.xml
@@ -86,9 +86,9 @@
 
   <!-- HACK: configure and activate the sensor node via a process execute since state
     transition is currently not availabe through launch.xml format -->
-  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_sensor configure"
+  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_driver configure"
     launch-prefix="bash -c 'sleep 0; $0 $@'" output="screen"/>
-  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_sensor activate"
+  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_driver activate"
     launch-prefix="bash -c 'sleep 1; $0 $@'" output="screen"/>
 
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">
diff --git a/ouster-ros/launch/sensor_mtp.launch.xml b/ouster-ros/launch/sensor_mtp.launch.xml
index 66c12986..d8539e50 100644
--- a/ouster-ros/launch/sensor_mtp.launch.xml
+++ b/ouster-ros/launch/sensor_mtp.launch.xml
@@ -94,9 +94,9 @@
 
   <!-- HACK: configure and activate the sensor node via a process execute since state
     transition is currently not availabe through launch.xml format -->
-  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_sensor configure"
+  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_driver configure"
     launch-prefix="bash -c 'sleep 0; $0 $@'" output="screen"/>
-  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_sensor activate"
+  <executable cmd="$(find-exec ros2) lifecycle set /$(var ouster_ns)/os_driver activate"
     launch-prefix="bash -c 'sleep 1; $0 $@'" output="screen"/>
 
   <include if="$(var viz)" file="$(find-pkg-share ouster_ros)/launch/rviz.launch.xml">