Skip to content

Enable dynamic calibration for NeuralDepth and Rectification nodes #1579

Open
lnotspotl wants to merge 14 commits intodevelopfrom
lnotspotl/neural_depth_dynamic_calib
Open

Enable dynamic calibration for NeuralDepth and Rectification nodes #1579
lnotspotl wants to merge 14 commits intodevelopfrom
lnotspotl/neural_depth_dynamic_calib

Conversation

@lnotspotl
Copy link
Copy Markdown
Contributor

@lnotspotl lnotspotl commented Dec 8, 2025

The two nodes mentioned should now update their rectification meshes whenever a change in the calibration data is detected. This all is to enable dynamic calibration to have affect on these nodes.

Screencast.from.2025-12-09.00-02-11.webm

For testing, feel free to use the following script that artificially perturbs the calibration data and then lets the dynamic calibration node recover the original extrinsic parameters:

neural_depth.py

Note: There is a same-name PR open in the RVC4 codebase

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced rectification calibration to automatically detect EEPROM parameter changes and reinitialize matrices as needed, improving system reliability.
  • Chores

    • Updated device version identifier.
    • Streamlined rectification component by removing unnecessary includes and platform-specific validation checks.

@lnotspotl lnotspotl self-assigned this Dec 8, 2025
@lnotspotl lnotspotl marked this pull request as draft December 8, 2025 23:26
@lnotspotl lnotspotl marked this pull request as ready for review December 14, 2025 22:23
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@lnotspotl lnotspotl added the testable PR is ready to be tested label Dec 14, 2025
@lnotspotl lnotspotl removed the testable PR is ready to be tested label Jan 5, 2026
@lnotspotl lnotspotl added the testable PR is ready to be tested label Jan 5, 2026
@richard-xx
Copy link
Copy Markdown
Contributor

Are there any updates available? I would like to enable this feature to enhance the stereo effect.

@lnotspotl
Copy link
Copy Markdown
Contributor Author

CC: @moratom

@lnotspotl lnotspotl added testable PR is ready to be tested and removed testable PR is ready to be tested labels Mar 2, 2026
@moratom
Copy link
Copy Markdown
Collaborator

moratom commented Mar 12, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

This change updates a CMake version variable, removes an unused include from the ToF header, and modifies the Rectification node to detect EEPROM changes and reinitialize rectification matrices accordingly instead of enforcing platform validation.

Changes

Cohort / File(s) Summary
Version & Configuration
cmake/Depthai/DepthaiDeviceRVC4Config.cmake
Updated DEPTHAI_DEVICE_RVC4_VERSION from git hash 619c87a0... to 3a19ee3e....
Include Cleanup
include/depthai/pipeline/node/ToF.hpp
Removed unused <fstream> include directive.
Rectification Logic
src/pipeline/node/Rectification.cpp
Removed host platform validation logic; added EEPROM change detection to trigger reinitialization of rectification matrices when EEPROM ID changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A version hops along so new,
Unused includes we bid adieu,
EEPROM whispers change is near,
Matrices reinit, crystal clear! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Enable dynamic calibration for NeuralDepth and Rectification nodes' directly corresponds to the main objective of the PR: updating NeuralDepth and Rectification nodes to handle dynamic calibration updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch lnotspotl/neural_depth_dynamic_calib
📝 Coding Plan for PR comments
  • Generate coding plan

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/pipeline/node/Rectification.cpp (1)

109-145: ⚠️ Potential issue | 🟠 Major

Track calibration changes from the same source you apply.

Lines 109-145 watch getParentPipeline().getEepromId(), but Line 150 rebuilds maps from getCalibrationData(), which switches to device->readCalibration() when device is present. In that path, a device-side calibration update can leave the pipeline EEPROM ID unchanged, so initialized never resets and rectification keeps using stale maps. Please derive the change token from the same calibration source used for R/T.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pipeline/node/Rectification.cpp` around lines 109 - 145, The code
currently uses getParentPipeline().getEepromId()
(latestEepromId/currentEepromId) to decide when to reset initialized, but R/T
and map rebuilding use getCalibrationData() (which may call
device->readCalibration()), so change detection must use the same calibration
source: replace or supplement the EEPROM-based token with a token derived from
getCalibrationData() (e.g., a calibration version/id field or a stable hash of
the calibration blob returned by getCalibrationData()/device->readCalibration())
and compare that token each loop to set initialized = false when it changes;
update references to latestEepromId/currentEepromId to use this calibrationToken
and ensure the token is computed before map rebuilding logic in
Rectification.cpp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/pipeline/node/Rectification.cpp`:
- Around line 109-145: The code currently uses getParentPipeline().getEepromId()
(latestEepromId/currentEepromId) to decide when to reset initialized, but R/T
and map rebuilding use getCalibrationData() (which may call
device->readCalibration()), so change detection must use the same calibration
source: replace or supplement the EEPROM-based token with a token derived from
getCalibrationData() (e.g., a calibration version/id field or a stable hash of
the calibration blob returned by getCalibrationData()/device->readCalibration())
and compare that token each loop to set initialized = false when it changes;
update references to latestEepromId/currentEepromId to use this calibrationToken
and ensure the token is computed before map rebuilding logic in
Rectification.cpp.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3fec1c7f-944b-4700-b743-de60912c9ac5

📥 Commits

Reviewing files that changed from the base of the PR and between fa5658a and eb9180b.

📒 Files selected for processing (3)
  • cmake/Depthai/DepthaiDeviceRVC4Config.cmake
  • include/depthai/pipeline/node/ToF.hpp
  • src/pipeline/node/Rectification.cpp
💤 Files with no reviewable changes (1)
  • include/depthai/pipeline/node/ToF.hpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testable PR is ready to be tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants