Housing calibration in CalibrationHandler + tests#1602
Conversation
pheec
force-pushed
the
dev_tf_HousingHandler
branch
from
fb4b9ba to
ad6111c
Compare
pheec
force-pushed
the
dev_tf_HousingHandler
branch
2 times, most recently
from
eb8935e to
4c150a0
Compare
MaticTonin
left a comment
MaticTonin
left a comment
There was a problem hiding this comment.
Looks fine to me, some small nitpicks and comments regarding tests mostly,.
| // Helper function to compare 4x4 transformation matrices | ||
| static void requireMatrixApproxEqual(const std::vector<std::vector<float>>& result, | ||
| const std::vector<std::vector<float>>& expected, | ||
| float margin = 1e-3) { |
There was a problem hiding this comment.
Lets try higher margin, -4 or -5.
| } | ||
|
|
||
| // Helper to create calibration data with housing and product name | ||
| static CalibrationHandler loadHandlerWithHousing() { |
There was a problem hiding this comment.
Handlers on top of the file, not in the middle of file
|
|
||
|
|
||
| // Helper to create calibration data directly in code | ||
| static CalibrationHandler loadHandlerWithHousingRotation() { |
There was a problem hiding this comment.
Same nitpick, on top of file
| return CalibrationHandler::fromJson(calibJson); | ||
| } | ||
|
|
||
| TEST_CASE("getHousingCalibration - T_CAM_C_to_housing with specTranslation", "[getHousingCalibration I]") { |
There was a problem hiding this comment.
Combine I and II and III into one test. where there is one with regular translation and one with specTranslation.
In this case, rename to expected variable as needed
| requireMatrixApproxEqual(camToHousing, expected, 1e-6); | ||
| } | ||
|
|
||
| TEST_CASE("getHousingCalibration - T_CAM_A_to_housing with regular translation", "[getHousingCalibration III-B]") { |
This comment was marked as resolved.
This comment was marked as resolved.
|
And as well check for line 495 in https://github.com/luxonis/depthai-core/actions/runs/20720798482/job/59485200316?pr=1602#step:3:496 and why would the test be failing. |
|
Claude AI: Actual Failure: SSH/Docker Connection This is a CI infrastructure issue, not a code/test issue. The SSH connection to the HIL (Hardware-in-Loop) test runner at 10.12.143.133 failed during cleanup. |
|
style check failing due to the issue in develop branch, will be resolved #1603. |
|
The test is failing because the compilation is missing the flag -DDEPTHAI_HAVE_HOUSING_COORDINATES=ON. This is actually expected behavior: the CalibrationHandler first checks whether the translation file exists, and if not, it falls back to values from the calib file. In this test, we want to verify that the translation values in the generated file are correct, but since the file is never generated, the test fails. I suggest renaming this flag from DEPTHAI_HAVE_HOUSING_COORDINATES to DEPTHAI_GENERATE_HOUSING_COORDS, and setting it to ON by default. This would align the behavior with the test’s intention. |
pheec
force-pushed
the
dev_tf_HousingHandler
branch
from
eb46fcc to
8471935
Compare
|
I did the discussed changes of the flag. Rdy for testing again |
MaticTonin
added
testable
|
Before the merge, the depthai_boards must be updated to main, so that we do not have detached branch. |
4 participants
Purpose
The housing calibration, which is stored in the eeprom, can now be handled using the CalibrationHandler.
Testing & Validation
The methods are tested in integration tests. We also conducted a successful experimental depth test on calibration rig.