Introduce RMW_DURATION_INFINITE constant and API return value promise

[emersonknapp]

Resolves #210
Alternative to #255

Introduce a new constant, RMW_DURATION_INFINITE. Specify in get_topic_endpoint_info that it is expected to be returned to represent infinite durations. Add some helper functions that enable better usage of this new value.

This change is non-breaking, all RMW implementations will continue to work correctly as-is. However, there are two expectations that they must eventually implement:

1. If a user inputs RMW_DURATION_INFINITE for Deadline, Lifespan, or Liveliness Lease Duration QoS policy values, this should be translated into the implementation-specific infinity value.
2. When returning values in get_topic_endpoint_info, RMW impementations must intercept their own internall-defined "infinity" definitions and replace those values with RMW_DURATION_INFINITE, so that they can be understood correctly.

Today, I will not be changing the default qos_profile values to RMW_DURATION_INFINITY, as this would break existing implementations that do not know how to handle this value correctly. I would like to create a "value deprecation" so that this default value could be switched over in time for H-Turtle, but I'm not sure how this should be done.

Dependent (but not linked, can be merged after) PRs:

• Take and return new RMW_DURATION_INFINITE correctly rmw_fastrtps#515
• Take and return new RMW_DURATION_INFINITE correctly rmw_cyclonedds#288
• Take and return new RMW_DURATION_INFINITE correctly rmw_connext#491
• rosbag2 playback
• ros2topic info should print "Infinite" instead of some large number for readability
• Out of tree DDS implementations are recommended to follow suit. This feature will affect their ability to have rosbags played back correctly, and for the CLI to print "infinite" in the right places.

[emersonknapp]

@jacobperron @clalancette @mm318 @hidmic @fujitatomoya @Barry-Xu-2018

Hey all, as participants in the #210 discussion, I would like your feedback on this. See discussion on #298 for why I'm not addressing #215 here.

I've only performed one RMW implementation update, to show the pattern, without doing too much extra work. After going too far on other iterations of this feature, I wanted to wait until we have some consensus on the API before opening more update PRs.

[jacobperron]

If a user inputs RMW_DURATION_INFINITE for Deadline, Lifespan, or Liveliness Lease Duration QoS policy values, this should be interpreted in the same way that {0, 0} (RMW_QOS_DURATION_UNSPECIFIED) has historically been interpreted.

I think we can improve the documentation around this point. The current documentation implies that a zero value means the policies are ignored (ie. not enforced), but digging through the various implementations it looks like a zero value causes the RMW's default value to be used instead, which happens to be "infinity" in most cases. The current documentation seems misleading, since really the policies are being enforced (albeit with very large values). Also, it isn't clear to me if the RMW default value might be changed via a vendor-specific configuration (then I think all bets are off).

Having an explicit INFINITE value in rmw makes sense to me. But I'm not sure if the behavior should be the same as a "default" value, considering the "default" value might be configured with a vendor config.

This discussion relates to another one we're having about QoS compatibility, see ros2/rmw_dds_common#45 (comment)

[emersonknapp]

@jacobperron @hidmic @fujitatomoya Thanks for the feedback - I've pushed a new version that I hope addresses all concerns:

• Move time declarations and utilities into time.h, time.c, test_time.cpp
• Explicitly state the differences between RMW_DURATION_UNSPECIFIED and RMW_DURATION_INFINITE
• Add bounds checking to time utilities (negative durations, overflows)

[fujitatomoya]

lgtm

[hidmic]

LGTM pending green CI

[emersonknapp]

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

• Linux
• Linux-aarch64
• macOS
• Windows

[jacobperron]

LGTM

[emersonknapp]

@jacobperron looks all good except the rcl_action tests on Windows - it looks like those same tests failed in last night's nightly https://ci.ros2.org/view/nightly/job/nightly_win_rel/1863/#showFailuresLink so I'm assuming it's a preexisting issue - should we block on merging?

[jacobperron]

@emersonknapp Looks like this is the related issue: ros2/rmw_connext#472

I think it's okay to not block on the rcl_action graph test failures.

[emersonknapp]

Sounds good to me! Note: I don't have merge access here so a maintainer will need to merge

[emersonknapp]

@jacobperron @hidmic friendly ping: can you merge this or direct it to the person who can?

[hidmic]

Thanks for the bump @emersonknapp. Merging.