Skip to content

Conversation

@AviaAv
Copy link
Contributor

@AviaAv AviaAv commented Aug 14, 2025

Tracked on: [LRS-1299]

@AviaAv AviaAv added the ai-assist AI was used to develop this feature label Aug 14, 2025
@AviaAv AviaAv requested review from Nir-Az and Copilot August 14, 2025 12:32
Copy link

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 PR implements Auto HDR tests for CI by creating a comprehensive test suite for High Dynamic Range (HDR) functionality. The changes restructure existing HDR tests and add new performance and configuration tests to ensure HDR features work correctly across different scenarios.

  • Refactors existing HDR test code into a reusable helper module
  • Creates new basic HDR preset tests with manual and auto configurations
  • Adds performance testing to validate FPS under HDR operations
  • Implements comprehensive configuration testing across multiple resolutions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
unit-tests/live/hdr/test-hdr-preset.py Basic HDR sanity tests for manual and auto modes
unit-tests/live/hdr/test-hdr-performance.py Performance validation ensuring HDR maintains acceptable FPS
unit-tests/live/hdr/test-hdr-configurations.py Comprehensive testing across resolutions and HDR disable functionality
unit-tests/live/hdr/hdr_helper.py Shared helper module with HDR configurations and test utilities
unit-tests/live/d400/test-hdr-preset.py Removal of old HDR test implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.



def test_json_load(config, test_title):
global batch_size
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Using global variables can make code harder to test and maintain. Consider passing batch_size as a parameter or using a class-based approach to encapsulate state.

Suggested change
global batch_size

Copilot uses AI. Check for mistakes.
device, ctx = test.find_first_device_or_exit()
am = rs.rs400_advanced_mode(device)
sensor = device.first_depth_sensor()
batch_size = 0 # updated on test_json_load
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Global variable initialization at module level can lead to test interdependencies and state leakage between tests. Consider initializing this within test functions or using a test fixture.

Copilot uses AI. Check for mistakes.
test.check(measured_fps > ACCEPTABLE_FPS, f"Measured FPS {measured_fps:.2f}")


test_hdr_performance()
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Using global variables in test functions can cause race conditions and make tests non-deterministic. Consider using a class or passing state through function parameters.

Suggested change
test_hdr_performance()
class HDRPerformanceTest:
EXPECTED_FPS = 30
ACCEPTABLE_FPS = EXPECTED_FPS * 0.9
TIME_FOR_STEADY_STATE = 3
TIME_TO_COUNT_FRAMES = 5
def __init__(self, sensor):
self.sensor = sensor
self.count = 0
self.count_frames = False
def frame_counter_callback(self, frame):
if not self.count_frames:
return # Skip counting if not enabled
self.count += 1
log.d("Frame callback called, frame number:", frame.get_frame_number())
def run(self):
"""
Test HDR performance with various configurations
"""
for i, config in enumerate(HDR_CONFIGURATIONS):
config_type = "Auto" if "depth-ae" in json.dumps(config) else "Manual"
num_items = len(config["hdr-preset"]["items"])
test_name = f"Config {i + 1} ({config_type}, {num_items} items)"
test_json_load(config, test_name)
self.count = 0
depth_profile = next(p for p in self.sensor.get_stream_profiles() if p.stream_type() == rs.stream.depth)
self.sensor.open(depth_profile)
self.sensor.start(lambda frame: self.frame_counter_callback(frame))
time.sleep(self.TIME_FOR_STEADY_STATE)
self.count_frames = True # Start counting frames
time.sleep(self.TIME_TO_COUNT_FRAMES)
self.count_frames = False # Stop counting
self.sensor.stop()
self.sensor.close()
measured_fps = self.count / self.TIME_TO_COUNT_FRAMES
log.d(f"Test {test_name}: Counted frames = {self.count}, Measured FPS = {measured_fps:.2f}")
test.check(measured_fps > self.ACCEPTABLE_FPS, f"Measured FPS {measured_fps:.2f}")
hdr_test = HDRPerformanceTest(sensor)
hdr_test.run()

Copilot uses AI. Check for mistakes.

measured_fps = count / TIME_TO_COUNT_FRAMES
log.d(f"Test {test_name}: Counted frames = {count}, Measured FPS = {measured_fps:.2f}")
test.check(measured_fps > ACCEPTABLE_FPS, f"Measured FPS {measured_fps:.2f}")
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The error message should include the expected threshold for clarity. Consider: f"Measured FPS {measured_fps:.2f} is below acceptable threshold {ACCEPTABLE_FPS}"

Suggested change
test.check(measured_fps > ACCEPTABLE_FPS, f"Measured FPS {measured_fps:.2f}")
test.check(measured_fps > ACCEPTABLE_FPS, f"Measured FPS {measured_fps:.2f} is below acceptable threshold {ACCEPTABLE_FPS}")

Copilot uses AI. Check for mistakes.
perform_auto_hdr_test(hdr_config, test_title, resolution)


device, ctx = test.find_first_device_or_exit()
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Global device and context initialization in a helper module can cause issues when multiple tests run concurrently. Consider moving device initialization into individual test functions or using proper test fixtures.

Copilot uses AI. Check for mistakes.
@AviaAv AviaAv requested a review from Nir-Az August 20, 2025 06:55
@Nir-Az Nir-Az merged commit 762f152 into IntelRealSense:development Aug 20, 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

ai-assist AI was used to develop this feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants