-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unit tests to cover message's send and received timestamps during recording #1641
Add unit tests to cover message's send and received timestamps during recording #1641
Conversation
- CycloneDDS doesn't support 'received_timestamps' and settling up zeros in message_info for that field. - Fallback to the timestamp from node clock in case of the CycloneDDS. Signed-off-by: Michael Orlov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with green CI. Good eye 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for typo
Signed-off-by: Michael Orlov <[email protected]>
- Also fixed typo in 'rwm_has_send_timestamp_support' in test Signed-off-by: Michael Orlov <[email protected]>
- The fix will be done on the `rmw_cyclonedds` layer in the scope of the ros2/rmw_cyclonedds#491 Signed-off-by: Michael Orlov <[email protected]>
So with ros2/rmw_cyclonedds#491 now merged, this one isn't strictly required. Totally up to you if you want to add in the additional tests here, but I'm going to suggest that if we do that, we do not backport to Jazzy at this time. |
Pulls: #1641 |
https://github.com/Mergifyio backport jazzy |
✅ Backports have been created
|
… recording (#1641) * Workaround for zeros in a message's received timestamp with CycloneDDS - CycloneDDS doesn't support 'received_timestamps' and settling up zeros in message_info for that field. - Fallback to the timestamp from node clock in case of the CycloneDDS. Signed-off-by: Michael Orlov <[email protected]> * Fix typo in rmw_has_received_timestamp_support Signed-off-by: Michael Orlov <[email protected]> * Add one more check in tests that "recv_timestamp >= send_timestamp" - Also fixed typo in 'rwm_has_send_timestamp_support' in test Signed-off-by: Michael Orlov <[email protected]> * Revert workaround for `rmw_cyclonedds` in recorder.cpp - The fix will be done on the `rmw_cyclonedds` layer in the scope of the ros2/rmw_cyclonedds#491 Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> (cherry picked from commit b7d9b5b)
… recording (#1641) (#1673) * Workaround for zeros in a message's received timestamp with CycloneDDS - CycloneDDS doesn't support 'received_timestamps' and settling up zeros in message_info for that field. - Fallback to the timestamp from node clock in case of the CycloneDDS. Signed-off-by: Michael Orlov <[email protected]> * Fix typo in rmw_has_received_timestamp_support Signed-off-by: Michael Orlov <[email protected]> * Add one more check in tests that "recv_timestamp >= send_timestamp" - Also fixed typo in 'rwm_has_send_timestamp_support' in test Signed-off-by: Michael Orlov <[email protected]> * Revert workaround for `rmw_cyclonedds` in recorder.cpp - The fix will be done on the `rmw_cyclonedds` layer in the scope of the ros2/rmw_cyclonedds#491 Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> (cherry picked from commit b7d9b5b) Co-authored-by: Michael Orlov <[email protected]>
received_timestamps
and settling up zeros inmessage_info
for that field.