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

Min Distance can't get higher value than the Max Distance #6564

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

aseelegbaria
Copy link
Contributor

@aseelegbaria aseelegbaria commented Jun 10, 2020

Min Distance can't get higher value than the Max Distance in Threshold Filter and Depth Visualization
Tracked on: DSO-12346, DSO-12163, RS5-8689

src/rs.cpp Outdated
@@ -534,6 +534,21 @@ void rs2_set_option(const rs2_options* options, rs2_option option, float value,
VALIDATE_NOT_NULL(options);
VALIDATE_OPTION(options, option);
options->options->get_option(option).set(value);

Copy link
Collaborator

Choose a reason for hiding this comment

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

The logic should be dowstreamed from the API level to the core class, in a way that resembles the Auto-exposure/manual gain relationship.

src/rs.cpp Outdated

if (option == RS2_OPTION_MIN_DISTANCE) {
float max_val = options->options->get_option(RS2_OPTION_MAX_DISTANCE).query();
if (max_val < value) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This flow will result in having min=max=val. It is not clear that this is better than throwing exception

src/option.h Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved
src/option.h Outdated

/** \brief class provided a control
* that changes min distance when changing the max distance value */
class max_distance_control : public proxy_option
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we call these options and not controls?

src/proc/threshold.cpp Outdated Show resolved Hide resolved
src/proc/colorizer.cpp Outdated Show resolved Hide resolved
src/option.h Outdated Show resolved Hide resolved

void enable_recording(std::function<void(const option &)> record_action) override
{
_recording_function = record_action;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this call _proxy->enable_recording(...)?

I'm OK leaving it as-is, since that's the way it was before, but I'll get @ev-mp input...

@ev-mp ev-mp merged commit e3b7c6d into IntelRealSense:development Sep 16, 2020
@aseelegbaria aseelegbaria deleted the distanceBug branch May 2, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants