Skip to content

Commit

Permalink
add tests for copy and move operations of the Handle class (#2011)
Browse files Browse the repository at this point in the history
  • Loading branch information
@saikishor
saikishor authored Feb 7, 2025
1 parent e574645 commit 5d4933d
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 40 deletions.
78 changes: 38 additions & 40 deletions hardware_interface/include/hardware_interface/handle.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef HARDWARE_INTERFACE__HANDLE_HPP_
#define HARDWARE_INTERFACE__HANDLE_HPP_

#include <algorithm>
#include <limits>
#include <memory>
#include <mutex>
Expand All @@ -29,7 +30,7 @@
namespace hardware_interface
{

using HANDLE_DATATYPE = std::variant<double>;
using HANDLE_DATATYPE = std::variant<std::monostate, double>;

/// A handle used to get and set a value on a given interface.
class Handle
Expand Down Expand Up @@ -72,55 +73,22 @@ class Handle
{
}

Handle(const Handle & other) noexcept
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = other.prefix_name_;
interface_name_ = other.interface_name_;
handle_name_ = other.handle_name_;
value_ = other.value_;
value_ptr_ = other.value_ptr_;
}

Handle(Handle && other) noexcept
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = std::move(other.prefix_name_);
interface_name_ = std::move(other.interface_name_);
handle_name_ = std::move(other.handle_name_);
value_ = std::move(other.value_);
value_ptr_ = std::move(other.value_ptr_);
}
Handle(const Handle & other) noexcept { copy(other); }

Handle & operator=(const Handle & other)
{
if (this != &other)
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = other.prefix_name_;
interface_name_ = other.interface_name_;
handle_name_ = other.handle_name_;
value_ = other.value_;
value_ptr_ = other.value_ptr_;
copy(other);
}
return *this;
}

Handle(Handle && other) noexcept { swap(*this, other); }

Handle & operator=(Handle && other)
{
if (this != &other)
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = std::move(other.prefix_name_);
interface_name_ = std::move(other.interface_name_);
handle_name_ = std::move(other.handle_name_);
value_ = std::move(other.value_);
value_ptr_ = std::move(other.value_ptr_);
}
swap(*this, other);
return *this;
}

Expand Down Expand Up @@ -187,11 +155,41 @@ class Handle
// END
}

private:
void copy(const Handle & other) noexcept
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = other.prefix_name_;
interface_name_ = other.interface_name_;
handle_name_ = other.handle_name_;
value_ = other.value_;
if (std::holds_alternative<std::monostate>(value_))
{
value_ptr_ = other.value_ptr_;
}
else
{
value_ptr_ = std::get_if<double>(&value_);
}
}

void swap(Handle & first, Handle & second) noexcept
{
std::unique_lock<std::shared_mutex> lock(first.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(second.handle_mutex_);
std::swap(first.prefix_name_, second.prefix_name_);
std::swap(first.interface_name_, second.interface_name_);
std::swap(first.handle_name_, second.handle_name_);
std::swap(first.value_, second.value_);
std::swap(first.value_ptr_, second.value_ptr_);
}

protected:
std::string prefix_name_;
std::string interface_name_;
std::string handle_name_;
HANDLE_DATATYPE value_;
HANDLE_DATATYPE value_ = std::monostate{};
// BEGIN (Handle export change): for backward compatibility
// TODO(Manuel) redeclare as HANDLE_DATATYPE * value_ptr_ if old functionality is removed
double * value_ptr_;
Expand Down
103 changes: 103 additions & 0 deletions hardware_interface/test/test_handle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,106 @@ TEST(TestHandle, interface_description_command_interface_name_getters_work)
EXPECT_EQ(handle.get_interface_name(), POSITION_INTERFACE);
EXPECT_EQ(handle.get_prefix_name(), JOINT_NAME_1);
}

