-
Notifications
You must be signed in to change notification settings - Fork 5k
Create timestamp synchronization test #14100
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
Conversation
| ir1_frame_ts = ir1_frame.get_frame_metadata(rs.frame_metadata_value.frame_timestamp) | ||
| ir2_frame_ts = ir2_frame.get_frame_metadata(rs.frame_metadata_value.frame_timestamp) | ||
|
|
||
| log.d(f"Depth TS: {depth_ts}, IR1 TS: {ir1_ts}, IR2 TS: {ir2_ts}") |
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.
Please add color stream
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.
And check with an acceptable tolerance, see the Jira ticket for bad and good numbers
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.
You know what, since the original bug was for IR + Depth that's OK to keep as is.
Do we expect the exactly same TS?
|
DO we know why the CI failed? |
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.
Pull Request Overview
This PR creates a new test to verify timestamp consistency between different frame types (depth, infrared, and color) from RealSense devices. The test validates both global timestamps and frame metadata timestamps to ensure proper synchronization.
- Adds a comprehensive timestamp consistency test for D400 series devices
- Validates synchronization between depth, infrared (channels 1 & 2), and color streams
- Includes tolerance checks for both global and frame-level timestamps
| ir2_frame = frames.get_infrared_frame(2) | ||
| color_frame = frames.get_color_frame() | ||
|
|
||
| if not (depth_frame and ir1_frame and ir2_frame): |
Copilot
AI
Jul 29, 2025
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.
The condition doesn't check for color_frame validity, but color_frame is used later in the code (lines 41, 44, 49, 53, 59, 62, 67). This could cause AttributeError if color_frame is None.
| if not (depth_frame and ir1_frame and ir2_frame): | |
| if not (depth_frame and ir1_frame and ir2_frame and color_frame): |
| test.check(depth_ts == ir1_ts) | ||
| test.check(depth_ts == ir2_ts) |
Copilot
AI
Jul 29, 2025
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.
Exact equality comparison for timestamps may be too strict and could lead to flaky tests due to minor timing variations. Consider using approximate equality with a small tolerance instead.
| test.check(depth_ts == ir1_ts) | |
| test.check(depth_ts == ir2_ts) | |
| test.check_approx_abs(depth_ts, ir1_ts, GLOBAL_TS_TOLERANCE) | |
| test.check_approx_abs(depth_ts, ir2_ts, GLOBAL_TS_TOLERANCE) |
|
|
||
| # Check global timestamps | ||
| test.check(depth_ts == ir1_ts) | ||
| test.check(depth_ts == ir2_ts) |
Copilot
AI
Jul 29, 2025
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.
Exact equality comparison for timestamps may be too strict and could lead to flaky tests due to minor timing variations. Consider using approximate equality with a small tolerance instead.
| test.check(depth_ts == ir2_ts) | |
| test.check_approx_abs(depth_ts, ir2_ts, GLOBAL_TS_TOLERANCE) |
| test.check(depth_frame_ts == ir1_frame_ts) | ||
| test.check(depth_frame_ts == ir2_frame_ts) |
Copilot
AI
Jul 29, 2025
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.
Exact equality comparison for frame timestamps may be too strict and could lead to flaky tests due to minor timing variations. Consider using approximate equality with a small tolerance instead.
| test.check(depth_frame_ts == ir1_frame_ts) | |
| test.check(depth_frame_ts == ir2_frame_ts) | |
| test.check_approx_abs(depth_frame_ts, ir1_frame_ts, FRAME_TS_TOLERANCE) | |
| test.check_approx_abs(depth_frame_ts, ir2_frame_ts, FRAME_TS_TOLERANCE) |
| test.check(depth_frame_ts == ir1_frame_ts) | ||
| test.check(depth_frame_ts == ir2_frame_ts) |
Copilot
AI
Jul 29, 2025
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.
Exact equality comparison for frame timestamps may be too strict and could lead to flaky tests due to minor timing variations. Consider using approximate equality with a small tolerance instead.
| test.check(depth_frame_ts == ir1_frame_ts) | |
| test.check(depth_frame_ts == ir2_frame_ts) | |
| test.check_approx_abs(depth_frame_ts, ir1_frame_ts, FRAME_TS_TOLERANCE) | |
| test.check_approx_abs(depth_frame_ts, ir2_frame_ts, FRAME_TS_TOLERANCE) |
| import pyrealsense2 as rs | ||
| from rspy import test, log | ||
|
|
||
| # This test is checking that timestamps of depth and infrared frames are consistent |
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.
please add about color
remibettan
left a comment
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.
few comments - impressive python!
remibettan
left a comment
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
|
|
||
| # Tolerance for gap between depth and color | ||
| GLOBAL_TS_TOLERANCE = 1 # in ms | ||
| FRAME_TS_TOLERANCE = 100 # in microseconds |
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.
Is that too strict? where did you get 100 us from?
|
@remibettan since I see the tests failed on CI I removed your approval. |
added sleep, enable global ts if needed
Tracked on: [LRS-1275]