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

Make uvc sensor weak pointer #12484

Merged
merged 3 commits into from
Dec 10, 2023

Conversation

Tamir91
Copy link
Contributor

@Tamir91 Tamir91 commented Dec 5, 2023

Tracked on [RSDSO-19355]

@@ -584,6 +584,8 @@ namespace librealsense
auto& depth_sensor = get_depth_sensor();
auto& raw_depth_sensor = get_raw_depth_sensor();

//_raw_depth_sensor2 = dynamic_cast<>.get_raw_sensor();
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment deleted

@@ -681,7 +683,7 @@ namespace librealsense
"Generate trigger from the camera to external device once per frame"));

depth_sensor.register_option(RS2_OPTION_ASIC_TEMPERATURE,
std::make_shared<asic_and_projector_temperature_options>(raw_depth_sensor,
std::make_shared<asic_and_projector_temperature_options>(std::dynamic_pointer_cast<uvc_sensor>(raw_depth_sensor.shared_from_this()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Coverity will yell here,
You know how to solve it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Null checking added.

@@ -1298,7 +1300,7 @@ namespace librealsense
std::vector<float>{0.f, 2.f}, 1.f));

depth_ep.register_option(RS2_OPTION_PROJECTOR_TEMPERATURE,
std::make_shared<asic_and_projector_temperature_options>(depth_ep,
std::make_shared<asic_and_projector_temperature_options>(std::dynamic_pointer_cast<uvc_sensor>(depth_ep.shared_from_this()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Null checking added.

@@ -73,7 +73,7 @@ namespace librealsense
else
{
_depth_ep.register_option(RS2_OPTION_PROJECTOR_TEMPERATURE,
std::make_shared<asic_and_projector_temperature_options>(_raw_depth_ep,
std::make_shared<asic_and_projector_temperature_options>(std::dynamic_pointer_cast<uvc_sensor>(_raw_depth_ep.shared_from_this()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Null checking added.

auto temperature_data = static_cast<temperature>(_ep.invoke_powered(
auto strong_ep = _ep.lock();
if (!strong_ep)
throw invalid_value_exception(rsutils::string::from() << "EP is not exists");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Who will catch this exception?
BTW
EP does not exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During our previous conversation, we agreed to leave the use of the throw.
The string has been renamed.

return _ep.is_streaming();
auto strong_ep = _ep.lock();
if (!strong_ep)
throw invalid_value_exception(rsutils::string::from() << "Can not retriev temperature as no valid EP");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of throw, maybe we should return false and log_warning this issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During our previous conversation, we agreed to leave the use of the throw.

Checking dynamic casts
Exception type changed
@Tamir91 Tamir91 requested a review from Nir-Az December 7, 2023 08:28
Copy link
Collaborator

@Nir-Az Nir-Az left a comment

Choose a reason for hiding this comment

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

LGTM

@Nir-Az Nir-Az merged commit 59ffb92 into IntelRealSense:development Dec 10, 2023
16 of 17 checks passed
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.

2 participants