Skip to content

Fix date_time_with_ms millisecond formatting#570

Open
ahuazuipiaoliang wants to merge 1 commit into
ros2:rollingfrom
ahuazuipiaoliang:fix-date-time-with-ms
Open

Fix date_time_with_ms millisecond formatting#570
ahuazuipiaoliang wants to merge 1 commit into
ros2:rollingfrom
ahuazuipiaoliang:fix-date-time-with-ms

Conversation

@ahuazuipiaoliang
Copy link
Copy Markdown

Summary

Fix {date_time_with_ms} formatting so the subsecond field is computed as milliseconds instead of taking the first three digits from an unpadded nanosecond string.

Details

The previous implementation converted the nanosecond remainder to a decimal string without zero-padding and then used the first three characters. For values with leading zeros, such as 085860000 ns, this produced .858 instead of .085.

This PR computes milliseconds directly with nanoseconds / 1000000 and formats the value with %03.

Testing

Added coverage for a timestamp whose nanosecond remainder has leading zeros.

Closes #569

Signed-off-by: Mark Jin <mark@pixmoving.net>
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

good eye! thanks for the PR.
this is real bug. any timestamp whose subsecond portion is under 100 ms produced wrong output, which is a substantial fraction of timestamps in practice.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

Pulls: #570
Gist: https://gist.githubusercontent.com/fujitatomoya/e35582465a12e4a61963c3387a965caf/raw/bea084ce649b28a197429b2f6d526ff131e4b133/ros2.repos
BUILD args: --packages-above-and-dependencies rcutils
TEST args: --packages-above rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19389

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

date_time_with_ms formats milliseconds incorrectly when nanoseconds have leading zeros

2 participants