rmw_time_t is redundant with rcutils_duration_value_t and doesn't need to be a struct

[rotu]

rmw_time_t is defined as:

typedef struct RMW_PUBLIC_TYPE rmw_time_t
{
  uint64_t sec;
  uint64_t nsec;
} rmw_time_t;

This differs from rcutils_duration_value_t which admits a value as just a int64_t holding nanoseconds. It probably makes sense to replace rmw_time_t with a type alias for rcutils_duration_value_t, and to make clear that it is a duration, not a time point.

[rotu]

It occurs to me that there is (until #214) no rmw concept of absolute time. It may make sense in the future to think about adding a rmw-implemented timestamp and comparator function so the RMW can use a different time concept, like vector clocks.

[iluetkeb]

Why do you think rmw_time_t is supposed to be a duration? Its uses of "uint64_t" seems to suggest it's a timestamp.

That means it would be redundant with rcutils_time_point_value_t, though.

[rotu]

Because every existing use of it (wait time, liveliness, lifespan, deadline) is a relative time and because https://github.com/ros2/rclcpp/blob/911291f8d32da8a9a70f69adc7bcf1e41477e017/rclcpp/include/rclcpp/duration.hpp#L111.

Both time_point and duration are int64_t. The distinction is pretty meaningless in C, but matters in C++ (where you can enforce timestamp + duration = timestamp)

[rotu]

BTW, I don’t think this issue is high priority. I’d like to see it eventually fixed.

[fujitatomoya]

related to https://github.com/ros2/rmw/issues/210

[emersonknapp]

I'm looking at working on this in parallel to #210

Right now, I think rmw_time_t is used in 11 packages of ros2.repos. Most of the use is in tests or demos, luckily most use is hidden behind language client facades (rclcpp::Duration, rclpy.duration.Duration).

It makes sense to make a pass through ros2.repos, removing use, but it seems aggressive to delete the rmw_time_t type immediately. Should we give the type a deprecation notice via RCUTILS_DEPRECATED_WITH_MSG for Galactic, waiting to remove the type in H-turtle?

[emersonknapp]

Noting discussion from middleware WG meeting - no immediate concern about calling this type deprecated in Galactic to be removed in H-turtle. Expectation would be to remove the use of the types from ros2.repos in time for Galactic so that there are not compiler warnings in the core distro

[fujitatomoya]

I think certain time window should be given for deprecation, the reasons are

• All non-Tier1 rmw implementation will get build break because of this change.
• Possibly implementation uses rmw_time_t internally.
• fix all of the related packages are kinda heavy.

[clalancette]

I think certain time window should be given for deprecation, the reasons are

Agreed. We actually talked about this in more detail over at https://github.com/ros2/rmw/pull/298#discussion_r579420814 , and the consensus there was to revisit this after Galactic.