Backwards duration passed as a positive number

ros2 · Aug 27, 2018 · 2c9bbc8 · 2c9bbc8
1 parent bbc55a5
commit 2c9bbc8
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 9 deletions.
diff --git a/rcl/include/rcl/time.h b/rcl/include/rcl/time.h
@@ -97,12 +97,11 @@ typedef struct rcl_jump_threshold_t
 {
   /// True to call callback when the clock type changes.
   bool on_clock_change;
-  /// A positive duration indicating the minimum jump forwards to be considered exceeded, or zero
-  /// to disable.
-  /// The struct is invalid if this value is negative.
+  /// A positive duration indicating the minimum magnitude of a jump forwards to be considered
+  /// exceeded, or zero to disable.
   rcl_duration_t min_forward;
-  /// The minimum jump backwards to be considered exceeded, or zero to disable.
-  /// The duration may be positive or negative; the absolute value will be used.
+  /// A positive duration indicating the minimum magnitude of a jump backwards to be considered
+  /// exceeded, or zero to disable.
   rcl_duration_t min_backward;
 } rcl_jump_threshold_t;
 

diff --git a/rcl/src/rcl/time.c b/rcl/src/rcl/time.c
@@ -276,6 +276,7 @@ _rcl_clock_call_callbacks(
     if (
       (is_clock_change && info->threshold.on_clock_change) ||
       (time_jump->delta.nanoseconds < 0 &&
+      // Note: min_backward was stored as a negative duration
       time_jump->delta.nanoseconds <= info->threshold.min_backward.nanoseconds) ||
       (time_jump->delta.nanoseconds > 0 &&
       time_jump->delta.nanoseconds >= info->threshold.min_forward.nanoseconds))
@@ -406,9 +407,9 @@ rcl_clock_add_jump_callback(
     RCL_SET_ERROR_MSG("forward jump theshold must be positive", clock->allocator);
     return RCL_RET_INVALID_ARGUMENT;
   }
-  if (threshold.min_backward.nanoseconds > 0) {
-    // store threshold with a negative value for convenience
-    threshold.min_backward.nanoseconds *= -1;
+  if (threshold.min_backward.nanoseconds < 0) {
+    RCL_SET_ERROR_MSG("backward jump theshold must be positive", clock->allocator);
+    return RCL_RET_INVALID_ARGUMENT;
   }
 
   // Callback/user_data pair must be unique
@@ -431,6 +432,8 @@ rcl_clock_add_jump_callback(
   clock->jump_callbacks = callbacks;
   clock->jump_callbacks[clock->num_jump_callbacks].callback = callback;
   clock->jump_callbacks[clock->num_jump_callbacks].threshold = threshold;
+  // store backwards jump threshold with a negative value for convenience
+  clock->jump_callbacks[clock->num_jump_callbacks].threshold.min_backward.nanoseconds *= -1;
   clock->jump_callbacks[clock->num_jump_callbacks].user_data = user_data;
   ++(clock->num_jump_callbacks);
   return RCL_RET_OK;

diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp
@@ -547,7 +547,7 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_backward_jump_callbacks)
   rcl_jump_threshold_t threshold;
   threshold.on_clock_change = false;
   threshold.min_forward.nanoseconds = 0;
-  threshold.min_backward.nanoseconds = -1;
+  threshold.min_backward.nanoseconds = 1;
   ASSERT_EQ(RCL_RET_OK,
     rcl_clock_add_jump_callback(ros_clock, threshold, clock_callback, &time_jump)) <<
     rcl_get_error_string_safe();