hdr test converted to python and stabilized #12670

AviaAv · 2024-02-15T11:40:18Z

Tracked on: [LRS-1000]

unit-tests/live/d400/test-hdr.py

Nir-Az · 2024-02-15T12:18:46Z

unit-tests/live/d400/test-hdr.py

+    ctx = rs.context()
+    device = test.find_first_device_or_exit()
+    if ctx is not None:
+        devices_list = ctx.query_devices()


The original test used to perform a HW reset between each test, using restart_first_device_and_return_depth_sensor
This was removed, is this not required for this test suite?
@remibettan ?

It would be better to perform HW reset between the tests, if they are changing some parameters, as the exposure for ID sequence 1/2.
Not needed for other tests.

unit-tests/live/d400/test-hdr.py

Nir-Az · 2024-02-15T12:24:42Z

unit-tests/live/d400/test-hdr.py

+            min_counter_set = False
+
+        if not min_counter_set:
+            min_counter = int(depth_counter)


is this casting needed in python?

the original file had a static cast there, so i thought to do it like that.
but I'll just remove it, it's not really needed

Still exist, let's please handle and answer all comments and than re-ask for a review from @remibettan

unit-tests/live/d400/test-hdr.py

Nir-Az · 2024-02-15T12:35:23Z

unit-tests/live/d400/test-hdr.py

+                test.check(depth_sensor.get_option(rs.option.hdr_enabled) == 1)
+
+                sequence_id = -1
+                iterations_for_preparation = 30


Can you explain the increase from 6 to 30?

interesting...?

unit-tests/live/d400/test-hdr.py

Nir-Az · 2024-02-15T12:41:47Z

unit-tests/live/d400/test-hdr.py

+
+                # emitter on/off works with PWM (pulse with modulation) in the hardware
+                # this takes some time to configure it
+                iterations_for_preparation = 30


Why 10 --> 30?

unit-tests/live/d400/test-hdr.py

Nir-Az · 2024-02-29T15:28:31Z

Please try to fix this conflict:

Oh this file was deleted, weird
Please rebase and make sure the file is deleted

remibettan · 2024-03-06T07:38:07Z

unit-tests/live/d400/test-hdr.py

+    device = test.find_first_device_or_exit()
+    depth_sensor = device.first_depth_sensor()
+
+    if depth_sensor and depth_sensor.supports(rs.option.hdr_enabled):


The test will not fail if this if statement is not filled, but the test itself will not be checked - please add else statement with failure of the test, with relevant message.
This comment is relevant for all below tests.

remibettan · 2024-03-06T07:39:20Z

unit-tests/live/d400/test-hdr.py

+        depth_sensor.set_option(rs.option.sequence_id, 1)
+        test.check(depth_sensor.get_option(rs.option.sequence_id) == 1)
+        exp = depth_sensor.get_option(rs.option.exposure)
+        test.check(depth_sensor.get_option(rs.option.exposure) == exposure_range.default - 1000)  # w/a


Please add some comment to explain this w/a

remibettan · 2024-03-06T07:40:06Z

unit-tests/live/d400/test-hdr.py

+        depth_sensor.set_option(rs.option.hdr_enabled, 1)
+        test.check(depth_sensor.get_option(rs.option.hdr_enabled) == 1)
+
+        depth_sensor.set_option(rs.option.sequence_id, 1)


Please add some comment to explain that seq id 1 should be with default values

remibettan · 2024-03-06T07:40:20Z

unit-tests/live/d400/test-hdr.py

+        test.check(depth_sensor.get_option(rs.option.exposure) == exposure_range.default - 1000)  # w/a
+        test.check(depth_sensor.get_option(rs.option.gain) == gain_range.default)
+
+        depth_sensor.set_option(rs.option.sequence_id, 2)


Please add some comment to explain that seq id 2 should be with min values

remibettan · 2024-03-06T07:41:18Z

unit-tests/live/d400/test-hdr.py

+        test.check(depth_sensor.get_option(rs.option.hdr_enabled) == 0)
+
+        device.hardware_reset()
+        time.sleep(5)


isn't it long? 5 seconds.

I see these lines are between each test, please make a function for it

remibettan · 2024-03-06T07:50:02Z

unit-tests/live/d400/test-hdr.py

+"""
+helper method
+checks that the frames resulting from the hdr_merge processing block are generated
+from frames streamed within this method (and not using old frames) - see RSDSO-17341.


I would change "within this method" to "within this streaming session" - only if you agree

remibettan · 2024-03-06T07:51:56Z

unit-tests/live/d400/test-hdr.py

+5           3 (no frame to merge with 5 as 6 didn't arrive yet)
+6           5 (merged frames 5+6)
+... (more frames, until a frame counter reset)
+1           1 (hdr # should like the value we reset to)


Did you mean "hdr # should be around the value we reset to"?

not around, should be that value exactly... I'll fix that typo

AviaAv · 2024-03-06T07:52:21Z

unit-tests/live/d400/test-hdr.py

+        pipe.stop()
+        depth_sensor.set_option(rs.option.hdr_enabled, 0)  # disable hdr before next tests
+
+


Yes it's on purpose, by your comment I figured it's not needed here?

remibettan · 2024-03-06T07:54:09Z

unit-tests/live/d400/test-hdr.py

+        # get depth frame data
+        depth_frame = data.get_depth_frame()
+        if not test.check(depth_frame.supports_frame_metadata(rs.frame_metadata_value.sequence_id)):
+            return


Could you add here some log reading the fact that this metadata could not be read?
Do you think it should fail the test?

on a second look... I dont think we need sequence id here, right? just the frame counter, what if we just remove it and remove also test.check(merged_frame.supports_frame_metadata(rs.frame_metadata_value.sequence_id)) ?

remibettan · 2024-03-06T08:00:58Z

unit-tests/live/d400/test-hdr.py

+    frame_counter = frame.get_frame_metadata(rs.frame_metadata_value.frame_counter)
+    frame_seq_id = frame.get_frame_metadata(rs.frame_metadata_value.sequence_id)
+    if frame_counter != prev_frame_counter + 1:  # can only compare sequential frames
+        sequence_id = frame_seq_id


Please change this param name from sequence_id to expected_sequence_id.

remibettan · 2024-03-06T08:12:25Z

unit-tests/live/d400/test-hdr.py

+            depth_sensor.set_option(rs.option.emitter_on_off, 1)
+            test.check(depth_sensor.get_option(rs.option.emitter_on_off) == 0)
+
+        # Control's presence verified in the beginning of the block


I do not understand this comment - could you rephrase it, please?

remibettan

Few comments - but really great work!

Nir-Az · 2024-03-07T12:05:03Z

Still conflict that needs to be resolved

remibettan

Great task!

AviaAv requested a review from Nir-Az February 15, 2024 11:40

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

Nir-Az reviewed Feb 15, 2024

View reviewed changes

unit-tests/live/d400/test-hdr.py Outdated Show resolved Hide resolved

AviaAv force-pushed the hdr-py-test branch 2 times, most recently from 750d293 to 94ce367 Compare February 21, 2024 15:14

AviaAv force-pushed the hdr-py-test branch 2 times, most recently from c3a48e2 to 74e36a8 Compare February 29, 2024 12:52

Nir-Az closed this Feb 29, 2024

Nir-Az reopened this Feb 29, 2024

Nir-Az requested a review from remibettan March 4, 2024 06:48

remibettan reviewed Mar 6, 2024

View reviewed changes

AviaAv commented Mar 6, 2024

View reviewed changes

remibettan reviewed Mar 6, 2024

View reviewed changes

AviaAv force-pushed the hdr-py-test branch from 2926a72 to 737090f Compare March 6, 2024 15:23

remibettan self-requested a review March 7, 2024 09:24

AviaAv added 6 commits March 7, 2024 14:24

converted test-hdr to python

0df43ce

stablized test

80f2888

now supports D457 + more changes

3f9eda5

restabilized test

e3a22f1

removed int() cast

97c17c6

hw resets removed, split to 2 files - sanity+long

b18ea82

AviaAv force-pushed the hdr-py-test branch from 737090f to b18ea82 Compare March 7, 2024 12:25

remibettan approved these changes Mar 7, 2024

View reviewed changes

Nir-Az merged commit de220a4 into IntelRealSense:development Mar 7, 2024
16 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hdr test converted to python and stabilized #12670

hdr test converted to python and stabilized #12670

AviaAv commented Feb 15, 2024

Nir-Az Feb 15, 2024

remibettan Feb 19, 2024

Nir-Az Feb 15, 2024

AviaAv Feb 15, 2024

Nir-Az Mar 4, 2024

Nir-Az Feb 15, 2024

remibettan Feb 19, 2024

Nir-Az Feb 15, 2024

Nir-Az commented Feb 29, 2024 •

edited

Loading

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

AviaAv Mar 6, 2024

AviaAv Mar 6, 2024

remibettan Mar 6, 2024 •

edited

Loading

AviaAv Mar 6, 2024

remibettan Mar 6, 2024

remibettan Mar 6, 2024

remibettan left a comment

Nir-Az commented Mar 7, 2024

remibettan left a comment

		pipe.stop()
		depth_sensor.set_option(rs.option.hdr_enabled, 0) # disable hdr before next tests

hdr test converted to python and stabilized #12670

hdr test converted to python and stabilized #12670

Conversation

AviaAv commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nir-Az commented Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remibettan Mar 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remibettan left a comment

Choose a reason for hiding this comment

Nir-Az commented Mar 7, 2024

remibettan left a comment

Choose a reason for hiding this comment

Nir-Az commented Feb 29, 2024 •

edited

Loading

remibettan Mar 6, 2024 •

edited

Loading