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
L515 - Keep USB power on when multiple HWM calls made #8593
L515 - Keep USB power on when multiple HWM calls made #8593
Conversation
288bbd0
to
92e977c
Compare
92e977c
to
571b0ee
Compare
src/l500/l500-color.cpp
Outdated
_owner->_hw_monitor->send(cmdTprocThresholdEp5); | ||
|
||
// Keep the USB power on while triggering multiple calls on it. | ||
_owner->get_raw_depth_sensor().invoke_powered([&](platform::uvc_device& dev) { |
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 don't like manual access to _owner->get_raw_depth_sensor()
-- it might change, and we might want to do other stuff in the future.
I think we should add a utility function that inside will know how to raise power... group_hw_monitor_calls
or something along those lines.
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
src/l500/l500-serializable.cpp
Outdated
to_string() << "Failed to get raw depth sensor from serializable device" ); | ||
|
||
|
||
return l500_uvc_sensor->invoke_powered([&](platform::uvc_device& dev) { |
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.
Indentation off?
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.
Fixed
src/l500/l500-serializable.cpp
Outdated
auto l500_uvc_sensor = std::dynamic_pointer_cast<uvc_sensor>(_depth_sensor.get_raw_sensor()); | ||
if (!l500_uvc_sensor) | ||
throw invalid_value_exception( | ||
to_string() << "Failed to get raw depth sensor from serializable device" ); |
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 the exception isn't needed -- just don't call invoke_powered?
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.
Up to you
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.
That also mean that we don't do the action, invoke power get the inner code and run it
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 can call the lambda manually.
But up to you.
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.
This code was removed
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.
For internal review. please do not merge yet
dev = test.find_first_device_or_exit() | ||
|
||
product_line = "Unknown" | ||
if dev.supports(rs.camera_info.product_line): |
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 can pretty much require that the device supports a product line, no? I think you can just skip this test...
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.
Changed
// therefore can be overridden automatically according to gain | ||
if( preset == RS2_L500_VISUAL_PRESET_AUTOMATIC ) | ||
{ | ||
auto curr_preset = ( rs2_l500_visual_preset )(int)_preset->query(); |
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.
This doesn't actually go to the FW... the grouping can happen only inside reset_hw_controls
?
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 wrapped the whole function because there are several call to the FW, right after _preset->query();
change_gain
change_alt_ir
set_preset_controls_to_defaults
change_laser_power
Reduce L515 device creation and sensor open/start calls time by keeping the USB power on when multiple access are made
Tracked on [RS5-8208]