Check for negative time in rclcpp::Time(int64_t nanoseconds, ...) constructor

[sharminramli]

rclcpp::Time currently allows a negative time when constructing with int64_t nanoseconds (but not with int32_t seconds even though both can possibly hold a negative value). For a more uniform interface and based off of #525 where time cannot be negative:

• Added the negative time check and throw from the rclcpp::Time(int32_t seconds, uint32_t nanoseconds, ...) constructor to the rclcpp::Time(int64_t nanoseconds, ...) constructor
• Added a test to check that it throws with negative time
• Removed the tests which allowed for negative time (the cases should already be covered by the other tests?)

Closes #2507.

[fujitatomoya]

i think this is right thing to do.

@clalancette @wjwwood what do you think?

[clalancette]

This looks reasonable to me with green CI.

Let's put this on the queue to merge once Rolling is unfrozen.

[ahcorde]

• Linux
• Linux-aarch64
• Windows

[ahcorde]

@clalancette can we backport this to jazzy ? or we should wait the frozen period?

[clalancette]

@clalancette can we backport this to jazzy ? or we should wait the frozen period?

We are open for backports now. My only question is whether we should actually backport it to Jazzy or not. We are not allowing new features into Jazzy, but we are allowing bugfixes. The other thing we want to avoid at this point is too much breakage to downstream packages.

What do you think @ahcorde ? Do you consider this a bugfix or a feature, and what do you think the downstream impact is going to be?

[fujitatomoya]

@clalancette @ahcorde IMO, this could possibly break the user application. so i would not backport this to released distros...