Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added API for depth auto calibration. #5346

Merged
merged 5 commits into from
Dec 2, 2019

Conversation

aangerma
Copy link
Contributor

Added auto_calibrated_device extension for on-chip (plane fit RMS) and tare calibration (absolute distance).
Use this API in viewer and added example on python for auto calibration.
Fixed viewer to put the default resolution on top and fixed default resolution in RS435.

aangerma and others added 2 commits November 28, 2019 17:42
@@ -173,6 +173,7 @@ typedef enum rs2_extension
RS2_EXTENSION_UPDATE_DEVICE,
RS2_EXTENSION_L500_DEPTH_SENSOR,
RS2_EXTENSION_TM2_SENSOR,
RS2_EXTENSION_AUTO_CALIBRARED_DEVICE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Calibrated

}

std::shared_ptr<ds5_advanced_mode_base> preset_recover;
if (_speed == 4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace hard-coded values with enums

// Begin auto-calibration
_hw_monitor->send(command{ ds::AUTO_CALIB, auto_calib_begin, _speed });

DirectSearchCalibrationResult result;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be simplified with {}

src/ds5/ds5-auto-calibration.cpp Show resolved Hide resolved
src/ds5/ds5-auto-calibration.cpp Show resolved Hide resolved
src/rs.cpp Show resolved Hide resolved
wrappers/python/pyrs_device.cpp Outdated Show resolved Hide resolved
wrappers/python/pyrs_device.cpp Outdated Show resolved Hide resolved
wrappers/python/pyrs_device.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Add Jira reference

src/ds5/ds5-auto-calibration.h Outdated Show resolved Hide resolved
src/ds5/ds5-auto-calibration.h Outdated Show resolved Hide resolved
src/ds5/ds5-auto-calibration.h Outdated Show resolved Hide resolved
* \param[in] callback callback to get progress notifications
* \return new calibration table
* \param[in] callback Callback to get progress notifications
* \return Nnew calibration table
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nnew

include/librealsense2/hpp/rs_device.hpp Outdated Show resolved Hide resolved
include/librealsense2/hpp/rs_device.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Maybe it is better for now to extract the refactoring and deal with it in a separate PR

@@ -16,6 +16,7 @@ target_sources(${LRS_TARGET}
"${CMAKE_CURRENT_LIST_DIR}/advanced_mode/presets.cpp"
"${CMAKE_CURRENT_LIST_DIR}/advanced_mode/advanced_mode.cpp"
"${CMAKE_CURRENT_LIST_DIR}/ds5-auto-calibration.cpp"
"${CMAKE_CURRENT_LIST_DIR}/ds5-wide-fov.cpp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tabs
I strongly recommend to rename the class to ov9282_depth or similar - wide-fov descriptive and not unique given the other SKUs to consider. In addition it doesn't allow for scaling and reuse

@@ -28,4 +29,5 @@ target_sources(${LRS_TARGET}
"${CMAKE_CURRENT_LIST_DIR}/ds5-fw-update-device.h"
"${CMAKE_CURRENT_LIST_DIR}/advanced_mode/json_loader.hpp"
"${CMAKE_CURRENT_LIST_DIR}/ds5-auto-calibration.h"
"${CMAKE_CURRENT_LIST_DIR}/ds5-wide-fov.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tabs + rename

const int DEFAULT_AVERAGE_STEP_COUNT = 20;
const int DEFAULT_STEP_COUNT = 10;
const int DEFAULT_ACCURACY = 2;
const int DEFAULT_SPEED = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

= auto_calib_speed::speed_slow;

const int DEFUALT_SPEED = 3;
const int DEFAULT_AVERAGE_STEP_COUNT = 20;
const int DEFAULT_STEP_COUNT = 10;
const int DEFAULT_ACCURACY = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

= subpixel_accuracy::speed_medium;

src/ds5/ds5-auto-calibration.h Show resolved Hide resolved
auto usb_spec = get_usb_spec();
if (usb_spec >= platform::usb3_type || usb_spec == platform::usb_undefined)
{
tags.push_back({ RS2_STREAM_COLOR, -1, 640, 480, RS2_FORMAT_RGB8, 30, profile_tag::PROFILE_TAG_SUPERSET | profile_tag::PROFILE_TAG_DEFAULT });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should we reduce 720P to VGA for USB3?

}
return tags;
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

EOF is missing

{
std::vector<tagged_profile> tags;
auto usb_spec = get_usb_spec();
if (usb_spec >= platform::usb3_type || usb_spec == platform::usb_undefined)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You also can use the ds5-factory.cpp method for maintainability and clearness:

 bool usb3mode = (usb_spec >= platform::usb3_type || usb_spec == platform::usb_undefined);
uint32_t width  = usb3mode ?  1280 : 640;
uint32_t height = usb3mode ?   720 : 480;
uint32_t fps    = usb3mode ?    30 :  15;

tags.push_back({ RS2_STREAM_COLOR, -1, width, height, RS2_FORMAT_RGB8, fps,...

@@ -0,0 +1,21 @@
#include "ds5-wide-fov.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing copyright

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Approved with the reservation for wrappers.
Remove ds5-wide-fov class and files

@ev-mp ev-mp merged commit 93780b6 into IntelRealSense:development Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants