From 7da5d95f076892844d621e9932f88971b46c92b7 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Mon, 27 Aug 2018 10:25:11 -0700
Subject: [PATCH] Positive forward, negative backward

---
 rcl/include/rcl/time.h     | 8 ++++----
 rcl/src/rcl/time.c         | 9 +++------
 rcl/test/rcl/test_time.cpp | 2 +-
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/rcl/include/rcl/time.h b/rcl/include/rcl/time.h
index fb540e858f..c5dea13d14 100644
--- a/rcl/include/rcl/time.h
+++ b/rcl/include/rcl/time.h
@@ -97,11 +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 magnitude of a jump forwards to be considered
-  /// exceeded, or zero to disable.
+  /// A positive duration indicating the minimum jump forwards to be considered exceeded, or zero
+  /// to disable.
   rcl_duration_t min_forward;
-  /// A positive duration indicating the minimum magnitude of a jump backwards to be considered
-  /// exceeded, or zero to disable.
+  /// A negative duration indicating the minimum 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
index 85547d7bbc..b320d92bb0 100644
--- a/rcl/src/rcl/time.c
+++ b/rcl/src/rcl/time.c
@@ -276,7 +276,6 @@ _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))
@@ -404,11 +403,11 @@ rcl_clock_add_jump_callback(
     return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(callback, RCL_RET_INVALID_ARGUMENT, clock->allocator);
   if (threshold.min_forward.nanoseconds < 0) {
-    RCL_SET_ERROR_MSG("forward jump theshold must be positive", clock->allocator);
+    RCL_SET_ERROR_MSG("forward jump theshold must be positive or zero", clock->allocator);
     return RCL_RET_INVALID_ARGUMENT;
   }
-  if (threshold.min_backward.nanoseconds < 0) {
-    RCL_SET_ERROR_MSG("backward jump theshold must be positive", clock->allocator);
+  if (threshold.min_backward.nanoseconds > 0) {
+    RCL_SET_ERROR_MSG("backward jump theshold must be negative or zero", clock->allocator);
     return RCL_RET_INVALID_ARGUMENT;
   }
 
@@ -432,8 +431,6 @@ 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
index ce814e619d..2a52919634 100644
--- 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();