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.

Sign up for GitHub

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

Add new get_value API for Handles and Interfaces #1976

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add new get_value API for Handles and Interfaces #1976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f2be0f9
Add new get_value API for handles and interfaces
saikishor Jan 2, 2025
37964b7
Merge branch 'ros-controls:master' into add_new/get_value/api
saikishor Jan 2, 2025
169ba83
add minor fixes
saikishor Jan 2, 2025
a01be64
update the other get_value method
saikishor Jan 2, 2025
4403195
Fix tests
saikishor Jan 2, 2025
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
35 changes: 31 additions & 4 deletions hardware_interface/include/hardware_interface/handle.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <limits>
#include <memory>
#include <mutex>
#include <optional>
#include <shared_mutex>
#include <string>
#include <utility>
Expand Down Expand Up @@ -142,7 +143,9 @@ class Handle

const std::string & get_prefix_name() const { return prefix_name_; }

[[deprecated("Use bool get_value(double & value) instead to retrieve the value.")]]
[[deprecated(
"Use std::optional<T> get_value() or T get_value(bool &status) or bool get_value(double & "
"value) instead to retrieve the value.")]]
double get_value() const
{
std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
Expand All @@ -157,7 +160,32 @@ class Handle
// END
}

[[nodiscard]] bool get_value(double & value) const
template <typename T = double>
[[nodiscard]] std::optional<T> get_value() const
{
std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
if (!lock.owns_lock())
{
return std::nullopt;
}
return value_ptr_ != nullptr ? *value_ptr_ : std::get<T>(value_);
}

template <typename T = double>
[[nodiscard]] T get_value(bool & status) const
{
std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
if (!lock.owns_lock())
{
status = false;
return T();
}
status = true;
return value_ptr_ != nullptr ? *value_ptr_ : std::get<T>(value_);
}

template <typename T>
[[nodiscard]] bool get_value(T & value) const
{
std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
if (!lock.owns_lock())
Expand All @@ -166,8 +194,7 @@ class Handle
}
// BEGIN (Handle export change): for backward compatibility
// TODO(Manuel) set value directly if old functionality is removed
THROW_ON_NULLPTR(value_ptr_);
value = *value_ptr_;
value = value_ptr_ != nullptr ? *value_ptr_ : std::get<T>(value_);
return true;
// END
}
Expand Down
48 changes: 47 additions & 1 deletion hardware_interface/include/hardware_interface/loaned_command_interface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,12 @@ class LoanedCommandInterface
return true;
}

[[deprecated(
"Use std::optional<T> get_value() or T get_value(bool &status) or bool get_value(double & "
"value) instead to retrieve the value.")]]
double get_value() const
{
double value;
double value = std::numeric_limits<double>::quiet_NaN();
if (get_value(value))
{
return value;
Expand All @@ -126,6 +129,49 @@ class LoanedCommandInterface
}
}

template <typename T = double>
[[nodiscard]] std::optional<T> get_value(unsigned int max_tries = 10) const
{
unsigned int nr_tries = 0;
do
{
++get_value_statistics_.total_counter;
const std::optional<T> data = command_interface_.get_value<T>();
if (data.has_value())
{
return data;
}
++get_value_statistics_.failed_counter;
++nr_tries;
std::this_thread::yield();
} while (nr_tries < max_tries);

++get_value_statistics_.timeout_counter;
return std::nullopt;
}

template <typename T = double>
[[nodiscard]] T get_value(bool & status, unsigned int max_tries = 10) const
{
unsigned int nr_tries = 0;
do
{
status = false;
++get_value_statistics_.total_counter;
const T data = command_interface_.get_value<T>(status);
if (status)
{
return data;
}
++get_value_statistics_.failed_counter;
++nr_tries;
std::this_thread::yield();
} while (nr_tries < max_tries);

++get_value_statistics_.timeout_counter;
return T();
}

template <typename T>
[[nodiscard]] bool get_value(T & value, unsigned int max_tries = 10) const
{
Expand Down
48 changes: 47 additions & 1 deletion hardware_interface/include/hardware_interface/loaned_state_interface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ class LoanedStateInterface

const std::string & get_prefix_name() const { return state_interface_.get_prefix_name(); }

[[deprecated(
"Use std::optional<T> get_value() or T get_value(bool &status) or bool get_value(double & "
"value) instead to retrieve the value.")]]
double get_value() const
{
double value;
double value = std::numeric_limits<double>::quiet_NaN();
if (get_value(value))
{
return value;
Expand All @@ -101,6 +104,49 @@ class LoanedStateInterface
}
}

template <typename T = double>
[[nodiscard]] std::optional<T> get_value(unsigned int max_tries = 10) const
{
unsigned int nr_tries = 0;
do
{
++get_value_statistics_.total_counter;
const std::optional<T> data = state_interface_.get_value<T>();
if (data.has_value())
{
return data;
}
++get_value_statistics_.failed_counter;
++nr_tries;
std::this_thread::yield();
} while (nr_tries < max_tries);

++get_value_statistics_.timeout_counter;
return std::nullopt;
}

template <typename T = double>
[[nodiscard]] T get_value(bool & status, unsigned int max_tries = 10) const
{
unsigned int nr_tries = 0;
do
{
status = false;
++get_value_statistics_.total_counter;
const T data = state_interface_.get_value<T>(status);
if (status)
{
return data;
}
++get_value_statistics_.failed_counter;
++nr_tries;
std::this_thread::yield();
} while (nr_tries < max_tries);

++get_value_statistics_.timeout_counter;
return T();
}

template <typename T>
[[nodiscard]] bool get_value(T & value, unsigned int max_tries = 10) const
{
Expand Down
31 changes: 24 additions & 7 deletions hardware_interface/test/test_handle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,26 @@ TEST(TestHandle, command_interface)
double value = 1.337;
CommandInterface interface{JOINT_NAME, FOO_INTERFACE, &value};
EXPECT_DOUBLE_EQ(interface.get_value(), value);
EXPECT_NO_THROW(interface.set_value(0.0));
EXPECT_DOUBLE_EQ(interface.get_value(), 0.0);
ASSERT_TRUE(interface.get_value<double>().has_value());
EXPECT_DOUBLE_EQ(interface.get_value<double>().value(), value);
EXPECT_DOUBLE_EQ(interface.get_value(), value);
EXPECT_NO_THROW(bool status = interface.set_value(0.0));
bool status;
EXPECT_DOUBLE_EQ(interface.get_value(status), 0.0);
ASSERT_TRUE(status);
ASSERT_TRUE(interface.get_value<double>().has_value());
EXPECT_DOUBLE_EQ(interface.get_value<double>().value(), 0.0);
}

TEST(TestHandle, state_interface)
{
double value = 1.337;
StateInterface interface{JOINT_NAME, FOO_INTERFACE, &value};
EXPECT_DOUBLE_EQ(interface.get_value(), value);
bool status = false;
EXPECT_DOUBLE_EQ(interface.get_value(status), value);
ASSERT_TRUE(status);
ASSERT_TRUE(interface.get_value<double>().has_value());
EXPECT_DOUBLE_EQ(interface.get_value<double>().value(), value);
// interface.set_value(5); compiler error, no set_value function
}

Expand All @@ -56,16 +67,22 @@ TEST(TestHandle, value_methods_throw_for_nullptr)
{
CommandInterface handle{JOINT_NAME, FOO_INTERFACE};
EXPECT_ANY_THROW(handle.get_value());
EXPECT_ANY_THROW(handle.set_value(0.0));
EXPECT_ANY_THROW(bool status = handle.set_value(0.0));
}

TEST(TestHandle, value_methods_work_on_non_nullptr)
{
double value = 1.337;
CommandInterface handle{JOINT_NAME, FOO_INTERFACE, &value};
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(handle.set_value(0.0));
EXPECT_DOUBLE_EQ(handle.get_value(), 0.0);
bool status;
EXPECT_DOUBLE_EQ(handle.get_value(status), value);
ASSERT_TRUE(handle.get_value<double>().has_value());
EXPECT_DOUBLE_EQ(handle.get_value<double>().value(), value);
EXPECT_DOUBLE_EQ(handle.get_value(status), value);
ASSERT_TRUE(status);
EXPECT_NO_THROW(bool status_set = handle.set_value(0.0));
ASSERT_TRUE(handle.get_value<double>().has_value());
EXPECT_DOUBLE_EQ(handle.get_value<double>().value(), 0.0);
}

TEST(TestHandle, interface_description_state_interface_name_getters_work)
Expand Down
Loading