Skip to content

Conversation

@Noy-Zini
Copy link
Contributor

tracked on: [LRS-1287]

@Noy-Zini Noy-Zini requested a review from Nir-Az June 24, 2025 07:41
@Nir-Az Nir-Az requested a review from Copilot June 24, 2025 11:28
Copy link
Contributor

Copilot AI left a 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 pull request adds a new live unit test for basic color image quality by verifying that key pixel positions display the expected RGB values consistently over multiple frames.

  • Introduces a test that checks pixel color matches for three specific color points over 10 frames.
  • Logs detailed mismatch information and enforces a threshold of 80% frame success per color.

Comment on lines 59 to 61
test.finish()
pipeline.stop()
Copy link

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

Consider moving pipeline.stop() into a finally block to ensure the device is properly stopped even when an exception occurs.

Suggested change
test.finish()
pipeline.stop()
finally:
if pipeline:
pipeline.stop()
test.finish()

Copilot uses AI. Check for mistakes.
Comment on lines 59 to 61
test.finish()
pipeline.stop()
Copy link

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

Review the ordering of cleanup calls; invoking test.finish() before stopping the pipeline may prevent proper resource cleanup.

Suggested change
test.finish()
pipeline.stop()
pipeline.stop()
test.finish()

Copilot uses AI. Check for mistakes.
@Nir-Az Nir-Az requested a review from alexkunin-gh June 24, 2025 11:34
cfg = rs.config()
cfg.enable_stream(rs.stream.color, 1280, 720, rs.format.rgb8, 30)
pipeline_profile = pipeline.start(cfg)
time.sleep(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to sleep here, I prefer waiting for frame and then sleep 2 sec so the exp will stabilize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added waiting for frame before sleep 2.

log.d(f"Frame {i} - {color} at ({x},{y}): {pixel} ≠ {expected_rgb}")

# Check that each color passed in at least 80% of the frames
min_passes = int(NUM_FRAMES * 0.8)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets move to a constant the 80%

Copy link
Contributor 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 changed the title basic color test IQ basic color test Jun 24, 2025
@Nir-Az
Copy link
Collaborator

Nir-Az commented Jul 15, 2025

Please add a "donotrun" label so we can merge it

@Noy-Zini Noy-Zini force-pushed the Basic-color-image-quality-test branch from 51b417c to e258afd Compare August 13, 2025 11:15
@Nir-Az Nir-Az merged commit a24c405 into realsenseai:development Aug 13, 2025
25 of 26 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