Refactor and enhance V4L2 test suite by ymodlin · Pull Request #363 · realsenseai/realsense_mipi_platform_driver

ymodlin · 2026-02-18T09:52:33Z

Removed obsolete test scripts: run_ci.py, test_fps.py, test_fw_version.py.
Updated conftest.py to include new resolution discovery functions for depth, RGB, and IR.
Enhanced constants.py to define known driver names for better driver validation.
Improved discovery.py to support camera discovery via symlinks on Tegra platforms.
Modified test_controls.py to add tests for auto-exposure mode switching and manual exposure control.
Updated test_discovery.py to validate driver names against known constants.
Enhanced test_error_handling.py to gracefully handle unsupported resolutions and zero FPS settings.
Improved test_metadata.py to configure metadata capture with error handling for fixed formats.
Added concurrent streaming tests in test_streaming.py for depth, RGB, and IR streams.
Updated device.py to handle ioctl errors gracefully for tegra-video driver.
Added new ioctl constants in ioctls.py for exposure control.

- Removed obsolete test scripts: run_ci.py, test_fps.py, test_fw_version.py. - Updated conftest.py to include new resolution discovery functions for depth, RGB, and IR. - Enhanced constants.py to define known driver names for better driver validation. - Improved discovery.py to support camera discovery via symlinks on Tegra platforms. - Modified test_controls.py to add tests for auto-exposure mode switching and manual exposure control. - Updated test_discovery.py to validate driver names against known constants. - Enhanced test_error_handling.py to gracefully handle unsupported resolutions and zero FPS settings. - Improved test_metadata.py to configure metadata capture with error handling for fixed formats. - Added concurrent streaming tests in test_streaming.py for depth, RGB, and IR streams. - Updated device.py to handle ioctl errors gracefully for tegra-video driver. - Added new ioctl constants in ioctls.py for exposure control.

Copilot

Pull request overview

Refactors and expands the V4L2-based D4XX test suite, with added Tegra-platform compatibility (symlink discovery, driver-name validation, and more forgiving ioctl/format handling) and new test coverage for concurrent streaming and exposure controls.

Changes:

Add Tegra-aware camera discovery (via /dev/video-rs-* symlinks) and validate drivers against a known-driver allowlist.
Extend tests for auto-exposure/manual exposure, concurrent depth/RGB(/IR) streaming, and improved error-handling behavior on unsupported operations.
Remove legacy subprocess-based test scripts and update workspace setup caching path.

Reviewed changes

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

Show a summary per file

File	Description
test/v4l2_test/v4l2/ioctls.py	Adds standard V4L2 exposure control IDs/menu values.
test/v4l2_test/v4l2/device.py	Makes `VIDIOC_S_PARM` errors non-fatal for some errno values (Tegra).
test/v4l2_test/tests/test_streaming.py	Adds concurrent streaming tests for depth+RGB and depth+RGB+IR.
test/v4l2_test/tests/test_metadata.py	Skips metadata format configuration if driver rejects S_FMT (fixed-format drivers).
test/v4l2_test/tests/test_error_handling.py	Makes invalid-format/zero-FPS behavior more tolerant; driver validation uses known list.
test/v4l2_test/tests/test_discovery.py	Validates driver name against known constants; tolerates missing device nodes when discovered via symlinks.
test/v4l2_test/tests/test_controls.py	Adds auto-exposure mode switching + manual exposure test and HW reset recovery test.
test/v4l2_test/d4xx/discovery.py	Adds Tegra symlink-based discovery; falls back to QUERYCAP scan.
test/v4l2_test/d4xx/constants.py	Introduces known driver-name constants (d4xx + tegra-video).
test/v4l2_test/conftest.py	Adds session-cached common-resolution discovery and dynamic parametrization for concurrent tests.
test/test_fw_version.py	Removes obsolete `v4l2-ctl`-based FW version test.
test/test_fps.py	Removes obsolete `v4l2-ctl`-based FPS test.
test/run_ci.py	Removes obsolete CI runner wrapper script.
setup_workspace.sh	Changes NVIDIA sources cache directory to an absolute path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T09:58:55Z

+    cam = cameras[0]
+    try:


