Adding Claude skills by ymodlin · Pull Request #349 · realsenseai/realsense_mipi_platform_driver

ymodlin · 2026-02-09T09:50:46Z

No description provided.

…rm driver

…tions for Jetson

…n 6.2

Copilot

Pull request overview

Adds a Python-based V4L2 test harness for D4XX cameras (discovery, controls, streaming, metadata) and introduces Claude guidance/skills documentation, while removing older kernel 6.2 deployment helper scripts.

Changes:

Introduce a lightweight V4L2 Python wrapper (ctypes structs + ioctl numbers + device/stream/control helpers).
Add pytest-based on-device validation suite (discovery, controls, streaming, metadata, error handling) plus a summary report plugin.
Add CLAUDE.md and .claude/ skills/agents docs; remove legacy kernel 6.2 deployment scripts.

Reviewed changes

Copilot reviewed 24 out of 49 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
test/v4l2_test/v4l2/structs.py	Adds ctypes definitions for V4L2 ioctl argument structs used by the test harness.
test/v4l2_test/v4l2/stream.py	Implements mmap streaming context manager for STREAMON/DQBUF/QBUF workflows.
test/v4l2_test/v4l2/ioctls.py	Defines ioctl number encoding + pixel format constants used by device and tests.
test/v4l2_test/v4l2/device.py	Adds device wrapper for open/close/ioctl plus common V4L2 operations (format, parms, controls).
test/v4l2_test/v4l2/controls.py	Adds typed helpers for standard and extended V4L2 controls.
test/v4l2_test/tests/test_streaming.py	Adds streaming FPS/sequence validation tests across depth/IR/RGB.
test/v4l2_test/tests/test_metadata.py	Adds metadata capture/parse/CRC tests alongside depth streaming.
test/v4l2_test/tests/test_error_handling.py	Adds boundary/recovery tests (invalid format/FPS, double STREAMON, etc.).
test/v4l2_test/tests/test_discovery.py	Adds discovery/capabilities/format enumeration tests and DFU node presence check.
test/v4l2_test/tests/test_controls.py	Adds control roundtrip tests for common D4XX CIDs (laser, exposure, ROI, tables).
test/v4l2_test/report.py	Adds pytest plugin to summarize results by category at session end.
test/v4l2_test/pytest.ini	Defines `d457` marker and testpaths for the new test suite.
test/v4l2_test/d4xx/metadata.py	Adds metadata ctypes layouts + parsing and CRC32 validation utilities.
test/v4l2_test/d4xx/discovery.py	Implements /dev/video* scanning and grouping into D4XX cameras.
test/v4l2_test/d4xx/constants.py	Adds D4XX CID constants and per-camera stream layout constants.
test/v4l2_test/conftest.py	Adds pytest fixtures for camera discovery, device lifecycle, and report plugin setup.
scripts/install_to_kernel_6.2.sh	Removes legacy on-device kernel install helper for JP 6.2.
scripts/deploy_kernel_6.2.sh	Removes legacy deploy wrapper for JP 6.2.
scripts/aggregate_kernel_6.x.sh	Removes legacy kernel artifact aggregation script.
CLAUDE.md	Adds repo-wide build/test/deploy guidance aimed at Claude Code usage.
.claude/skills/workspace-setup/SKILL.md	Adds Claude “workspace setup” skill guide for onboarding/build prep.
.claude/skills/build-deploy/SKILL.md	Adds Claude “build & deploy” skill guide for JetPack workflows.
.claude/agents/test-runner.md	Adds agent doc for running/diagnosing the new native V4L2 pytest suite.
.claude/agents/dt-helper.md	Adds agent doc for DT overlay/include workflows and camera DT troubleshooting.

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

Copilot · 2026-02-09T10:03:52Z

+    def dequeue(self, timeout=5.0):
+        """Dequeue a single buffer. Returns (v4l2_buffer, data_bytes)."""
+        ready, _, _ = select.select([self.device.fileno()], [], [], timeout)
+        if not ready:
+            raise TimeoutError(f"No frame within {timeout}s")
+
+        buf = S.v4l2_buffer()
+        buf.type = self.buf_type
+        buf.memory = ioctls.V4L2_MEMORY_MMAP
+        self.device.ioctl(ioctls.VIDIOC_DQBUF, buf)


V4L2Device.open() uses O_NONBLOCK, so VIDIOC_DQBUF can still raise EAGAIN even after select() indicates readability. This can make streaming flaky. Consider retrying VIDIOC_DQBUF on EAGAIN until the timeout expires (or open the fd without O_NONBLOCK when using select()).

