ros2 / ros2/rclcpp

rclcpp::Time and rclcpp::Duration max values not near max int64_t

Open
#1,351 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backlog enhancement
Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 17h
Merged PRs (30d)
27

Description

Follow up Issue to track the conversation in https://github.com/ros2/rclcpp/pull/1347#discussion_r496920298

Basically this is to consider a new approach to the one agreed on #538. The max method for Time and Duration IMO should return the max value of the representation in nanoseconds.

Not sure if that was a constrain in the past, but the sum / substraction operators can handle properly overflow and underflow operations, so I don't see any reason to limit the max value returned by the max method.

If this is not a desired approach, an update to the header of the function will be required, as it stated \return the maximum representable value which is not the case.

Tagging @clalancette for awareness

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the rclcpp::Time and rclcpp::Duration max methods and read the conversation linked from pull request #1347, discussion 496920298, together with the earlier decision in issue #538. Determine whether max should return the representation's maximum nanosecond value; done means either that behavior is implemented or the function header accurately documents its intended limit.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
robotics
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.