Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change rcutils_time_point_value_t type from uint64_t to int64_t #208

Merged
merged 1 commit into from
Feb 1, 2018
Merged

change rcutils_time_point_value_t type from uint64_t to int64_t #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions rcl/include/rcl/timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ typedef struct rcl_timer_t
* was called, because that information is no longer accessible via the timer.
* The time since the last callback call is given in nanoseconds.
*/
typedef void (* rcl_timer_callback_t)(rcl_timer_t *, uint64_t);
typedef void (* rcl_timer_callback_t)(rcl_timer_t *, int64_t);

/// Return a zero initialized timer.
RCL_PUBLIC
Expand Down Expand Up @@ -86,7 +86,7 @@ rcl_get_zero_initialized_timer(void);
* If the callback is `NULL`, the caller client library is responsible for
* firing the timer callback.
* Else, it must be a function which returns void and takes two arguments,
* the first being a pointer to the associated timer, and the second a uint64_t
* the first being a pointer to the associated timer, and the second a int64_t
* which is the time since the previous call, or since the timer was created
* if it is the first call to the callback.
*
Expand All @@ -95,7 +95,7 @@ rcl_get_zero_initialized_timer(void);
* ```c
* #include <rcl/rcl.h>
*
* void my_timer_callback(rcl_timer_t * timer, uint64_t last_call_time)
* void my_timer_callback(rcl_timer_t * timer, int64_t last_call_time)
* {
* // Do timer work...
* // Optionally reconfigure, cancel, or reset the timer...
Expand Down Expand Up @@ -137,7 +137,7 @@ RCL_WARN_UNUSED
rcl_ret_t
rcl_timer_init(
rcl_timer_t * timer,
uint64_t period,
int64_t period,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the passed period be checked to ensure it is >= 0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so, I'm not sure what a negative period would indicate. I suppose it could be lumped in with zero, but my instinct would be to only allow zero as a special case and raise an error when the period is zero.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my instinct would be to only allow zero as a special case and raise an error when the period is zero.

Thanks, done in #295.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wjwwood,

A negative period and frequency are well defined derived units:
https://en.wikipedia.org/wiki/Negative_frequency

The problem with software timers is that we don't know in what direction the elctrons will flow ;)

const rcl_timer_callback_t callback,
rcl_allocator_t allocator);

Expand Down Expand Up @@ -284,13 +284,13 @@ rcl_timer_get_time_until_next_call(const rcl_timer_t * timer, int64_t * time_unt
/// Retrieve the time since the previous call to rcl_timer_call() occurred.
/**
* This function calculates the time since the last call and copies it into
* the given uint64_t variable.
* the given int64_t variable.
*
* Calling this function within a callback will not return the time since the
* previous call but instead the time since the current callback was called.
*
* The time_since_last_call argument must be a pointer to an already allocated
* uint64_t.
* int64_t.
*
* <hr>
* Attribute | Adherence
Expand All @@ -311,13 +311,13 @@ rcl_timer_get_time_until_next_call(const rcl_timer_t * timer, int64_t * time_unt
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_timer_get_time_since_last_call(const rcl_timer_t * timer, uint64_t * time_since_last_call);
rcl_timer_get_time_since_last_call(const rcl_timer_t * timer, int64_t * time_since_last_call);

/// Retrieve the period of the timer.
/**
* This function retrieves the period and copies it into the give variable.
*
* The period argument must be a pointer to an already allocated uint64_t.
* The period argument must be a pointer to an already allocated int64_t.
*
* <hr>
* Attribute | Adherence
Expand All @@ -329,7 +329,7 @@ rcl_timer_get_time_since_last_call(const rcl_timer_t * timer, uint64_t * time_si
* <i>[1] if `atomic_is_lock_free()` returns true for `atomic_int_least64_t`</i>
*
* \param[in] timer the handle to the timer which is being queried
* \param[out] period the uint64_t in which the period is stored
* \param[out] period the int64_t in which the period is stored
* \return `RCL_RET_OK` if the period was retrieved successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_TIMER_INVALID` if the timer is invalid, or
Expand All @@ -338,7 +338,7 @@ rcl_timer_get_time_since_last_call(const rcl_timer_t * timer, uint64_t * time_si
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period);
rcl_timer_get_period(const rcl_timer_t * timer, int64_t * period);

/// Exchange the period of the timer and return the previous period.
/**
Expand All @@ -347,7 +347,7 @@ rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period);
*
* Exchanging (changing) the period will not affect already waiting wait sets.
*
* The old_period argument must be a pointer to an already allocated uint64_t.
* The old_period argument must be a pointer to an already allocated int64_t.
*
* <hr>
* Attribute | Adherence
Expand All @@ -359,8 +359,8 @@ rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period);
* <i>[1] if `atomic_is_lock_free()` returns true for `atomic_int_least64_t`</i>
*
* \param[in] timer the handle to the timer which is being modified
* \param[out] new_period the uint64_t to exchange into the timer
* \param[out] old_period the uint64_t in which the previous period is stored
* \param[out] new_period the int64_t to exchange into the timer
* \param[out] old_period the int64_t in which the previous period is stored
* \return `RCL_RET_OK` if the period was retrieved successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_TIMER_INVALID` if the timer is invalid, or
Expand All @@ -369,7 +369,7 @@ rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period);
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_timer_exchange_period(const rcl_timer_t * timer, uint64_t new_period, uint64_t * old_period);
rcl_timer_exchange_period(const rcl_timer_t * timer, int64_t new_period, int64_t * old_period);

/// Return the current timer callback.
/**
Expand Down
10 changes: 5 additions & 5 deletions rcl/src/rcl/timer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ rcl_get_zero_initialized_timer()
rcl_ret_t
rcl_timer_init(
rcl_timer_t * timer,
uint64_t period,
int64_t period,
const rcl_timer_callback_t callback,
rcl_allocator_t allocator)
{
Expand Down Expand Up @@ -116,7 +116,7 @@ rcl_timer_call(rcl_timer_t * timer)
(rcl_timer_callback_t)rcl_atomic_load_uintptr_t(&timer->impl->callback);

if (typed_callback != NULL) {
uint64_t since_last_call = now_steady - previous_ns;
int64_t since_last_call = now_steady - previous_ns;
typed_callback(timer, since_last_call);
}
return RCL_RET_OK;
Expand Down Expand Up @@ -154,7 +154,7 @@ rcl_timer_get_time_until_next_call(const rcl_timer_t * timer, int64_t * time_unt
if (ret != RCL_RET_OK) {
return ret; // rcl error state should already be set.
}
uint64_t period = rcl_atomic_load_uint64_t(&timer->impl->period);
int64_t period = rcl_atomic_load_uint64_t(&timer->impl->period);
*time_until_next_call =
(rcl_atomic_load_uint64_t(&timer->impl->last_call_time) + period) - now;
return RCL_RET_OK;
Expand Down Expand Up @@ -182,7 +182,7 @@ rcl_timer_get_time_since_last_call(
}

rcl_ret_t
rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period)
rcl_timer_get_period(const rcl_timer_t * timer, int64_t * period)
{
RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = rcl_timer_get_allocator(timer);
Expand All @@ -195,7 +195,7 @@ rcl_timer_get_period(const rcl_timer_t * timer, uint64_t * period)
}

rcl_ret_t
rcl_timer_exchange_period(const rcl_timer_t * timer, uint64_t new_period, uint64_t * old_period)
rcl_timer_exchange_period(const rcl_timer_t * timer, int64_t new_period, int64_t * old_period)
{
RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = rcl_timer_get_allocator(timer);
Expand Down