multi devices tests added#14778
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new live multi-device unit tests under unit-tests/live/multi_devices to validate basic multi-device enumeration and simultaneous multi-stream (depth/color/IR) streaming behavior across connected D400 devices (RSDEV-6160).
Changes:
- Added a basic device-enumeration test that iterates through all connected devices and verifies sensor responsiveness.
- Added a multi-device, multi-stream stress test that finds a common stream configuration across devices, streams simultaneously, and checks for frame drops/independence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| unit-tests/live/multi_devices/test-devices-streaming.py | New multi-device multi-stream test with common-profile discovery, streaming collection, and drop analysis. |
| unit-tests/live/multi_devices/test-devices-enumeration.py | New multi-device enumeration test that logs device info and checks sensors are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please update once ready for review |
5fd5940 to
1433bef
Compare
|
@AviaAv please review |
|
|
||
| if device_count != 2: | ||
| log.e(f"FAIL: Test requires exactly 2 D400 devices but found {device_count}") | ||
| test.print_results_and_exit() |
There was a problem hiding this comment.
log.f
since those are the first multiple devices tests, I'd even consider adding a function that will do find_first_device_or_exit for those tests - find_n_devices_or_exit(number_of_wanted_devices) or something similar
relevant for both tests
| if len(stream_configs) < 2: | ||
| log.w(f"Insufficient common streams found ({len(stream_configs)})") | ||
| log.w("At least 2 stream types needed for multi-stream test") | ||
| test.check(False, "Devices should support at least 2 common stream types") |
There was a problem hiding this comment.
log.f here too, this will exit the test so we can also remove the else block and unindent
note that it might be needed to be outside of the test closure, on_fail=test.abort is also an option
| All streams will use the same resolution and FPS for simplicity. | ||
| """ | ||
| # Guard against empty device list | ||
| if len(devs) == 0: |
|
|
||
| # Try common resolutions in order of preference | ||
| # 640x360 added as fallback to support safety camera profiles | ||
| target_resolutions = [ |
There was a problem hiding this comment.
This is a variable that is being passed as a parameter many times, but only used at _select_best_resolution - I suggest we move it to that function, or have it as a global variable in this test, passing it as a parameter on many functions seems confusing IMO
also, the test is running currently just on D400 - do we need SC resolutions? if yes, is there another resolution that should be present on both SC and D400?
There was a problem hiding this comment.
great comment.
the resolution 480x360 is present on both.
| return common_profiles | ||
|
|
||
|
|
||
| def _select_best_resolution(available_configs, target_resolutions): |
There was a problem hiding this comment.
function can be simplified to something like
next((t for t in target_resolutions if t in available_configs), None)
There was a problem hiding this comment.
not used - becasue of your next comment
| return all_profiles | ||
|
|
||
|
|
||
| def _find_common_profiles(all_profiles, stream_key): |
There was a problem hiding this comment.
def find_common_profile(devs, stream_type, stream_format, target_resolutions):
"""Return (width, height, fps) available on all devices, or None."""
for w, h, fps in target_resolutions:
if all(
any(p.stream_type() == stream_type and p.format() == stream_format
and p.as_video_stream_profile().width() == w
and p.as_video_stream_profile().height() == h
and p.fps() == fps
for sensor in dev.query_sensors()
for p in sensor.profiles
if p.is_video_stream_profile())
for dev in devs
):
return w, h, fps
return None
That replaces _discover_device_profiles, _find_common_profiles, _select_best_resolution, and all three _try_add_*_stream
functions — ~120 lines down to ~12. And it uses the API the way every other test in the repo does.
can you please try and see if works for us? this test seems very long
There was a problem hiding this comment.
great improvement!
unit-tests/py/rspy/test.py
Outdated
| devices_list = c.query_devices() | ||
|
|
||
| device_count = devices_list.size() | ||
| if device_count != n: |
There was a problem hiding this comment.
the original find_first_device_or_exit does not fail if there's more than one device - IMO let's make this function also pass if there are more devices and just return the first n requested - this if should be device_count < n - up to you
Tracked by: RSDEV-6160