Skip to content
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

Enhance metadata test to verify sensor timestamp #12642

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

Nir-Az
Copy link
Collaborator

@Nir-Az Nir-Az commented Feb 8, 2024

We expect frame timestamp to be different than sensor timestamp.

Sensor timestamp should be around frame timestamp - (actual exposure / 2), for sanity we just check it is increasing and not equal to the frame timestamp

Tracked on [RSDEV-401]

@Nir-Az Nir-Az requested a review from OhadMeir February 8, 2024 08:19
@Nir-Az Nir-Az changed the base branch from master to development February 8, 2024 08:43
@Nir-Az Nir-Az closed this Feb 8, 2024
@Nir-Az Nir-Az reopened this Feb 8, 2024
@Nir-Az
Copy link
Collaborator Author

Nir-Az commented Feb 8, 2024

Excluded D457 as it is not supported

@@ -106,6 +129,19 @@ def is_value_keep_increasing(metadata_value, number_frames_to_test=50) -> bool:
test.check(is_value_keep_increasing(rs.frame_metadata_value.frame_timestamp))
test.finish()

# Test #3 Increasing frame timestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

sensor timestamp

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment on lines 83 to 84
def is_value_keep_increasing(metadata_value, number_frames_to_test=50) -> bool:
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update prev_metadata_value each iteration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wow, how did it work before??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -57,6 +59,27 @@ def append_testing_profiles(dev) -> None:
testing_profiles[p] = s


def is_metadata_values_different(metadata_type_1, metadata_type_2, number_frames_to_test=50) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

are_metadata_values_different
Also in the comment - Check that the given 2 metadata types values are different

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

def is_metadata_values_different(metadata_type_1, metadata_type_2, number_frames_to_test=50) -> bool:
"""
Check that the given 2 metadata types value is different, it is handy when we expect different timetags / counters and such
:param metadata_type_1: first valuse that we need to check
Copy link
Contributor

Choose a reason for hiding this comment

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

valuse --> value

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -8,7 +8,7 @@

#############################################################################################
# get metadata depth units value and make sure it's non zero and equal to the depth sensor matching option value
test.start("checking depth units on metadata")
test.start("checking sensor timestanp on metadata")
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo
Also looks like this did not need to change. Please check again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, will revert

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@Nir-Az Nir-Az merged commit 5f083eb into IntelRealSense:development Feb 8, 2024
17 checks passed
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.

2 participants