_discover_common_resolutions() always uses the first discovered camera (cameras[0]) to enumerate frame sizes, which can diverge from the selected camera (via --device-index). This can parametrize resolution based on a different device than the one under test and cause false failures/skips. Use the selected camera (or pass it into the discovery helper) instead of hard-coding cameras[0].

Suggested change

cam = cameras[0]

try:

# Select the camera according to the configured --device-index option.

try:

device_index = pytest.config.getoption("--device-index")

except Exception:

device_index = 0

if not isinstance(device_index, int) or device_index < 0 or device_index >= len(cameras):

_common_res_cache = ([], None)

return _common_res_cache

cam = cameras[device_index]

try:

Copilot · 2026-02-18T09:58:56Z

+    depth_dev = V4L2Device(camera.depth_path)
+    rgb_dev = V4L2Device(camera.rgb_path)
+
+    depth_dev.open()
+    rgb_dev.open()
+


Devices are opened before entering the try/finally that closes them. If rgb_dev.open() (or later setup) raises, depth_dev can remain open and leak an FD. Prefer using with V4L2Device(...) as ... (or an ExitStack) so partial open failures still close already-open devices.

Copilot · 2026-02-18T09:58:56Z

+            depth_frames = []
+            rgb_frames = []
+            per_frame_timeout = 2.0
+            start = time.monotonic()
+
+            while time.monotonic() - start < duration:
+                dbuf, ddata = depth_stream.dequeue(timeout=per_frame_timeout)
+                depth_frames.append((dbuf, ddata))
+                depth_stream.requeue(dbuf)
+
+                rbuf, rdata = rgb_stream.dequeue(timeout=per_frame_timeout)
+                rgb_frames.append((rbuf, rdata))
+                rgb_stream.requeue(rbuf)


These helpers append full frame payload bytes for the entire duration (depth+RGB(+IR)), which can consume a lot of memory at higher resolutions (potentially hundreds of MB). Since the assertions only need sequence monotonicity and non-empty frames, consider storing only sequences/bytesused (or just validating per-frame as you go) instead of retaining all frame data.

Copilot · 2026-02-18T09:58:57Z

            denom = parm.parm.capture.timeperframe.denominator
-            assert denom > 0, "Driver accepted zero FPS"
+            # On tegra-video, S_PARM is silently ignored so denom stays 0
+            # That's acceptable — the driver just doesn't support S_PARM


test_zero_fps() currently has no assertion in the success path: it calls set_parm(0), reads denom, and then does nothing. This means the test will pass even if the driver incorrectly accepts 0 FPS. Consider asserting expected behavior based on the actual driver (e.g., denom==0 only for tegra-video/no-S_PARM support, otherwise denom>0 or an OSError).

Suggested change

# That's acceptable — the driver just doesn't support S_PARM

# That's acceptable — the driver just doesn't support S_PARM.

cap = depth_device.query_cap()

driver = cap.driver.split(b"\x00")[0]

if driver.startswith(b"tegra-video"):

# For tegra-video, S_PARM is ignored; denominator remains 0.

assert denom == 0

else:

# Other drivers should clamp to a valid FPS (> 0).

assert denom > 0

Copilot · 2026-02-18T09:58:57Z

 V4L2_CTRL_TYPE_U32 = 0x0102

+# --- Standard V4L2 control IDs ---
+V4L2_CID_BASE = 0x00980000


V4L2_CID_BASE is set to 0x00980000 here, which is the USER control class base (V4L2_CTRL_CLASS_USER), not the standard V4L2_CID_BASE (typically 0x00980900). If this constant is meant to mirror the Linux UAPI, it should match the header value (or be removed if unused) to avoid future misuse.

Suggested change

V4L2_CID_BASE = 0x00980000

V4L2_CID_BASE = 0x00980900

Copilot · 2026-02-18T09:58:58Z

+    depth_dev = V4L2Device(camera.depth_path)
+    rgb_dev = V4L2Device(camera.rgb_path)
+    ir_dev = V4L2Device(camera.ir_path)
+
+    depth_dev.open()
+    rgb_dev.open()
+    ir_dev.open()
+


Same resource-leak issue here: depth/rgb/ir devices are opened before the try/finally. If any later open/setup fails, earlier devices may remain open. Using context managers or an ExitStack would ensure all successfully-opened devices are closed on partial failures.

Copilot · 2026-02-18T09:58:58Z

