Skip to content

Conversation

@Tamir91
Copy link
Contributor

@Tamir91 Tamir91 commented Dec 3, 2023

Tracked on [LRS-971]

@Tamir91 Tamir91 requested a review from Nir-Az December 3, 2023 09:28
def test_amp_factor(am_device, new_a_factor: float):
"""
This function set new A Factor value to advance mode device
:am_am_device: advance mode device
Copy link
Collaborator

Choose a reason for hiding this comment

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

am_am please rename

"""
This function set new A Factor value to advance mode device
:am_am_device: advance mode device
:a_factor: new A Factor value
Copy link
Collaborator

Choose a reason for hiding this comment

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

new_a_factor?

:am_am_device: advance mode device
:a_factor: new A Factor value
"""
factor = am_device.get_amp_factor()
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to previous_amp_factor

am_device.set_amp_factor(factor)

log.d('Checking A factor: ' + str(new_a_factor))
test.check(am_device.get_amp_factor().a_factor - new_a_factor < 0.01)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try using this API test.check_float_lists

factor.a_factor = new_a_factor
am_device.set_amp_factor(factor)

log.d('Checking A factor: ' + str(new_a_factor))
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 for this log

depth_profile_depth = next(p for p in depth_sensor.profiles if p.stream_type() == rs.stream.depth)
depth_profile_infrared = next(p for p in depth_sensor.profiles if p.stream_type() == rs.stream.infrared)

test.start('Check that Disparity modulation receive values:')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please wrap all the logic with test.start
If you don't get into the if you are not in the test?
And you do finish it in the end?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can wrap with with test.closure( 'verify set/get of Disparity modulation' ):
and remove the test.finish.
Like here:
https://github.com/IntelRealSense/librealsense/blob/c918402db93d6e74cade6bec1f5accf0387c4fff/unit-tests/dds/test-metadata.py#L127

if depth_sensor and advance_mode_device:

depth_profile_depth = next(p for p in depth_sensor.profiles if p.stream_type() == rs.stream.depth)
depth_profile_infrared = next(p for p in depth_sensor.profiles if p.stream_type() == rs.stream.infrared)
Copy link
Collaborator

Choose a reason for hiding this comment

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

WHat are you doing with lines 41 + 42?

test_amp_factor(advance_mode_device, 0.1)
test_amp_factor(advance_mode_device, 0.15)
test_amp_factor(advance_mode_device, 0.2)
test_amp_factor(advance_mode_device, 0.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

1/2 set is enough

@Tamir91 Tamir91 added the python label Dec 4, 2023
@Tamir91
Copy link
Contributor Author

Tamir91 commented Dec 4, 2023

Here is the commit with the changes.

@Tamir91 Tamir91 requested a review from Nir-Az December 4, 2023 09:52


device = test.find_first_device_or_exit()
depth_sensor = device.first_depth_sensor()
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Why do I need depth_sensor?
  2. Why do I need to close depth sensor if I didn't ask start streaming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I thought I would use it and leave it in the code. Removed


with test.closure('Verify set/get of Disparity modulation'):
if depth_sensor and advance_mode_device:
a_factor_values = [0.05, 0.0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's check 0.01 & 0.05
0 is a special case which I don't know if is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I found that 0 is a default value, so thought to return the default value at the end of the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked values out of scope [0, 0.2] and it gave me error.

@Tamir91 Tamir91 requested a review from Nir-Az December 4, 2023 13:54
Copy link
Collaborator

@Nir-Az Nir-Az left a comment

Choose a reason for hiding this comment

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

LGTM

@Nir-Az Nir-Az merged commit 56309b2 into realsenseai:development Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants