-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add Reflectivity option restrictions #7706
Add Reflectivity option restrictions #7706
Conversation
src/l500/l500-options.cpp
Outdated
@@ -226,6 +226,8 @@ namespace librealsense | |||
|
|||
void l500_options::on_set_option(rs2_option opt, float value) | |||
{ | |||
verify_max_usable_range_restrictions(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opt
may not be VISUAL_PRESET
... but inside the verify
fn you compare the value to MAX_RANGE
...? Am I misunderstanding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, the logic was not good.
Changed.
|
||
if (ds.supports_option(RS2_OPTION_ENABLE_IR_REFLECTIVITY) && ds.get_option(RS2_OPTION_ENABLE_IR_REFLECTIVITY).query() == 1.0f) | ||
{ | ||
ds.get_option(RS2_OPTION_ENABLE_IR_REFLECTIVITY).set(0.0f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this only after the other test passes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We leave it as is as we discussed, we changed the throw to set MUR off
src/l500/l500-options.cpp
Outdated
(ds.get_option(RS2_OPTION_ENABLE_MAX_USABLE_RANGE).query() == 1.0) && | ||
(value != rs2_sensor_mode::RS2_SENSOR_MODE_VGA)) | ||
{ | ||
throw wrong_api_call_sequence_exception("Max Usable Range support VGA resolution only"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should turn it off, not throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
common/model-views.cpp
Outdated
{ | ||
int sensor_mode = static_cast<int>(s->get_option(RS2_OPTION_SENSOR_MODE)); | ||
int width = 0, height = 0; | ||
width_height_from_resolution(static_cast<rs2_sensor_mode>(sensor_mode), width, height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add function resolution_id_from_sensor_mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
common/model-views.cpp
Outdated
@@ -76,6 +76,27 @@ static void width_height_from_resolution(rs2_sensor_mode mode, int &width, int & | |||
} | |||
} | |||
|
|||
static int get_resolution_id_from_sensor_mode(int sensor_mode, const rs2::sensor &s, const std::vector<std::pair<int, int>> &res_values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pass in rs2_sensor_mode
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
common/model-views.cpp
Outdated
{ | ||
int width = 0, height = 0; | ||
width_height_from_resolution(static_cast<rs2_sensor_mode>(sensor_mode), width, height); | ||
auto iter = std::find_if(res_values.begin(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/l500/l500-options.cpp
Outdated
if( ds.supports_option( RS2_OPTION_ENABLE_IR_REFLECTIVITY ) | ||
&& ds.get_option( RS2_OPTION_ENABLE_IR_REFLECTIVITY ).query() == 1.0f ) | ||
throw librealsense::wrong_api_call_sequence_exception( | ||
"Enabling Alternate IR while IR Reflectivity is enabled is prohibited" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternate IR cannot be enabled with IR Reflectivity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Implement IR Reflectivity option restrictions according to [RS5-8358]
Note: