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
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/ds/d400/d400-device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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


using namespace platform;

// minimal firmware version in which hdr feature is supported
Expand Down Expand Up @@ -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.

RS2_OPTION_ASIC_TEMPERATURE));

// D457 dev - get_xu fails for D457 - error polling id not defined
Expand Down Expand Up @@ -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.

RS2_OPTION_PROJECTOR_TEMPERATURE));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/ds/ds-active-common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.

RS2_OPTION_PROJECTOR_TEMPERATURE));
}
}
Expand Down
28 changes: 21 additions & 7 deletions src/ds/ds-options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "ds-options.h"
#include <src/hid-sensor.h>
#include <memory>



namespace librealsense
Expand Down Expand Up @@ -53,7 +55,11 @@ namespace librealsense
};
#pragma pack(pop)

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.


auto temperature_data = static_cast<temperature>(strong_ep->invoke_powered(
[this](platform::uvc_device& dev)
{
temperature temp{};
Expand Down Expand Up @@ -82,11 +88,11 @@ namespace librealsense
is_valid_field = &temperature::is_projector_valid;
break;
default:
throw invalid_value_exception(rsutils::string::from() << _ep.get_option_name(_option) << " is not temperature option!");
throw invalid_value_exception(rsutils::string::from() << strong_ep->get_option_name(_option) << " is not temperature option!");
}

if (0 == temperature_data.*is_valid_field)
LOG_ERROR(_ep.get_option_name(_option) << " value is not valid!");
LOG_ERROR(strong_ep->get_option_name(_option) << " value is not valid!");

return temperature_data.*field;
}
Expand All @@ -98,24 +104,32 @@ namespace librealsense

bool asic_and_projector_temperature_options::is_enabled() const
{
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.


return strong_ep->is_streaming();
}

const char* asic_and_projector_temperature_options::get_description() const
{
auto strong_ep = _ep.lock();
if (! strong_ep)
Nir-Az marked this conversation as resolved.
Show resolved Hide resolved
throw invalid_value_exception(rsutils::string::from() << "EP is not exists");

switch (_option)
{
case RS2_OPTION_ASIC_TEMPERATURE:
return "Current Asic Temperature (degree celsius)";
case RS2_OPTION_PROJECTOR_TEMPERATURE:
return "Current Projector Temperature (degree celsius)";
default:
throw invalid_value_exception(rsutils::string::from() << _ep.get_option_name(_option) << " is not temperature option!");
throw invalid_value_exception(rsutils::string::from() << strong_ep->get_option_name(_option) << " is not temperature option!");
}
}

asic_and_projector_temperature_options::asic_and_projector_temperature_options(uvc_sensor& ep, rs2_option opt)
: _option(opt), _ep(ep)
asic_and_projector_temperature_options::asic_and_projector_temperature_options(std::shared_ptr<uvc_sensor> && ep, rs2_option opt)
: _option(opt), _ep(std::move(ep))
Nir-Az marked this conversation as resolved.
Show resolved Hide resolved
{}

float motion_module_temperature_option::query() const
Expand Down
4 changes: 2 additions & 2 deletions src/ds/ds-options.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ namespace librealsense

const char* get_description() const override;

explicit asic_and_projector_temperature_options(uvc_sensor& ep, rs2_option opt);
explicit asic_and_projector_temperature_options(std::shared_ptr <uvc_sensor> && ep, rs2_option opt);

private:
uvc_sensor& _ep;
std::weak_ptr<uvc_sensor> _ep;
rs2_option _option;
};

Expand Down
Loading