From 86384ff9b78c587a3097b09680bb35e28646f7aa Mon Sep 17 00:00:00 2001 From: Michael Jeronimo Date: Tue, 10 Nov 2020 11:05:08 -0800 Subject: [PATCH] Create a utility function to limit rmw_time_t to 32-bit values (#37) * Create a utility function to limit rmw_time_t to 32-bit values The RMW layer maintains the rmw_time_t's seconds and nanoseconds values as 64-bit unsigned integers. However, the DDS standard specifies the Time_t and Duration_t types with "long sec" and "unsigned long nanosec", and the IDL to C++11 mapping states that the C++ types for long and unsigned long are mapped to int32_t and uint32_t, respectively. This difference can result in the seconds (and/or nanoseconds) value being truncated and incorrect, causing other downstream failures. This function can be used in different RMW adaptation layers that interface to the DDS providers when converting from rmw_time_to to the DDS Duration_t (or Time_t), such as rmw_time_to_fast_rtps. Signed-off-by: Michael Jeronimo * Change name of function based on code review feedback Signed-off-by: Michael Jeronimo * One more name change for the function Signed-off-by: Michael Jeronimo * Fix uncrustify issues Signed-off-by: Michael Jeronimo --- rmw_dds_common/CMakeLists.txt | 9 +++- .../include/rmw_dds_common/time_utils.hpp | 48 +++++++++++++++++ rmw_dds_common/src/time_utils.cpp | 45 ++++++++++++++++ rmw_dds_common/test/test_time_utils.cpp | 54 +++++++++++++++++++ 4 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 rmw_dds_common/include/rmw_dds_common/time_utils.hpp create mode 100644 rmw_dds_common/src/time_utils.cpp create mode 100644 rmw_dds_common/test/test_time_utils.cpp diff --git a/rmw_dds_common/CMakeLists.txt b/rmw_dds_common/CMakeLists.txt index 72c0cbc..375a84d 100644 --- a/rmw_dds_common/CMakeLists.txt +++ b/rmw_dds_common/CMakeLists.txt @@ -32,7 +32,9 @@ rosidl_generate_interfaces( add_library(${PROJECT_NAME}_library SHARED src/gid_utils.cpp - src/graph_cache.cpp) + src/graph_cache.cpp + src/time_utils.cpp) + set_target_properties(${PROJECT_NAME}_library PROPERTIES OUTPUT_NAME ${PROJECT_NAME}) ament_target_dependencies(${PROJECT_NAME}_library @@ -90,6 +92,11 @@ if(BUILD_TESTING) target_link_libraries(test_gid_utils ${PROJECT_NAME}_library) endif() + ament_add_gmock(test_time_utils test/test_time_utils.cpp) + if(TARGET test_time_utils) + target_link_libraries(test_time_utils ${PROJECT_NAME}_library) + endif() + add_performance_test(benchmark_graph_cache test/benchmark/benchmark_graph_cache.cpp) if(TARGET benchmark_graph_cache) target_link_libraries(benchmark_graph_cache ${PROJECT_NAME}_library) diff --git a/rmw_dds_common/include/rmw_dds_common/time_utils.hpp b/rmw_dds_common/include/rmw_dds_common/time_utils.hpp new file mode 100644 index 0000000..ef6329c --- /dev/null +++ b/rmw_dds_common/include/rmw_dds_common/time_utils.hpp @@ -0,0 +1,48 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RMW_DDS_COMMON__TIME_UTILS_HPP_ +#define RMW_DDS_COMMON__TIME_UTILS_HPP_ + +#include "rmw/types.h" + +#include "rmw_dds_common/visibility_control.h" + +namespace rmw_dds_common +{ + +/// Check if an rmw_time_t's 64-bit seconds/nanoseconds values exceed +/// 32-bit limits and adjust accordingly +/* + * The DDS standard specifies the Time_t and Duration_t types with + * "long sec" and "unsigned long nanosec", and the IDL to C++11 mapping + * states that the C++ types for long and unsigned long are mapped + * to int32_t and uint32_t, respectively. So, the 64-bit seconds and + * nanoseconds values will be truncated to 32-bits when converted + * from rmw_time_t to either a DDS Duration_t or Time_t. This function + * limits the seconds value to the signed 32-bit maximum and attempts + * to move the excess seconds to the nanoseconds field. If the resulting + * nanoseconds value is too large for unsigned 32-bits it saturates + * and issues a warning. + * + * \param[in] time to convert + * \return converted time value + */ +RMW_DDS_COMMON_PUBLIC +rmw_time_t +clamp_rmw_time_to_dds_time(const rmw_time_t & time); + +} // namespace rmw_dds_common + +#endif // RMW_DDS_COMMON__TIME_UTILS_HPP_ diff --git a/rmw_dds_common/src/time_utils.cpp b/rmw_dds_common/src/time_utils.cpp new file mode 100644 index 0000000..51ade3f --- /dev/null +++ b/rmw_dds_common/src/time_utils.cpp @@ -0,0 +1,45 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rmw/types.h" +#include "rcutils/logging_macros.h" + +#include "rmw_dds_common/time_utils.hpp" + +rmw_time_t +rmw_dds_common::clamp_rmw_time_to_dds_time(const rmw_time_t & time) +{ + rmw_time_t t = time; + + // Let's try to keep the seconds value representable with 32-bits + // by moving the excess seconds over to the nanoseconds field. + if (t.sec > INT_MAX) { + uint64_t diff = t.sec - INT_MAX; + t.sec -= diff; + t.nsec += diff * (1000LL * 1000LL * 1000LL); + } + + // In case the nanoseconds value is too large for a 32-bit unsigned, + // we can at least saturate and emit a warning + if (t.nsec > UINT_MAX) { + RCUTILS_LOG_WARN_NAMED( + "rmw_dds_common", + "nanoseconds value too large for 32-bits, saturated at UINT_MAX"); + t.nsec = UINT_MAX; + } + + return t; +} diff --git a/rmw_dds_common/test/test_time_utils.cpp b/rmw_dds_common/test/test_time_utils.cpp new file mode 100644 index 0000000..3619e09 --- /dev/null +++ b/rmw_dds_common/test/test_time_utils.cpp @@ -0,0 +1,54 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include "rmw_dds_common/time_utils.hpp" + +static bool +operator==(rmw_time_t t1, rmw_time_t t2) +{ + return t1.sec == t2.sec && t1.nsec == t2.nsec; +} + +TEST(test_time_utils, test_unmodified_zeros) +{ + const rmw_time_t zeros {0, 0}; + auto t1 = rmw_dds_common::clamp_rmw_time_to_dds_time(zeros); + EXPECT_TRUE(t1 == zeros); +} + +TEST(test_time_utils, test_unmodified_max) +{ + const rmw_time_t max_dds_time {0x7FFFFFFF, 0xFFFFFFFF}; + auto t2 = rmw_dds_common::clamp_rmw_time_to_dds_time(max_dds_time); + EXPECT_TRUE(t2 == max_dds_time); +} + +TEST(test_time_utils, test_seconds_overflow) +{ + const rmw_time_t slightly_too_large {2147483651, 0}; + auto t3 = rmw_dds_common::clamp_rmw_time_to_dds_time(slightly_too_large); + const rmw_time_t result3 {0x7FFFFFFF, 4000000000}; + EXPECT_TRUE(t3 == result3); +} + +TEST(test_time_utils, test_saturation) +{ + const rmw_time_t max_64 {LLONG_MAX, ULLONG_MAX}; + const rmw_time_t max_dds_time {0x7FFFFFFF, 0xFFFFFFFF}; + auto t4 = rmw_dds_common::clamp_rmw_time_to_dds_time(max_64); + EXPECT_TRUE(t4 == max_dds_time); +}