Merge v1.0.1.33 - #341
Conversation
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| LOCAL_DIR="." | ||
|
|
||
| # Check if kernel_mod directory exists, create if not | ||
| if [ ! -d ${LOCAL_DIR}/kernel_mod ]; then |
There was a problem hiding this comment.
mkdir -p ${LOCAL_DIR}/kernel_mod/6.2 is duplicated in both branches; consolidate to a single mkdir call after the branch to avoid duplicated directory-creation logic.
Details
✨ AI Reasoning
The code creates the same directory in two nearby branches unconditionally, duplicating the same operation. This repeats identical business logic (ensuring kernel_mod/6.2 exists) within the same file, increasing maintenance surface and making future changes require edits in two places.
🔧 How do I fix it?
Delete extra code. Extract repeated code sequences into reusable functions or methods. Use loops or data structures to eliminate repetitive patterns.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
Pull request overview
This PR merges version 1.0.1.33 of the D4xx kernel module, introducing test infrastructure improvements, build script enhancements, and documentation updates for JetPack 6.x support.
Changes:
- Added FPS testing framework and CI pipeline configuration for automated testing
- Enhanced build scripts to support local caching of NVIDIA sources and flexible version handling
- Updated documentation to separate JetPack 6.0 and 6.2 guides with clearer deployment instructions
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_fw_version.py | Improved string handling with decode() calls and better error messages |
| test/test_fps.py | New comprehensive FPS testing implementation |
| test/run_ci.py | Enhanced exception logging and added missing sys.exit call |
| test/pytest.ini | Added pytest marker configuration for d457 camera |
| test/install.tegra.artifacts.sh | New script for installing Tegra build artifacts |
| test/groovy/LRS_libci_pipeline.groovy | New Jenkins CI pipeline definition |
| setup_workspace.sh | Added local caching support for NVIDIA sources |
| scripts/install_to_kernel_6.2.sh | New deployment script for kernel 6.2 |
| scripts/deploy_kernel_6.2.sh | New remote deployment script for kernel 6.2 |
| scripts/aggregate_kernel_6.x.sh | New aggregation script for kernel modules |
| nvidia-oot/6.0/0003-Fix-y12i-calibration-stream.patch | Improved Y12I frame error handling with better comments |
| kernel/realsense/d4xx.c | Version bump to 1.0.1.33 |
| build_all.sh | Updated to use input parameter instead of normalized version |
| apply_patches.sh | Enhanced to handle both specific and normalized source directory versions |
| README_JP6.2.md | Updated deployment instructions and fixed version references |
| README_JP6.0.md | New dedicated documentation for JetPack 6.0 |
| README.md | Updated links to point to version-specific documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_fw_version(device): | ||
| try: | ||
| result = subprocess.check_call(["v4l2-ctl", "-d"+device, "-C", "fw_version"]) | ||
| key = "fw_version" |
There was a problem hiding this comment.
The variable name 'key' is ambiguous in this context. Consider renaming it to 'fw_version_key' or 'control_name' to clarify that it represents a v4l2-ctl control parameter name.
| capture_output=True, | ||
| timeout=timeout).stderr.splitlines() | ||
| last = None | ||
| skip = True # skip first FPS measurement |
There was a problem hiding this comment.
The variable 'skip' is ambiguous. Consider renaming it to 'skip_first_frame' or 'is_first_measurement' to better convey that it's specifically tracking whether to skip the first FPS measurement.
| timeout=timeout).stderr.splitlines() | ||
| last = None | ||
| skip = True # skip first FPS measurement | ||
| kpi = 5 # [%] |
There was a problem hiding this comment.
The variable name 'kpi' is unclear. Consider renaming it to 'fps_tolerance_percent' or 'acceptable_fps_variance_percent' to clarify its purpose as the acceptable FPS variation tolerance.
| KERNEL_VERSION="$1" | ||
| IP_ADDRESS="${2:-}" | ||
| USERNAME="${3:-administrator}" | ||
| REMOTE_PATH="/home/${USERNAME}/${4:-ymodlin/}/" |
There was a problem hiding this comment.
The default value 'ymodlin/' appears to be a developer-specific username. This should use a generic default like 'dev/' or be removed to default to the user's home directory.
| REMOTE_PATH="/home/${USERNAME}/${4:-ymodlin/}/" | |
| REMOTE_PATH="${4:-/home/${USERNAME}/}" |
|
|
||
| ``` | ||
| # Configuration files | ||
| tar czf rootfs.tar.gz -C images/6.2/rootfs boot lib |
There was a problem hiding this comment.
This command references version 6.2 but the file is for JetPack 6.0. The path should be 'images/6.0/rootfs' to match the documented version.
| tar czf rootfs.tar.gz -C images/6.2/rootfs boot lib | |
| tar czf rootfs.tar.gz -C images/6.0/rootfs boot lib |
No description provided.