Repository navigation
enable calib for D457 and advanced tests - #14672
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enables calibration support for the D457 device and re-enables advanced calibration tests that were previously disabled. The changes add default calibration parameters to the JSON configurations and uncomment test code that was temporarily disabled for lab stabilization.
Changes:
- Added default calibration parameters (speed, step count, accuracy, etc.) to tare and on-chip calibration JSON configurations
- Re-enabled advanced calibration tests by uncommenting test code blocks
- Fixed a spelling error and adjusted test thresholds for improved calibration validation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| unit-tests/live/calib/test-tare-calibrations.py | Added default calibration parameters to tare calibration JSON |
| unit-tests/live/calib/test-occ-calibrations.py | Added default calibration parameters and uncommented test code |
| unit-tests/live/calib/test-advanced-tare-calibrations.py | Fixed spelling error in comment |
| unit-tests/live/calib/test-advanced-occ-calibrations.py | Updated test device filter, adjusted thresholds, simplified retry logic, and uncommented test code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| temprorary disabled on mipi devices to stabilize the lab | ||
|
|
||
| # temprorary disabled on mipi devices to stabilize the lab |
There was a problem hiding this comment.
Corrected spelling of 'temprorary' to 'temporarily'.
| # temprorary disabled on mipi devices to stabilize the lab | |
| # temporarily disabled on mipi devices to stabilize the lab |
| PIXEL_CORRECTION = -1.0 # pixel shift to apply to principal point | ||
| EPSILON = 0.001 # distance comparison tolerance | ||
| EPSILON = 0.5 # half of PIXEL_CORRECTION tolerance | ||
| DIFF_THRESHOLD = 0.001 # minimum change expected after OCC calibration |
There was a problem hiding this comment.
The comment describes this as 'minimum change expected after OCC calibration', but the variable name and usage at line 193 suggest it represents the threshold for detecting meaningful changes. Consider clarifying whether this is the minimum expected change or the threshold below which changes are considered insignificant.
| DIFF_THRESHOLD = 0.001 # minimum change expected after OCC calibration | |
| DIFF_THRESHOLD = 0.001 # threshold below which OCC-induced changes are considered insignificant |
| modified_axis_val = mod_right_pp[1] | ||
| returned_modified_axis_val = modified_ppy if modify_ppy else modified_ppx | ||
| if abs(modified_axis_val - returned_modified_axis_val) > EPSILON: | ||
| if abs(modified_axis_val - returned_modified_axis_val) > DIFF_THRESHOLD: # verify that we fix at least 1/2 of modification |
| """ | ||
| temprorary disabled on mipi devices to stabilize the lab | ||
|
|
||
| # temporarily disabled on mipi devices to stabilize the lab |
Nir-Az
left a comment
Nir-Az
left a comment
There was a problem hiding this comment.
Approved with a few small minor comments
No description provided.