enable dds devices on run-unit-tests #13684
Conversation
| import subprocess | ||
| run_time_stopwatch = Stopwatch() | ||
| run_time_threshold = 2 | ||
| run_time_threshold = 2 if 'D555' not in camera_name else 5 |
There was a problem hiding this comment.
Can we ask instead if the connection type is DDS?
There was a problem hiding this comment.
And add a comment explaining why?
| @@ -35,15 +35,15 @@ def usage(): | |||
| pyrs_dir = repo.find_pyrs_dir() | |||
| sys.path.insert( 1, pyrs_dir ) | |||
|
|
|||
| MAX_ENUMERATION_TIME = 10 # [sec] | |||
| MAX_ENUMERATION_TIME = 15 # [sec] | |||
There was a problem hiding this comment.
This change will increase the libci timing for all devices.
What happens if we keep it at 10?
There was a problem hiding this comment.
f it's only for the first connection we are OK but I think we use this const in tests too, please check
There was a problem hiding this comment.
If we keep it at 10, we will get "timed out waiting for a device connection" when we wait, so sometimes we might begin the test without the device, which can cause it to fail (find_first_device_or_exit for example).
Also, as far as I understand, it will only increase libci times once, when we call query() and wait for the hub initially. As for the other uses, it's there as a timeout for when waiting, so we only wait until they're recognized and not the whole 15 seconds
There was a problem hiding this comment.
@OhadMeir is this expected? more than 10 seconds to get D555 after HW reset?
There was a problem hiding this comment.
Let's make sure we understand why it takes more than 10 sec, but for now it's OK to proceed as is
There was a problem hiding this comment.
Checked with wireshark. Between last message from old participant ("stopping") to first appearance of new participant almost 11 seconds. Less than 2 seconds later handshake complete.
So 15 seconds is a good limit for now.
unit-tests/py/rspy/devices.py
Outdated
| try: | ||
| self._usb_location = _get_usb_location(self._physical_port) | ||
| if self._physical_port.startswith('realsense/'): |
There was a problem hiding this comment.
I see we already have dev , let's also ask if the connection type is dds?
There was a problem hiding this comment.
This way we fixed the error on D457 too, currently I see
-E- Failed to get usb location: invalid physical port '/dev/video-rs-depth-0'
On D457 the connection type is GMSL
unit-tests/py/rspy/devices.py
Outdated
| @@ -223,7 +232,7 @@ def query( monitor_changes=True, hub_reset=False, recycle_ports=True ): | |||
| # | |||
| # Get all devices, and store by serial-number | |||
| global _device_by_sn, _context, _port_to_sn | |||
| _context = rs.context( { 'dds': False } ) | |||
| _context = rs.context( { 'dds': { "enabled" : True } } ) | |||
There was a problem hiding this comment.
Today we ran DDS tests, maybe this comes externally?
Can you please verify how it's working without this change? as for other projects I prefer not to run DDS tests
There was a problem hiding this comment.
Without this change DDS devices are not found:
We can ignore DDS here if we get a flag in run_unit_tests, similar to what we do with --hub-reset, what do you think?
There was a problem hiding this comment.
About the DDS tests we ran - most of them were w/o a camera, the one that did worked on a USB camera (which was detected in this context), so I believe this is needed to detect the DDS cameras
There was a problem hiding this comment.
As discussed let's set this only if dds is in the context given
unit-tests/py/rspy/devices.py
Outdated
| @@ -536,6 +546,9 @@ def enable_only( serial_numbers, recycle = False, timeout = MAX_ENUMERATION_TIME | |||
| else: | |||
| log.d( 'no hub; ports left as-is' ) | |||
|
|
|||
| # doesn't matter what it did, enable_only should wait for the devices to be available again | |||
| _wait_for(serial_numbers, timeout=timeout) | |||
There was a problem hiding this comment.
Any impact on timing for non DDS devices after this addition?
There was a problem hiding this comment.
The idea was that when we call this function in run_unit_tests:
librealsense/unit-tests/run-unit-tests.py
Lines 597 to 598 in b7b0870
We will wait even if there's no reset, because we want the devices to be available anyway after the test.
However, I now noticed we now have a wait twice if 'recycle' is on, so I'll move it to the else block. For non-DDS devices there shouldn't be a change
| @@ -615,16 +628,19 @@ def hw_reset( serial_numbers, timeout = MAX_ENUMERATION_TIME ): | |||
| :param timeout: Maximum # of seconds to wait for the devices to come back online | |||
| :return: True if all devices have come back online before timeout | |||
| """ | |||
| # for usb and dds devices, we can wait until they're removed | |||
| removable_devs_sns = {sn for sn in serial_numbers if | |||
There was a problem hiding this comment.
it will not be a part of it, it has both is_dds off and its port is None
Tracked on: [LRS-1221]