Skip to content

Emitter On/Off - #2829

Merged
dorodnic merged 5 commits into
realsenseai:developmentfrom
dorodnic:daversintel-emitter_onoff_and_intrinsics
Dec 9, 2018
Merged

dorodnic merged 5 commits into
realsenseai:developmentfrom
dorodnic:daversintel-emitter_onoff_and_intrinsics

Conversation

@dorodnic

@dorodnic dorodnic commented Dec 3, 2018

Copy link
Copy Markdown
Contributor

Based on #2699

@daversintel please review and comment if my proposed changes are acceptable:

  1. I prefer not to add changes to RealSense Viewer in this PR. I feel we need to better understand how this feature is going to be used before creating a dedicated UI or processing-block.
  2. I added experimental predicate on firmware_version, to for now only allow this feature for firmware versions that support it.
  3. I propose to just call the option RS2_OPTION_EMITTER_ON_OFF for simplicity.

@dorodnic
dorodnic changed the base branch from master to development December 3, 2018 14:07
@daversintel

Copy link
Copy Markdown
Contributor

The changes are OK with me. But it still better to show it with the viewer.

Comment thread src/ds5/ds5-options.cpp Outdated
command cmd(ds::GET_PWM_ON_OFF);
auto res = _hwm.send(cmd);
if (res.empty())
throw invalid_value_exception("external_sync_mode::query result is empty!");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C&P

Comment thread src/ds5/ds5-private.cpp
else
{
// Intrinsics not found in the calibration table - use the generic calculation
ds5_rect_resolutions resolution = res_1920_1080;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The generic part assumes the sensor uses FullHd for calibration with 16/9 aspect. But for other sensors, e.g. AWGC 1MP with 1280X800 16/10 aspect ratio it will produce distortions during uv mapping, for instance.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can add the code to validate entry for res_1920_1080 and throw exception if the entry is not used for calibration or not valid.

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