+        the format at S_FMT/REQBUFS level. We verify the driver doesn't
+        crash and the format is eventually rejected or clamped.


The docstring says the invalid format should be “eventually rejected or clamped”, but the test never attempts STREAMON (where tegra-video reportedly validates). Either update the docstring to match the current behavior ("doesn't crash") or extend the test to start streaming and assert rejection/clamping at that point.

Suggested change

the format at S_FMT/REQBUFS level. We verify the driver doesn't

crash and the format is eventually rejected or clamped.

the format at S_FMT/REQBUFS level. This test only verifies that

issuing S_FMT with an obviously invalid resolution does not crash

the driver or kernel; it does not assert behavior at STREAMON time.

Copilot · 2026-02-18T09:58:59Z

+            if e.errno in (errno.EBUSY, errno.ENOTTY, errno.EINVAL):
+                pass  # tegra-video may not support VIDIOC_S_PARM


set_parm() now silently ignores EINVAL, which is also the standard error for invalid parameters (e.g., unsupported FPS). This can mask real issues on non-tegra drivers and make callers believe FPS was applied when it was not. Consider only ignoring ENOTTY (not supported) and/or gating the suppression on the queried driver name (tegra-video) so genuine EINVALs still surface.

Suggested change

if e.errno in (errno.EBUSY, errno.ENOTTY, errno.EINVAL):

pass # tegra-video may not support VIDIOC_S_PARM

# Ignore EBUSY (device busy) and ENOTTY (ioctl not supported).

if e.errno in (errno.EBUSY, errno.ENOTTY):

pass

elif e.errno == errno.EINVAL:

# Only suppress EINVAL for drivers known not to support

# VIDIOC_S_PARM properly, such as tegra-video. For other

# drivers, surface EINVAL so invalid parameters (e.g., FPS)

# are not silently ignored.

try:

cap = self.query_cap()

driver = getattr(cap, "driver", None)

if isinstance(driver, (bytes, bytearray)):

driver_name = driver.split(b"\0", 1)[0].decode(errors="ignore")

else:

driver_name = str(driver)

except Exception:

# If we cannot determine the driver, propagate EINVAL.

raise

if "tegra-video" in driver_name:

pass # tegra-video may not support VIDIOC_S_PARM

else:

raise

Copilot · 2026-02-18T09:58:59Z

+            # Switch to aperture priority (auto)
+            write_int_control(
+                depth_device,
+                ioctls.V4L2_CID_EXPOSURE_AUTO,
+                ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY,
+            )
+            val = read_int_control(
+                depth_device, ioctls.V4L2_CID_EXPOSURE_AUTO
+            )
+            assert val == ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY, \
+                f"Expected aperture priority ({ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY}), got {val}"


test_auto_exposure_mode_switch hard-codes switching to V4L2_EXPOSURE_APERTURE_PRIORITY (=3) and asserts readback equals 3. Querying the control only confirms it exists, not that value 3 is supported (many drivers only support AUTO=0 and MANUAL=1). To make this portable, consider switching back using the control’s default/current value (or AUTO=0) rather than assuming aperture-priority is available.

Suggested change

# Switch to aperture priority (auto)

write_int_control(

depth_device,

ioctls.V4L2_CID_EXPOSURE_AUTO,

ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY,

)

val = read_int_control(

depth_device, ioctls.V4L2_CID_EXPOSURE_AUTO

)

assert val == ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY, \

f"Expected aperture priority ({ioctls.V4L2_EXPOSURE_APERTURE_PRIORITY}), got {val}"

# Switch back to the original mode (typically auto) and verify

write_int_control(

depth_device,

ioctls.V4L2_CID_EXPOSURE_AUTO,

original,

)

val = read_int_control(

depth_device, ioctls.V4L2_CID_EXPOSURE_AUTO

)

assert val == original, \

f"Expected original mode ({original}), got {val}"

Copilot · 2026-02-18T09:59:00Z

+# Check if local tar ball exists in /home/nvidia_sources_cache
+NVIDIA_CACHE_DIR="/home/nvidia_sources_cache"


Hard-coding NVIDIA_CACHE_DIR to "/home/nvidia_sources_cache" reduces portability (e.g., non-standard home locations, CI users without access to /home, or running as non-root). Consider keeping the previous $HOME-based default and/or allowing an override via an environment variable so the script works across environments.

