From b85155e79d7db2bd629c358bd6649f71b81bd4ff Mon Sep 17 00:00:00 2001 From: leoca Date: Sat, 12 Sep 2026 00:33:33 +0200 Subject: [PATCH 1/2] Don't truncate rmw_time_t QoS durations in the override defaults rclcpp::detail::rmw_duration_to_int64_t built an rclcpp::Duration from static_cast(rmw_duration.sec). RMW_DURATION_INFINITE, and the best-available deadline and liveliness lease sentinels, all carry 9223372036 seconds, so the cast wrapped and the helper returned 633437444854775807 ns instead of 9223372036854775807 ns. That value is what declare_qos_parameters() declares as the default for the deadline, lifespan and liveliness_lease_duration overrides, and it is fed straight back into the profile through apply_qos_override(). Enabling those overrides on a profile carrying an infinite duration therefore silently replaced it with a finite ~20 year duration, which changes QoS compatibility matching. Seconds values in [2^31, 2^32) wrap to a negative int32_t and produce a negative duration, which QoS::deadline() then rejects with "rmw_time_t cannot be negative". Use rclcpp::Duration::from_rmw_time(), which is the conversion the QoS getters already use and which saturates instead of wrapping. Signed-off-by: leoca --- .../include/rclcpp/detail/qos_parameters.hpp | 5 +- rclcpp/test/rclcpp/test_qos_parameters.cpp | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/rclcpp/include/rclcpp/detail/qos_parameters.hpp b/rclcpp/include/rclcpp/detail/qos_parameters.hpp index 80051d86d9..e31ce1d7c2 100644 --- a/rclcpp/include/rclcpp/detail/qos_parameters.hpp +++ b/rclcpp/include/rclcpp/detail/qos_parameters.hpp @@ -281,10 +281,7 @@ inline int64_t rmw_duration_to_int64_t(rmw_time_t rmw_duration) { - return ::rclcpp::Duration( - static_cast(rmw_duration.sec), - static_cast(rmw_duration.nsec) - ).nanoseconds(); + return ::rclcpp::Duration::from_rmw_time(rmw_duration).nanoseconds(); } /// \internal Throw an exception if `policy_value_stringified` is NULL. diff --git a/rclcpp/test/rclcpp/test_qos_parameters.cpp b/rclcpp/test/rclcpp/test_qos_parameters.cpp index e535a65142..cd4e57bd96 100644 --- a/rclcpp/test/rclcpp/test_qos_parameters.cpp +++ b/rclcpp/test/rclcpp/test_qos_parameters.cpp @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -19,7 +20,10 @@ #include "gmock/gmock.h" +#include "rmw/time.h" + #include "rclcpp/detail/qos_parameters.hpp" +#include "rclcpp/duration.hpp" #include "rclcpp/exceptions.hpp" #include "rclcpp/node.hpp" #include "rclcpp/node_options.hpp" @@ -196,6 +200,53 @@ TEST(TestQosParameters, declare_qos_subscription_parameters) { rclcpp::shutdown(); } +TEST(TestQosParameters, declare_infinite_durations) { + // RMW_DURATION_INFINITE, and the "best available" deadline and liveliness lease + // sentinels, all carry 9223372036 seconds, which does not fit in the int32_t + // seconds field of rclcpp::Duration(int32_t, uint32_t). + constexpr rmw_time_t infinite = RMW_DURATION_INFINITE; + const int64_t infinite_ns = rclcpp::Duration::from_rmw_time(infinite).nanoseconds(); + + rclcpp::init(0, nullptr); + auto node = std::make_shared("my_node", "/ns"); + + rclcpp::QoS qos{rclcpp::KeepLast{10}}; + qos.deadline(infinite); + qos.lifespan(infinite); + qos.liveliness_lease_duration(infinite); + + EXPECT_EQ( + infinite_ns, + rclcpp::detail::get_default_qos_param_value( + rclcpp::QosPolicyKind::Deadline, qos).get()); + EXPECT_EQ( + infinite_ns, + rclcpp::detail::get_default_qos_param_value( + rclcpp::QosPolicyKind::Lifespan, qos).get()); + EXPECT_EQ( + infinite_ns, + rclcpp::detail::get_default_qos_param_value( + rclcpp::QosPolicyKind::LivelinessLeaseDuration, qos).get()); + + // Without a parameter override, declaring the overrides must hand back the profile + // it was given. + rclcpp::QoS declared = rclcpp::detail::declare_qos_parameters( + { + rclcpp::QosPolicyKind::Deadline, rclcpp::QosPolicyKind::Lifespan, + rclcpp::QosPolicyKind::LivelinessLeaseDuration + }, + node, + "/my/fully/qualified/topic_name", + qos, + rclcpp::detail::PublisherQosParametersTraits{}); + + EXPECT_EQ(infinite_ns, declared.deadline().nanoseconds()); + EXPECT_EQ(infinite_ns, declared.lifespan().nanoseconds()); + EXPECT_EQ(infinite_ns, declared.liveliness_lease_duration().nanoseconds()); + + rclcpp::shutdown(); +} + TEST(TestQosParameters, declare_with_id) { rclcpp::init(0, nullptr); auto node = std::make_shared("my_node", "/ns"); From ec6a61d0dd787ca7b7970733b0bd8ee51db254ab Mon Sep 17 00:00:00 2001 From: leoca Date: Mon, 14 Sep 2026 03:14:59 +0200 Subject: [PATCH 2/2] Fix indentation of the policy list in declare_infinite_durations Signed-off-by: leoca --- rclcpp/test/rclcpp/test_qos_parameters.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rclcpp/test/rclcpp/test_qos_parameters.cpp b/rclcpp/test/rclcpp/test_qos_parameters.cpp index cd4e57bd96..a63b834f6b 100644 --- a/rclcpp/test/rclcpp/test_qos_parameters.cpp +++ b/rclcpp/test/rclcpp/test_qos_parameters.cpp @@ -231,10 +231,8 @@ TEST(TestQosParameters, declare_infinite_durations) { // Without a parameter override, declaring the overrides must hand back the profile // it was given. rclcpp::QoS declared = rclcpp::detail::declare_qos_parameters( - { - rclcpp::QosPolicyKind::Deadline, rclcpp::QosPolicyKind::Lifespan, - rclcpp::QosPolicyKind::LivelinessLeaseDuration - }, + {rclcpp::QosPolicyKind::Deadline, rclcpp::QosPolicyKind::Lifespan, + rclcpp::QosPolicyKind::LivelinessLeaseDuration}, node, "/my/fully/qualified/topic_name", qos,