TEST(TestHandle, copy_constructor)
{
{
double value = 1.337;
hardware_interface::Handle handle{JOINT_NAME, FOO_INTERFACE, &value};
hardware_interface::Handle copy(handle);
EXPECT_DOUBLE_EQ(copy.get_value(), value);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(copy.set_value(0.0));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.0);
EXPECT_DOUBLE_EQ(handle.get_value(), 0.0);
}
{
double value = 1.337;
InterfaceInfo info;
info.name = FOO_INTERFACE;
info.data_type = "double";
InterfaceDescription itf_descr{JOINT_NAME, info};
hardware_interface::Handle handle{itf_descr};
EXPECT_TRUE(std::isnan(handle.get_value()));
handle.set_value(value);
hardware_interface::Handle copy(handle);
EXPECT_EQ(copy.get_name(), handle.get_name());
EXPECT_EQ(copy.get_interface_name(), handle.get_interface_name());
EXPECT_EQ(copy.get_prefix_name(), handle.get_prefix_name());
EXPECT_DOUBLE_EQ(copy.get_value(), value);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(copy.set_value(0.0));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.0);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(copy.set_value(0.52));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.52);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
}
}

TEST(TesHandle, move_constructor)
{
double value = 1.337;
hardware_interface::Handle handle{JOINT_NAME, FOO_INTERFACE, &value};
hardware_interface::Handle moved{std::move(handle)};
EXPECT_DOUBLE_EQ(moved.get_value(), value);
EXPECT_NO_THROW(moved.set_value(0.0));
EXPECT_DOUBLE_EQ(moved.get_value(), 0.0);
}

TEST(TestHandle, copy_assignment)
{
{
double value_1 = 1.337;
double value_2 = 2.337;
hardware_interface::Handle handle{JOINT_NAME, FOO_INTERFACE, &value_1};
hardware_interface::Handle copy{JOINT_NAME, "random", &value_2};
EXPECT_DOUBLE_EQ(copy.get_value(), value_2);
EXPECT_DOUBLE_EQ(handle.get_value(), value_1);
copy = handle;
EXPECT_DOUBLE_EQ(copy.get_value(), value_1);
EXPECT_DOUBLE_EQ(handle.get_value(), value_1);
EXPECT_NO_THROW(copy.set_value(0.0));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.0);
EXPECT_DOUBLE_EQ(handle.get_value(), 0.0);
EXPECT_DOUBLE_EQ(value_1, 0.0);
EXPECT_DOUBLE_EQ(value_2, 2.337);
}

{
double value = 1.337;
InterfaceInfo info;
info.name = FOO_INTERFACE;
info.data_type = "double";
InterfaceDescription itf_descr{JOINT_NAME, info};
hardware_interface::Handle handle{itf_descr};
EXPECT_TRUE(std::isnan(handle.get_value()));
handle.set_value(value);
hardware_interface::Handle copy = handle;
EXPECT_EQ(copy.get_name(), handle.get_name());
EXPECT_EQ(copy.get_interface_name(), handle.get_interface_name());
EXPECT_EQ(copy.get_prefix_name(), handle.get_prefix_name());
EXPECT_DOUBLE_EQ(copy.get_value(), value);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(copy.set_value(0.0));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.0);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
EXPECT_NO_THROW(copy.set_value(0.52));
EXPECT_DOUBLE_EQ(copy.get_value(), 0.52);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
}
}

TEST(TestHandle, move_assignment)
{
double value = 1.337;
double value_2 = 2.337;
hardware_interface::Handle handle{JOINT_NAME, FOO_INTERFACE, &value};
hardware_interface::Handle moved{JOINT_NAME, "random", &value_2};
EXPECT_DOUBLE_EQ(moved.get_value(), value_2);
EXPECT_DOUBLE_EQ(handle.get_value(), value);
moved = std::move(handle);
EXPECT_DOUBLE_EQ(moved.get_value(), value);
EXPECT_NO_THROW(moved.set_value(0.0));
EXPECT_DOUBLE_EQ(moved.get_value(), 0.0);
}

0 comments on commit 5d4933d

Please sign in to comment.