Copilot · 2026-02-09T10:03:53Z

+        fmt = S.v4l2_format()
+        fmt.type = buf_type


For V4L2_BUF_TYPE_META_CAPTURE, drivers commonly rely on v4l2_meta_format.buffersize being set/retained. Sending buffersize=0 can lead to VIDIOC_S_FMT being rejected or subsequent REQBUFS/QUERYBUF returning 0-length buffers (breaking mmap). Consider calling VIDIOC_G_FMT first and preserving fmt.fmt.meta.buffersize, or add a buffersize parameter and set it explicitly.

Suggested change

fmt = S.v4l2_format()

fmt.type = buf_type

# Preserve existing meta format fields (including buffersize) by

# first querying the current format, then updating only dataformat.

fmt = self.get_format(buf_type)

Copilot · 2026-02-09T10:03:53Z

+    Tries the extended MIPI struct first, falls back to normal mode.
+    Returns (struct_instance, struct_type_name).
+    """
+    ext_size = ctypes.sizeof(STMetaDataExtMipiDepthIR)
+    normal_size = ctypes.sizeof(STMetaDataDepthYNormalMode)
+
+    if len(data) >= ext_size:
+        md = STMetaDataExtMipiDepthIR()
+        ctypes.memmove(ctypes.addressof(md), data[:ext_size], ext_size)
+        return md, "ExtMipiDepthIR"
+
+    if len(data) >= normal_size:
+        md = STMetaDataDepthYNormalMode()
+        ctypes.memmove(ctypes.addressof(md), data[:normal_size], normal_size)
+        return md, "DepthYNormalMode"
+


Choosing the metadata struct purely by len(data) will mis-classify payloads when both formats are plausible sizes (e.g., normal-mode metadata may also be >= ext_size). This can cause incorrect field reads and CRC validation. Prefer discriminating based on a header field (e.g., metaDataID/size) or try parsing both and select the one that passes basic sanity checks/CRC.

Suggested change

Tries the extended MIPI struct first, falls back to normal mode.

Returns (struct_instance, struct_type_name).

"""

ext_size = ctypes.sizeof(STMetaDataExtMipiDepthIR)

normal_size = ctypes.sizeof(STMetaDataDepthYNormalMode)

if len(data) >= ext_size:

md = STMetaDataExtMipiDepthIR()

ctypes.memmove(ctypes.addressof(md), data[:ext_size], ext_size)

return md, "ExtMipiDepthIR"

if len(data) >= normal_size:

md = STMetaDataDepthYNormalMode()

ctypes.memmove(ctypes.addressof(md), data[:normal_size], normal_size)

return md, "DepthYNormalMode"

Tries both known layouts using basic sanity checks (CRC/header) and

returns the first one that validates. Falls back to length-based

selection to preserve existing behavior if validation fails.

Returns (struct_instance, struct_type_name) or (None, None).

"""

ext_size = ctypes.sizeof(STMetaDataExtMipiDepthIR)

normal_size = ctypes.sizeof(STMetaDataDepthYNormalMode)

ext_md = None

ext_ok = False

if len(data) >= ext_size:

ext_md = STMetaDataExtMipiDepthIR()

ctypes.memmove(ctypes.addressof(ext_md), data[:ext_size], ext_size)

# Prefer checking against the struct-sized slice so we do not

# accidentally include trailing bytes in the CRC calculation.

# Also sanity-check the embedded size field if present.

size_field_ok = (

not hasattr(ext_md, "size") or

ext_md.size == ext_size or

ext_md.size <= len(data)

)

if size_field_ok and validate_crc32(data[:ext_size], ext_md, "ExtMipiDepthIR"):

ext_ok = True

normal_md = None

normal_ok = False

if len(data) >= normal_size:

normal_md = STMetaDataDepthYNormalMode()

ctypes.memmove(ctypes.addressof(normal_md), data[:normal_size], normal_size)

if validate_crc32(data[:normal_size], normal_md, "DepthYNormalMode"):

normal_ok = True

# Prefer a CRC-valid extended struct, then CRC-valid normal struct.

if ext_ok:

return ext_md, "ExtMipiDepthIR"

if normal_ok:

return normal_md, "DepthYNormalMode"

# Fallback to original length-based behavior if validation failed,

# to avoid changing semantics for callers relying on the old logic.

if len(data) >= ext_size:

return ext_md, "ExtMipiDepthIR"

if len(data) >= normal_size:

return normal_md, "DepthYNormalMode"

Copilot · 2026-02-09T10:03:53Z

+            raw = ctrl.value
+            # FW version is packed as 4 bytes in a 32-bit int
+            major = (raw >> 24) & 0xFF
+            minor = (raw >> 16) & 0xFF
+            patch = (raw >> 8) & 0xFF
+            build = raw & 0xFF
+            return f"{major}.{minor}.{patch}.{build}", raw.to_bytes(4, "big")


v4l2_control.value is a signed 32-bit int; firmware versions with the high bit set will appear negative in Python. While the shifts/masks still work, raw.to_bytes(4, 'big') will raise for negative values. Mask to unsigned (raw &= 0xFFFFFFFF) before formatting/serializing, and use an unsigned conversion when producing bytes.

Copilot · 2026-02-09T10:03:54Z

+        major = (raw >> 24) & 0xFF
+        minor = (raw >> 16) & 0xFF
+        patch = (raw >> 8) & 0xFF
+        build = raw & 0xFF
+        return raw, f"{major}.{minor}.{patch}.{build}"


Same signed/unsigned issue as in discovery: ctrl.value is int32, so raw may be negative for some FW versions. Mask to raw_u32 = raw & 0xFFFFFFFF before extracting bytes/returning so callers and reporting remain consistent.

Suggested change

major = (raw >> 24) & 0xFF

minor = (raw >> 16) & 0xFF

patch = (raw >> 8) & 0xFF

build = raw & 0xFF

return raw, f"{major}.{minor}.{patch}.{build}"

raw_u32 = raw & 0xFFFFFFFF

major = (raw_u32 >> 24) & 0xFF

minor = (raw_u32 >> 16) & 0xFF

patch = (raw_u32 >> 8) & 0xFF

build = raw_u32 & 0xFF

return raw_u32, f"{major}.{minor}.{patch}.{build}"

Copilot · 2026-02-09T10:03:54Z

+The `--one-cam`/`--dual-cam` options only apply to JetPack 5.0.2.
+
+### Deploy to Jetson
+


This doc references scripts and paths that don’t match the changes in this PR: scripts/deploy_kernel_6.2.sh was removed, and the marker config was added under test/v4l2_test/pytest.ini (not test/pytest.ini). Also, pytest -vs -m d457 test/ doesn’t align with the new suite location (test/v4l2_test/). Update these references so the documented commands work with the new tree.

Copilot · 2026-02-09T10:03:54Z

+
+Always reset patches before re-applying:
+```bash
+./apply_patches.sh $VERSION reset


The apply_patches.sh invocation order here conflicts with the usage described elsewhere in the repo (e.g., ./apply_patches.sh reset <version>). Update the skill steps to match the actual script interface so users don’t run a non-working command.

Suggested change

./apply_patches.sh $VERSION reset

./apply_patches.sh reset $VERSION

Copilot · 2026-02-09T10:03:54Z

+
+The command must have all 3 arguments to perform the full deploy.
+Ask the user to provide username and remote path if not provided.
+Save in mempory for the next deploy command.


Corrected spelling of 'mempory' to 'memory'.

Suggested change

Save in mempory for the next deploy command.

Save in memory for the next deploy command.

Copilot · 2026-02-09T10:03:55Z

+import time
+
+import pytest
+
+from ..d4xx import constants as C


time and C are imported but not used in this test module. Removing unused imports avoids lint failures and keeps dependencies clearer.

Suggested change

import time

import pytest

from ..d4xx import constants as C

import pytest

Copilot · 2026-02-09T10:03:55Z

+"""V4L2 ioctl numbers and fourcc pixel format constants."""
+
+import ctypes
+import struct


struct is imported but not used in this module. Removing it avoids lint failures and reduces noise.

Suggested change

import struct

…er directory removal before linking

ipilchin1

Great work!

ymodlin added 6 commits February 8, 2026 13:09

Add build-deploy and workspace-setup skills for RealSense MIPI platfo…

5343303

…rm driver

Fix command order in patch application and enhance deployment instruc…

f8c474d

…tions for Jetson

Add V4L2 streaming tests and core functionality

ebe26a9

Remove obsolete kernel deployment and installation scripts for versio…

b06f926

…n 6.2

Add test-runner agent for D4XX camera tests on Jetson

51ec541

Add device tree helper documentation for D4XX camera on Jetson

4041ac2

ymodlin marked this pull request as ready for review February 9, 2026 10:00

Copilot AI review requested due to automatic review settings February 9, 2026 10:00

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Refactor nvethernetrm symlink handling for JetPack 6.x to ensure prop…

9f86e52

…er directory removal before linking

ymodlin requested a review from ipilchin1 February 9, 2026 12:26

ipilchin1 approved these changes Feb 9, 2026

View reviewed changes

ymodlin merged commit b43d8b6 into dev Feb 10, 2026
6 checks passed

ymodlin deleted the claude-skills branch February 10, 2026 09:53

Navigation Menu

Uh oh!

Adding Claude skills - #349

Adding Claude skills#349
ymodlin merged 7 commits into
devfrom
claude-skills

ymodlin commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

ipilchin1 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        fmt = S.v4l2_format()
-        fmt.type = buf_type
+        # Preserve existing meta format fields (including buffersize) by
+        # first querying the current format, then updating only dataformat.
+        fmt = self.get_format(buf_type)

-    Tries the extended MIPI struct first, falls back to normal mode.
-    Returns (struct_instance, struct_type_name).
-    """
-    ext_size = ctypes.sizeof(STMetaDataExtMipiDepthIR)
-    normal_size = ctypes.sizeof(STMetaDataDepthYNormalMode)
-    if len(data) >= ext_size:
-        md = STMetaDataExtMipiDepthIR()
-        ctypes.memmove(ctypes.addressof(md), data[:ext_size], ext_size)
-        return md, "ExtMipiDepthIR"
-    if len(data) >= normal_size:
-        md = STMetaDataDepthYNormalMode()
-        ctypes.memmove(ctypes.addressof(md), data[:normal_size], normal_size)
-        return md, "DepthYNormalMode"
+    Tries both known layouts using basic sanity checks (CRC/header) and
+    returns the first one that validates. Falls back to length-based
+    selection to preserve existing behavior if validation fails.
+    Returns (struct_instance, struct_type_name) or (None, None).
+    """
+    ext_size = ctypes.sizeof(STMetaDataExtMipiDepthIR)
+    normal_size = ctypes.sizeof(STMetaDataDepthYNormalMode)
+    ext_md = None
+    ext_ok = False
+    if len(data) >= ext_size:
+        ext_md = STMetaDataExtMipiDepthIR()
+        ctypes.memmove(ctypes.addressof(ext_md), data[:ext_size], ext_size)
+        # Prefer checking against the struct-sized slice so we do not
+        # accidentally include trailing bytes in the CRC calculation.
+        # Also sanity-check the embedded size field if present.
+        size_field_ok = (
+            not hasattr(ext_md, "size") or
+            ext_md.size == ext_size or
+            ext_md.size <= len(data)
+        )
+        if size_field_ok and validate_crc32(data[:ext_size], ext_md, "ExtMipiDepthIR"):
+            ext_ok = True
+    normal_md = None
+    normal_ok = False
+    if len(data) >= normal_size:
+        normal_md = STMetaDataDepthYNormalMode()
+        ctypes.memmove(ctypes.addressof(normal_md), data[:normal_size], normal_size)
+        if validate_crc32(data[:normal_size], normal_md, "DepthYNormalMode"):
+            normal_ok = True
+    # Prefer a CRC-valid extended struct, then CRC-valid normal struct.
+    if ext_ok:
+        return ext_md, "ExtMipiDepthIR"
+    if normal_ok:
+        return normal_md, "DepthYNormalMode"
+    # Fallback to original length-based behavior if validation failed,
+    # to avoid changing semantics for callers relying on the old logic.
+    if len(data) >= ext_size:
+        return ext_md, "ExtMipiDepthIR"
+    if len(data) >= normal_size:
+        return normal_md, "DepthYNormalMode"

-        major = (raw >> 24) & 0xFF
-        minor = (raw >> 16) & 0xFF
-        patch = (raw >> 8) & 0xFF
-        build = raw & 0xFF
-        return raw, f"{major}.{minor}.{patch}.{build}"
+        raw_u32 = raw & 0xFFFFFFFF
+        major = (raw_u32 >> 24) & 0xFF
+        minor = (raw_u32 >> 16) & 0xFF
+        patch = (raw_u32 >> 8) & 0xFF
+        build = raw_u32 & 0xFF
+        return raw_u32, f"{major}.{minor}.{patch}.{build}"

		The `--one-cam`/`--dual-cam` options only apply to JetPack 5.0.2.

		### Deploy to Jetson

	./apply_patches.sh $VERSION reset
	./apply_patches.sh reset $VERSION

	Save in mempory for the next deploy command.
	Save in memory for the next deploy command.

Uh oh!

Conversation

ymodlin commented Feb 9, 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 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

ipilchin1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants