Merge main back into develop#1892
Conversation
Release 3.8.0
📝 WalkthroughWalkthroughThe PR updates DepthAI to version 3.8.0, adjusts device and visualizer metadata, changes Depth and Sync wiring, revises ToF visualization ranges, refines calibration logging, and updates related test thresholds. ChangesDepthAI 3.8.0 integration updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bindings/python/src/pipeline/node/SyncBindings.cpp`:
- Line 47: Update the py::arg name in the Sync::setSyncAttempts binding from
maxDataSize to syncAttempts, matching the method’s C++ parameter and
sync-attempt semantics; leave the binding target and documentation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6be85e02-f81e-4eb8-9050-369edd43cce1
📒 Files selected for processing (17)
CMakeLists.txtbindings/python/src/pipeline/node/SyncBindings.cppcmake/Depthai/DepthaiDeviceRVC4Config.cmakecmake/Depthai/DepthaiDeviceSideConfig.cmakecmake/Depthai/DepthaiVisualizerConfig.cmakeexamples/cpp/ToF/tof_all_queues.cppexamples/cpp/ToF/tof_minimal.cppexamples/python/ToF/tof_align.pyexamples/python/ToF/tof_all_queues.pyexamples/python/ToF/tof_minimal.pyinclude/depthai/pipeline/node/Sync.hppinclude/depthai/pipeline/node/ToF.hppsrc/pipeline/node/Depth.cppsrc/pipeline/node/DynamicCalibrationNode.cppsrc/pipeline/node/Sync.cpptests/src/ondevice_tests/dynamic_calibration_test.cpptests/src/ondevice_tests/neural_depth_node_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-05-28T13:36:26.383Z
Learnt from: moratom
Repo: luxonis/depthai-core PR: 1812
File: examples/cpp/ImageManip/image_manip_remap.cpp:51-52
Timestamp: 2026-05-28T13:36:26.383Z
Learning: In depthai-core example code, do not set `ImageManip::Backend::GPU` unconditionally. The GPU backend is only available on RVC4 (not RVC2). Prefer leaving the backend as the default, or comment out the GPU backend selection and add a clear note explaining it is RVC4-only support (so the example won’t fail or mislead on RVC2).
Applied to files:
examples/cpp/ToF/tof_minimal.cppexamples/python/ToF/tof_all_queues.pyexamples/cpp/ToF/tof_all_queues.cppexamples/python/ToF/tof_minimal.pyexamples/python/ToF/tof_align.py
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/node/Sync.cppsrc/pipeline/node/DynamicCalibrationNode.cppsrc/pipeline/node/Depth.cpp
🔇 Additional comments (18)
src/pipeline/node/DynamicCalibrationNode.cpp (1)
31-33: 🗄️ Data Integrity & IntegrationVerify the exact DCL error-message contract.
These checks depend on exact, punctuation- and case-sensitive string matches. Any DCL wording change or added context will route an expected condition back to
warn, reintroducing warning spam. Prefer stable error codes/categories if DCL exposes them; otherwise add contract tests covering the supported messages.Also applies to: 534-538
tests/src/ondevice_tests/dynamic_calibration_test.cpp (1)
563-565: 🎯 Functional CorrectnessJustify the widened rotation tolerance.
Changing the threshold from
1e-5fto3e-2fincreases the permitted error by 3000×. RANSAC nondeterminism explains variability, but not why this bound is safe; the test may now accept materially incorrect calibration. Derive the threshold from repeated-run measurements or use a deterministic seed/domain-specific error bound.CMakeLists.txt (1)
78-78: LGTM!cmake/Depthai/DepthaiDeviceRVC4Config.cmake (1)
6-6: 🗄️ Data Integrity & IntegrationVerify the exact RVC4 firmware artifact before merging.
This value controls both artifact selection and the compiled resource filename (
depthai-device-rvc4-fwp-<version>.tar.xz). Confirm that the artifact for commitc0f6b6100c93f21e29c361cffad1f83bba0418cais published and matches the intended release.cmake/Depthai/DepthaiDeviceSideConfig.cmake (1)
5-5: 🗄️ Data Integrity & IntegrationVerify the device-side commit is published and compatible.
DEPTHAI_DEVICE_SIDE_COMMITis passed directly toDepthaiDownload; confirm thatc3f4bc9cef3937fa987135129757d950d1fbbd19resolves to the intended 3.8.0 device-side binary.cmake/Depthai/DepthaiVisualizerConfig.cmake (1)
2-2: 🔒 Security & PrivacyConfirm that using a mutable visualizer tag is intentional.
The comment specifies a full commit hash, but the value is the
3.5.2tag. Verify that tag-based resolution is supported and acceptable for reproducible release builds; otherwise pin the corresponding commit SHA.tests/src/ondevice_tests/neural_depth_node_test.cpp (1)
36-45: 🎯 Functional CorrectnessVerify fractional thresholds do not alter benchmark scheduling.
minFpsis multiplied by two at Line 73. The new1.6fvalue produces3.2, which may be implicitly converted to an integer bysendReportEveryNMessages; confirm the API type and use explicit rounding if needed to keep report cadence deterministic.include/depthai/pipeline/node/ToF.hpp (1)
31-36: LGTM!Also applies to: 77-77
src/pipeline/node/Depth.cpp (2)
782-789: LGTM! The#ifdef DEPTHAI_INTERNAL_DEVICE_BUILD_RVC4branches are consistent withToF.hppline 128 (#ifndef DEPTHAI_INTERNAL_DEVICE_BUILD_RVC4), which gates the top-leveldepth/confidenceoutputs on theToFclass. The profile enum change fromImageFiltersPresetMode::TOF_MID_RANGEtoToFConfig::Profile::MID_RANGEcorrectly aligns with the updatedToF::buildsignature.
796-801: 🗄️ Data Integrity & IntegrationNAS confidence should stay wired from
stereoDepthThe non-RVC4NeuralAssistedStereowrapper does not expose a top-levelconfidenceMap;stereoDepth->confidenceMapis the correct source for both platforms.> Likely an incorrect or invalid review comment.include/depthai/pipeline/node/Sync.hpp (1)
68-68: LGTM!src/pipeline/node/Sync.cpp (1)
27-28: LGTM!bindings/python/src/pipeline/node/SyncBindings.cpp (1)
50-52: LGTM! Named arguments improve Python API ergonomics and are consistent with the C++ signatures.examples/cpp/ToF/tof_all_queues.cpp (1)
53-55: LGTM! Depth visualization range updated consistently with the PR objective (minimum depth to 100 mm) and matching the Python ToF examples.examples/cpp/ToF/tof_minimal.cpp (1)
46-48: LGTM!examples/python/ToF/tof_align.py (1)
20-21: LGTM!examples/python/ToF/tof_all_queues.py (1)
37-38: LGTM!examples/python/ToF/tof_minimal.py (1)
33-34: LGTM!
| @@ -47,9 +47,9 @@ void bind_sync(pybind11::module& m, void* pCallstack) { | |||
| .def("setSyncAttempts", &Sync::setSyncAttempts, py::arg("maxDataSize"), DOC(dai, node, Sync, setSyncAttempts)) | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix incorrect py::arg name for setSyncAttempts.
Line 47 uses py::arg("maxDataSize") for setSyncAttempts, but the C++ parameter is syncAttempts and the method manages sync attempt count — not max data size. This is a pre-existing copy-paste error, but since this PR is already correcting Sync binding argument names, it's a natural one-line fix.
♻️ Proposed fix
- .def("setSyncAttempts", &Sync::setSyncAttempts, py::arg("maxDataSize"), DOC(dai, node, Sync, setSyncAttempts))
+ .def("setSyncAttempts", &Sync::setSyncAttempts, py::arg("syncAttempts"), DOC(dai, node, Sync, setSyncAttempts))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .def("setSyncAttempts", &Sync::setSyncAttempts, py::arg("maxDataSize"), DOC(dai, node, Sync, setSyncAttempts)) | |
| .def("setSyncAttempts", &Sync::setSyncAttempts, py::arg("syncAttempts"), DOC(dai, node, Sync, setSyncAttempts)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bindings/python/src/pipeline/node/SyncBindings.cpp` at line 47, Update the
py::arg name in the Sync::setSyncAttempts binding from maxDataSize to
syncAttempts, matching the method’s C++ parameter and sync-attempt semantics;
leave the binding target and documentation unchanged.
5 participants
Purpose
Specification
None / not applicable
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
Submitted code was reviewed by a human: YES/NO
The author is taking the responsibility for the contribution: YES/NO
Summary by CodeRabbit
New Features
Bug Fixes
Examples