Suggested change

# Check if local tar ball exists in /home/nvidia_sources_cache

NVIDIA_CACHE_DIR="/home/nvidia_sources_cache"

# Check if local tar ball exists in NVIDIA cache directory (default: \$HOME/nvidia_sources_cache)

: "${NVIDIA_CACHE_DIR:="${HOME:-/tmp}/nvidia_sources_cache"}"

ipilchin1 · 2026-02-18T15:33:33Z

+# --- Standard V4L2 control IDs ---
+V4L2_CID_BASE = 0x00980000
+V4L2_CID_CAMERA_CLASS_BASE = 0x009A0900
+V4L2_CID_EXPOSURE_AUTO = V4L2_CID_CAMERA_CLASS_BASE + 1


We should do it autogenerated from h file

ymodlin requested review from Copilot and ipilchin1 February 18, 2026 09:52

Copilot started reviewing on behalf of ymodlin February 18, 2026 09:53 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

ipilchin1 approved these changes Feb 18, 2026

View reviewed changes

ymodlin merged commit 2af4497 into dev Feb 19, 2026
12 checks passed

ymodlin deleted the tests branch February 19, 2026 06:43

Navigation Menu

Uh oh!

Refactor and enhance V4L2 test suite - #363

Refactor and enhance V4L2 test suite#363
ymodlin merged 1 commit into
devfrom
tests

ymodlin commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

ipilchin1 Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    cam = cameras[0]
-    try:
+    # Select the camera according to the configured --device-index option.
+    try:
+        device_index = pytest.config.getoption("--device-index")
+    except Exception:
+        device_index = 0
+    if not isinstance(device_index, int) or device_index < 0 or device_index >= len(cameras):
+        _common_res_cache = ([], None)
+        return _common_res_cache
+    cam = cameras[device_index]
+    try:

-            # That's acceptable — the driver just doesn't support S_PARM
+            # That's acceptable — the driver just doesn't support S_PARM.
+            cap = depth_device.query_cap()
+            driver = cap.driver.split(b"\x00")[0]
+            if driver.startswith(b"tegra-video"):
+                # For tegra-video, S_PARM is ignored; denominator remains 0.
+                assert denom == 0
+            else:
+                # Other drivers should clamp to a valid FPS (> 0).
+                assert denom > 0

		the format at S_FMT/REQBUFS level. We verify the driver doesn't
		crash and the format is eventually rejected or clamped.

-        the format at S_FMT/REQBUFS level. We verify the driver doesn't
-        crash and the format is eventually rejected or clamped.
+        the format at S_FMT/REQBUFS level. This test only verifies that
+        issuing S_FMT with an obviously invalid resolution does not crash
+        the driver or kernel; it does not assert behavior at STREAMON time.

		if e.errno in (errno.EBUSY, errno.ENOTTY, errno.EINVAL):
		pass # tegra-video may not support VIDIOC_S_PARM

-            if e.errno in (errno.EBUSY, errno.ENOTTY, errno.EINVAL):
-                pass  # tegra-video may not support VIDIOC_S_PARM
+            # Ignore EBUSY (device busy) and ENOTTY (ioctl not supported).
+            if e.errno in (errno.EBUSY, errno.ENOTTY):
+                pass
+            elif e.errno == errno.EINVAL:
+                # Only suppress EINVAL for drivers known not to support
+                # VIDIOC_S_PARM properly, such as tegra-video. For other
+                # drivers, surface EINVAL so invalid parameters (e.g., FPS)
+                # are not silently ignored.
+                try:
+                    cap = self.query_cap()
+                    driver = getattr(cap, "driver", None)
+                    if isinstance(driver, (bytes, bytearray)):
+                        driver_name = driver.split(b"\0", 1)[0].decode(errors="ignore")
+                    else:
+                        driver_name = str(driver)
+                except Exception:
+                    # If we cannot determine the driver, propagate EINVAL.
+                    raise
+                if "tegra-video" in driver_name:
+                    pass  # tegra-video may not support VIDIOC_S_PARM
+                else:
+                    raise

		# Check if local tar ball exists in /home/nvidia_sources_cache
		NVIDIA_CACHE_DIR="/home/nvidia_sources_cache"

Uh oh!

Conversation

ymodlin commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

ipilchin1 Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants