fix(timestamps): stamp message info with the wall clock, not the monotonic clock - #44
Closed
benaliabderrahmane wants to merge 1 commit into
Closed
fix(timestamps): stamp message info with the wall clock, not the monotonic clock#44benaliabderrahmane wants to merge 1 commit into
benaliabderrahmane wants to merge 1 commit into
Conversation
…tonic clock rmw_message_info_t/rmw_service_info_t timestamps are ns since the Unix epoch, but every stamp came from steady_clock — ns since boot. Since Jazzy, rosbag2 no longer stamps bags with the recorder node's clock and writes the middleware's send/receive timestamps verbatim (ros2/rosbag2#1531), so every recorded message landed on 1 January 1970 plus the writer's uptime. The publish, take, wait-drain and service request/response paths now stamp with system_clock, matching rmw_cyclonedds (dds_time()/system_clock). rmw_wait keeps steady_clock for its poll deadlines. Wire layout is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Bags recorded with this RMW under Jazzy show every message at 1 January 1970.
In Jazzy, rosbag2 stopped stamping messages with the recorder node's own clock and writes the send/receive timestamps as reported by the middleware (ros2/rosbag2#1531) — i.e.
rmw_message_info_t::received_timestamp/source_timestampgo into the bag verbatim.Those fields are defined as nanoseconds since the Unix epoch, but every stamp in this RMW came from
std::chrono::steady_clock(CLOCK_MONOTONIC— ns since boot). A recorder started 22 minutes after boot therefore wrote1312664618690ns ⇒1970-01-01T00:21:52Z.rmw_cycloneddsuses the wall clock for both fields (dds_time()→CLOCK_REALTIMEforsource_timestamp,std::chrono::system_clockforreceived_timestamp), which is why bags recorded over Cyclone are fine.Fix
Stamp message-visible timestamps with
system_clock:rmw_publish,rmw_publish_serialized_messagesource_timestamp_nsrmw_subscriptiondrainreceived_timestamp_nsrmw_waitdrain (the executor/rosbag2 path)received_timestamp_nsrmw_send_request/rmw_take_request,rmw_send_response/rmw_take_responsermw_waitkeepssteady_clockfor its poll deadlines — those must stay monotonic, so it now has both helpers. The remainingsteady_clockuses (socket/shm name uniquifiers) are unrelated to message timestamps and untouched.Wire layout is unchanged:
WireHeader::source_timestamp_nsis still anint64_tat offset 24, so the existingstatic_assertand cross-process compatibility hold. TRANSIENT_LOCAL late-joiner replay re-sends the cached original header, so replayed samples keep their true publish time.Tests
Three new tests, each written to fail first and bracketing the timestamps between two wall-clock reads:
PubSubTest.TakeWithInfoTimestampsAreUnixEpochRmwUdsNodeTest.WaitDrainStampsReceivedTimestampWithWallClock(covers thermw_waitdrain, which is the path rosbag2 actually takes)ServiceClientTest.RequestResponseTimestampsAreUnixEpochBefore the fix all three failed with e.g.
actual: 1312664618690 vs 1785743133835997198. After: full suite green — 127 tests, 0 failures.Notes
The wall clock can step (NTP), so
source_timestampmay occasionally exceedreceived_timestampor bag stamps may be non-monotonic. That is inherent to the epoch-based contract and matches Cyclone/Fast DDS; no timeout or ordering logic in this RMW reads these fields, so nothing can hang on a backwards step.