Skip to content

Commit

Permalink
Create a utility function to limit rmw_time_t to 32-bit values (#37)
Browse files Browse the repository at this point in the history
* 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 <michael.jeronimo@openrobotics.org>

* Change name of function based on code review feedback

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>

* One more name change for the function

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>

* Fix uncrustify issues

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
  • Loading branch information
mjeronimo authored Nov 10, 2020
1 parent 8f4a1bf commit 86384ff
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 1 deletion.
9 changes: 8 additions & 1 deletion rmw_dds_common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions rmw_dds_common/include/rmw_dds_common/time_utils.hpp
Original file line number Diff line number Diff line change
@@ -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_
45 changes: 45 additions & 0 deletions rmw_dds_common/src/time_utils.cpp
Original file line number Diff line number Diff line change
@@ -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 <climits>

#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;
}
54 changes: 54 additions & 0 deletions rmw_dds_common/test/test_time_utils.cpp
Original file line number Diff line number Diff line change
@@ -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 <gtest/gtest.h>
#include <climits>

#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);
}

0 comments on commit 86384ff

Please sign in to comment.