Fix for D435i loosing RGB-depth extrinsic calibration after firmware update - #3976
Conversation
ev-mp
left a comment
There was a problem hiding this comment.
Hi, pls review the changes.
Imo the D435 class shall be checked as well.
| { | ||
| for (auto j = 0; j < 3; i++) | ||
| { | ||
| if (rect_rot_mat(i, j) != 0) |
| float3 trans_vector = table->translation_rect; | ||
| float3x3 rect_rot_mat = table->rotation_matrix_rect; | ||
|
|
||
| for (auto i = 0; i < 3; i++) |
There was a problem hiding this comment.
Pls add a description of the heuristic
| environment::get_instance().get_extrinsics_graph().register_extrinsics(*_color_stream, *_depth_stream, _color_extrinsic); | ||
| } | ||
|
|
||
| bool try_restore_color_stream_extrinsic_from_asr_area(const int address) |
There was a problem hiding this comment.
Remove asr from the signature and rename into ..restore_from_address(..
Make the address uint to avoid negatives
| } | ||
| } | ||
|
|
||
| void restore_color_stream_extrinsic(std::vector<byte> calib) |
|
|
||
| if (!try_restore_color_stream_extrinsic_from_gold_sector()) | ||
| { | ||
| LOG_WARNING("The calibration on original 'gold' table is not valid"); |
There was a problem hiding this comment.
Pls change the wording to
"RGB extrinsic recovery - Gold calibration is invalid, restore from an alternative sector"
|
|
||
| if (!try_restore_color_stream_extrinsic_from_asr_area(gold_address)) | ||
| { | ||
| LOG_WARNING("The calibration on 'gold' table is not valid"); |
There was a problem hiding this comment.
Please adjust the log messages for consistency
|
|
||
| if (!try_restore_color_stream_extrinsic_from_asr_area(dynamic_address)) | ||
| { | ||
| LOG_WARNING("The calibration on 'dynamic' table is not valid"); |
There was a problem hiding this comment.
Add a warning that the recovery routine failed.
Also need to update the extrinsic graph so that color-depth will be removed (rather than zeroed)
| auto calib = ds5_device::_hw_monitor->send(cmd); | ||
| if (validate_color_stream_extrinsic(calib)) | ||
| { | ||
| restore_color_stream_extrinsic(calib); |
There was a problem hiding this comment.
Add the return value check and propagate it back to the caller
|
|
||
| void try_restore_color_stream_extrinsic() | ||
| { | ||
| const int gold_address = 0x17c49c; |
There was a problem hiding this comment.
Pls add a global try/catch to protect against FW API changes
| { | ||
| try | ||
| { | ||
| // check that data is valid |
There was a problem hiding this comment.
check that that data is valid verify extrinsic calibration table structure
| for (auto i = 0; i < 3; i++) | ||
| { | ||
| if (trans_vector[i] != 0) | ||
| if (std::fabs(trans_vector[i] - 0) > std::numeric_limits<double>::epsilon()) |
There was a problem hiding this comment.
Drop the ..-0) -
Types are not consistent - must compare to < float >
| restore_color_stream_extrinsic(calib); | ||
| return true; | ||
| } | ||
| LOG_WARNING("Restore from address" << address << " faild"); |
There was a problem hiding this comment.
"Restore from address" Restoring RGB Extrinsic
typo - failed
| restore_color_stream_extrinsic(calib); | ||
| return true; | ||
| } | ||
| LOG_WARNING("Restore from gold_sector faild"); |
| try | ||
| { | ||
| LOG_WARNING("The calibration on original 'gold' table is not valid"); | ||
| LOG_INFO("invalid extrinsic was identified, recovery routine was invoked"); |
There was a problem hiding this comment.
Pls raise to LOG_WARNING
also add RGB before extrinsic
| } | ||
| catch (...) | ||
| { | ||
| LOG_WARNING("Recovery routine failed"); |
There was a problem hiding this comment.
RGB Extrinsic recovery routine failed
| } | ||
| } | ||
|
|
||
| LOG_DEBUG("Suceeded to restore color stream extrinsic"); |
There was a problem hiding this comment.
Raise the level to LOG_INFO
RGB extrinsic restored
| //write the calibration to its correct address | ||
| command cmd(ds::fw_cmd::SETINTCALNEW, 0x20, 0x2); | ||
| cmd.data = calib; | ||
| auto res = ds5_device::_hw_monitor->send(cmd); |
| auto calib = ds5_device::_hw_monitor->send(cmd); | ||
| if (validate_color_stream_extrinsic(calib)) | ||
| { | ||
| restore_color_stream_extrinsic(calib); |
| { | ||
| LOG_WARNING("invalid RGB extrinsic was identified, recovery routine was invoked"); | ||
|
|
||
| const uint32_t gold_address = 0x17c49c; |
There was a problem hiding this comment.
Add comment explaining what these are
# Conflicts: # src/ds5/ds5-private.h
Fix FW calibration tables location that potentially disable alignment/texture mapping use-cases with D435i model.
The affected FW versions generate the RGB calibration table in the memory reserved for another SKU.
This PR provides a rectification method that restores the RGB calibration data on the affected units using either factory or improperly-located data sets.
This method is invoked and applied automatically on the device’s start and does not require user actions.
Addresses #3474, #3788, #3201
Tracked on: DSO-12623