Fix the keep_last warning when using system defaults. (#2082)

If the user creates SystemDefaultsQoS setting, they are
explicitly asking for SystemDefaults.  In that case, we should
*not* warn, and just let the underlying RMW choose what it
wants.  Implement that here by passing a boolean parameter
through that decides when we should print out the warning,
and then skip printing that warning when SystemDefaultsQoS
is created.

Signed-off-by: Chris Lalancette <[email protected]>

Signed-off-by: Chris Lalancette <[email protected]>

Author: clalancette

rclcpp/include/rclcpp/qos.hpp

@@ -80,7 +80,9 @@ struct RCLCPP_PUBLIC QoSInitialization
   size_t depth;
 
   /// Constructor which takes both a history policy and a depth (even if it would be unused).
-  QoSInitialization(rmw_qos_history_policy_t history_policy_arg, size_t depth_arg);
+  QoSInitialization(
+    rmw_qos_history_policy_t history_policy_arg, size_t depth_arg,
+    bool print_depth_warning = true);
 
   /// Create a QoSInitialization from an existing rmw_qos_profile_t, using its history and depth.
   static
@@ -97,7 +99,7 @@ struct RCLCPP_PUBLIC KeepAll : public rclcpp::QoSInitialization
 /// Use to initialize the QoS with the keep_last history setting and the given depth.
 struct RCLCPP_PUBLIC KeepLast : public rclcpp::QoSInitialization
 {
-  explicit KeepLast(size_t depth);
+  explicit KeepLast(size_t depth, bool print_depth_warning = true);
 };
 
 /// Encapsulation of Quality of Service settings.

rclcpp/src/rclcpp/qos.cpp

@@ -45,18 +45,29 @@ std::string qos_policy_name_from_kind(rmw_qos_policy_kind_t policy_kind)
   }
 }
 
-QoSInitialization::QoSInitialization(rmw_qos_history_policy_t history_policy_arg, size_t depth_arg)
+QoSInitialization::QoSInitialization(
+  rmw_qos_history_policy_t history_policy_arg, size_t depth_arg,
+  bool print_depth_warning)
 : history_policy(history_policy_arg), depth(depth_arg)
-{}
+{
+  if (history_policy == RMW_QOS_POLICY_HISTORY_KEEP_LAST && depth == 0 && print_depth_warning) {
+    RCLCPP_WARN_ONCE(
+      rclcpp::get_logger(
+        "rclcpp"),
+      "A zero depth with KEEP_LAST doesn't make sense; no data could be stored. "
+      "This will be interpreted as SYSTEM_DEFAULT");
+  }
+}
 
 QoSInitialization
 QoSInitialization::from_rmw(const rmw_qos_profile_t & rmw_qos)
 {
   switch (rmw_qos.history) {
     case RMW_QOS_POLICY_HISTORY_KEEP_ALL:
       return KeepAll();
-    case RMW_QOS_POLICY_HISTORY_KEEP_LAST:
     case RMW_QOS_POLICY_HISTORY_SYSTEM_DEFAULT:
+      return KeepLast(rmw_qos.depth, false);
+    case RMW_QOS_POLICY_HISTORY_KEEP_LAST:
     case RMW_QOS_POLICY_HISTORY_UNKNOWN:
     default:
       return KeepLast(rmw_qos.depth);
@@ -67,32 +78,16 @@ KeepAll::KeepAll()
 : QoSInitialization(RMW_QOS_POLICY_HISTORY_KEEP_ALL, 0)
 {}
 
-KeepLast::KeepLast(size_t depth)
-: QoSInitialization(RMW_QOS_POLICY_HISTORY_KEEP_LAST, depth)
+KeepLast::KeepLast(size_t depth, bool print_depth_warning)
+: QoSInitialization(RMW_QOS_POLICY_HISTORY_KEEP_LAST, depth, print_depth_warning)
 {
-  if (depth == 0) {
-    RCLCPP_WARN_ONCE(
-      rclcpp::get_logger(
-        "rclcpp"),
-      "A zero depth with KEEP_LAST doesn't make sense; no data could be stored."
-      "This will be interpreted as SYSTEM_DEFAULT");
-  }
 }
 
 QoS::QoS(
   const QoSInitialization & qos_initialization,
   const rmw_qos_profile_t & initial_profile)
 : rmw_qos_profile_(initial_profile)
 {
-  if (qos_initialization.history_policy == RMW_QOS_POLICY_HISTORY_KEEP_LAST &&
-    qos_initialization.depth == 0)
-  {
-    RCLCPP_WARN_ONCE(
-      rclcpp::get_logger(
-        "rclcpp"),
-      "A zero depth with KEEP_LAST doesn't make sense; no data could be stored."
-      "This will be interpreted as SYSTEM_DEFAULT");
-  }
   rmw_qos_profile_.history = qos_initialization.history_policy;
   rmw_qos_profile_.depth = qos_initialization.depth;
 }