Skip to content

Commit 34d2b36

Browse files
Manuel Muthbmagyar
andauthored
Adapt controller Reference/StateInterfaces to New Way of Exporting (variant support) (#1689)
--------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
1 parent ab84b74 commit 34d2b36

File tree

12 files changed

+202
-81
lines changed

12 files changed

+202
-81
lines changed

controller_interface/include/controller_interface/chainable_controller_interface.hpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
#ifndef CONTROLLER_INTERFACE__CHAINABLE_CONTROLLER_INTERFACE_HPP_
1616
#define CONTROLLER_INTERFACE__CHAINABLE_CONTROLLER_INTERFACE_HPP_
1717

18+
#include <memory>
1819
#include <string>
20+
#include <unordered_map>
1921
#include <vector>
2022

2123
#include "controller_interface/controller_interface_base.hpp"
@@ -57,10 +59,10 @@ class ChainableControllerInterface : public ControllerInterfaceBase
5759
bool is_chainable() const final;
5860

5961
CONTROLLER_INTERFACE_PUBLIC
60-
std::vector<hardware_interface::StateInterface> export_state_interfaces() final;
62+
std::vector<hardware_interface::StateInterface::SharedPtr> export_state_interfaces() final;
6163

6264
CONTROLLER_INTERFACE_PUBLIC
63-
std::vector<hardware_interface::CommandInterface> export_reference_interfaces() final;
65+
std::vector<hardware_interface::CommandInterface::SharedPtr> export_reference_interfaces() final;
6466

6567
CONTROLLER_INTERFACE_PUBLIC
6668
bool set_chained_mode(bool chained_mode) final;
@@ -131,11 +133,21 @@ class ChainableControllerInterface : public ControllerInterfaceBase
131133

132134
/// Storage of values for state interfaces
133135
std::vector<std::string> exported_state_interface_names_;
136+
std::vector<hardware_interface::StateInterface::SharedPtr> ordered_exported_state_interfaces_;
137+
std::unordered_map<std::string, hardware_interface::StateInterface::SharedPtr>
138+
exported_state_interfaces_;
139+
// BEGIN (Handle export change): for backward compatibility
134140
std::vector<double> state_interfaces_values_;
141+
// END
135142

136143
/// Storage of values for reference interfaces
137144
std::vector<std::string> exported_reference_interface_names_;
145+
// BEGIN (Handle export change): for backward compatibility
138146
std::vector<double> reference_interfaces_;
147+
// END
148+
std::vector<hardware_interface::CommandInterface::SharedPtr> ordered_reference_interfaces_;
149+
std::unordered_map<std::string, hardware_interface::CommandInterface::SharedPtr>
150+
reference_interfaces_ptrs_;
139151

140152
private:
141153
/// A flag marking if a chainable controller is currently preceded by another controller.

controller_interface/include/controller_interface/controller_interface.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,15 @@ class ControllerInterface : public controller_interface::ControllerInterfaceBase
4747
* \returns empty list.
4848
*/
4949
CONTROLLER_INTERFACE_PUBLIC
50-
std::vector<hardware_interface::StateInterface> export_state_interfaces() final;
50+
std::vector<hardware_interface::StateInterface::SharedPtr> export_state_interfaces() final;
5151

5252
/**
5353
* Controller has no reference interfaces.
5454
*
5555
* \returns empty list.
5656
*/
5757
CONTROLLER_INTERFACE_PUBLIC
58-
std::vector<hardware_interface::CommandInterface> export_reference_interfaces() final;
58+
std::vector<hardware_interface::CommandInterface::SharedPtr> export_reference_interfaces() final;
5959

6060
/**
6161
* Controller is not chainable, therefore no chained mode can be set.

controller_interface/include/controller_interface/controller_interface_base.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
235235
* \returns list of command interfaces for preceding controllers.
236236
*/
237237
CONTROLLER_INTERFACE_PUBLIC
238-
virtual std::vector<hardware_interface::CommandInterface> export_reference_interfaces() = 0;
238+
virtual std::vector<hardware_interface::CommandInterface::SharedPtr>
239+
export_reference_interfaces() = 0;
239240

240241
/**
241242
* Export interfaces for a chainable controller that can be used as state interface by other
@@ -244,7 +245,7 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
244245
* \returns list of state interfaces for preceding controllers.
245246
*/
246247
CONTROLLER_INTERFACE_PUBLIC
247-
virtual std::vector<hardware_interface::StateInterface> export_state_interfaces() = 0;
248+
virtual std::vector<hardware_interface::StateInterface::SharedPtr> export_state_interfaces() = 0;
248249

249250
/**
250251
* Set chained mode of a chainable controller. This method triggers internal processes to switch

controller_interface/src/chainable_controller_interface.cpp

Lines changed: 105 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,53 +44,134 @@ return_type ChainableControllerInterface::update(
4444
return ret;
4545
}
4646

47-
std::vector<hardware_interface::StateInterface>
47+
std::vector<hardware_interface::StateInterface::SharedPtr>
4848
ChainableControllerInterface::export_state_interfaces()
4949
{
5050
auto state_interfaces = on_export_state_interfaces();
51+
std::vector<hardware_interface::StateInterface::SharedPtr> state_interfaces_ptrs_vec;
52+
state_interfaces_ptrs_vec.reserve(state_interfaces.size());
53+
ordered_exported_state_interfaces_.reserve(state_interfaces.size());
54+
exported_state_interface_names_.reserve(state_interfaces.size());
5155

5256
// check if the names of the controller state interfaces begin with the controller's name
5357
for (const auto & interface : state_interfaces)
5458
{
5559
if (interface.get_prefix_name() != get_node()->get_name())
5660
{
57-
RCLCPP_FATAL(
58-
get_node()->get_logger(),
59-
"The name of the interface '%s' does not begin with the controller's name. This is "
60-
"mandatory for state interfaces. No state interface will be exported. Please "
61-
"correct and recompile the controller with name '%s' and try again.",
62-
interface.get_name().c_str(), get_node()->get_name());
63-
state_interfaces.clear();
64-
break;
61+
std::string error_msg =
62+
"The prefix of the interface '" + interface.get_prefix_name() +
63+
"' does not equal the controller's name '" + get_node()->get_name() +
64+
"'. This is mandatory for state interfaces. No state interface will be exported. Please "
65+
"correct and recompile the controller with name '" +
66+
get_node()->get_name() + "' and try again.";
67+
throw std::runtime_error(error_msg);
68+
}
69+
auto state_interface = std::make_shared<hardware_interface::StateInterface>(interface);
70+
const auto interface_name = state_interface->get_name();
71+
auto [it, succ] = exported_state_interfaces_.insert({interface_name, state_interface});
72+
// either we have name duplicate which we want to avoid under all circumstances since interfaces
73+
// need to be uniquely identify able or something else really went wrong. In any case abort and
74+
// inform cm by throwing exception
75+
if (!succ)
76+
{
77+
std::string error_msg =
78+
"Could not insert StateInterface<" + interface_name +
79+
"> into exported_state_interfaces_ map. Check if you export duplicates. The "
80+
"map returned iterator with interface_name<" +
81+
it->second->get_name() +
82+
">. If its a duplicate adjust exportation of InterfacesDescription so that all the "
83+
"interface names are unique.";
84+
exported_state_interfaces_.clear();
85+
exported_state_interface_names_.clear();
86+
state_interfaces_ptrs_vec.clear();
87+
throw std::runtime_error(error_msg);
6588
}
89+
ordered_exported_state_interfaces_.push_back(state_interface);
90+
exported_state_interface_names_.push_back(interface_name);
91+
state_interfaces_ptrs_vec.push_back(state_interface);
6692
}
6793

68-
return state_interfaces;
94+
return state_interfaces_ptrs_vec;
6995
}
7096

71-
std::vector<hardware_interface::CommandInterface>
97+
std::vector<hardware_interface::CommandInterface::SharedPtr>
7298
ChainableControllerInterface::export_reference_interfaces()
7399
{
74100
auto reference_interfaces = on_export_reference_interfaces();
101+
std::vector<hardware_interface::CommandInterface::SharedPtr> reference_interfaces_ptrs_vec;
102+
reference_interfaces_ptrs_vec.reserve(reference_interfaces.size());
103+
exported_reference_interface_names_.reserve(reference_interfaces.size());
104+
ordered_reference_interfaces_.reserve(reference_interfaces.size());
105+
106+
// BEGIN (Handle export change): for backward compatibility
107+
// check if the "reference_interfaces_" variable is resized to number of interfaces
108+
if (reference_interfaces_.size() != reference_interfaces.size())
109+
{
110+
std::string error_msg =
111+
"The internal storage for reference values 'reference_interfaces_' variable has size '" +
112+
std::to_string(reference_interfaces_.size()) + "', but it is expected to have the size '" +
113+
std::to_string(reference_interfaces.size()) +
114+
"' equal to the number of exported reference interfaces. Please correct and recompile the "
115+
"controller with name '" +
116+
get_node()->get_name() + "' and try again.";
117+
throw std::runtime_error(error_msg);
118+
}
119+
// END
75120

76121
// check if the names of the reference interfaces begin with the controller's name
77-
for (const auto & interface : reference_interfaces)
122+
const auto ref_interface_size = reference_interfaces.size();
123+
for (auto & interface : reference_interfaces)
78124
{
79125
if (interface.get_prefix_name() != get_node()->get_name())
80126
{
81-
RCLCPP_FATAL(
82-
get_node()->get_logger(),
83-
"The name of the interface '%s' does not begin with the controller's name. This is "
84-
"mandatory "
85-
" for reference interfaces. No reference interface will be exported. Please correct and "
86-
"recompile the controller with name '%s' and try again.",
87-
interface.get_name().c_str(), get_node()->get_name());
88-
reference_interfaces.clear();
89-
break;
127+
std::string error_msg = "The name of the interface " + interface.get_name() +
128+
" does not begin with the controller's name. This is mandatory for "
129+
"reference interfaces. Please "
130+
"correct and recompile the controller with name " +
131+
get_node()->get_name() + " and try again.";
132+
throw std::runtime_error(error_msg);
90133
}
134+
135+
hardware_interface::CommandInterface::SharedPtr reference_interface =
136+
std::make_shared<hardware_interface::CommandInterface>(std::move(interface));
137+
const auto inteface_name = reference_interface->get_name();
138+
// check the exported interface name is unique
139+
auto [it, succ] = reference_interfaces_ptrs_.insert({inteface_name, reference_interface});
140+
// either we have name duplicate which we want to avoid under all circumstances since interfaces
141+
// need to be uniquely identify able or something else really went wrong. In any case abort and
142+
// inform cm by throwing exception
143+
if (!succ)
144+
{
145+
std::string error_msg =
146+
"Could not insert Reference interface<" + inteface_name +
147+
"> into reference_interfaces_ map. Check if you export duplicates. The "
148+
"map returned iterator with interface_name<" +
149+
it->second->get_name() +
150+
">. If its a duplicate adjust exportation of InterfacesDescription so that all the "
151+
"interface names are unique.";
152+
reference_interfaces_.clear();
153+
exported_reference_interface_names_.clear();
154+
reference_interfaces_ptrs_vec.clear();
155+
throw std::runtime_error(error_msg);
156+
}
157+
ordered_reference_interfaces_.push_back(reference_interface);
158+
exported_reference_interface_names_.push_back(inteface_name);
159+
reference_interfaces_ptrs_vec.push_back(reference_interface);
91160
}
92161

93-
return reference_interfaces;
162+
if (reference_interfaces_ptrs_.size() != ref_interface_size)
163+
{
164+
std::string error_msg =
165+
"The internal storage for reference ptrs 'reference_interfaces_ptrs_' variable has size '" +
166+
std::to_string(reference_interfaces_ptrs_.size()) +
167+
"', but it is expected to have the size '" + std::to_string(ref_interface_size) +
168+
"' equal to the number of exported reference interfaces. Please correct and recompile the "
169+
"controller with name '" +
170+
get_node()->get_name() + "' and try again.";
171+
throw std::runtime_error(error_msg);
172+
}
173+
174+
return reference_interfaces_ptrs_vec;
94175
}
95176

96177
bool ChainableControllerInterface::set_chained_mode(bool chained_mode)
@@ -130,8 +211,8 @@ ChainableControllerInterface::on_export_state_interfaces()
130211
std::vector<hardware_interface::StateInterface> state_interfaces;
131212
for (size_t i = 0; i < exported_state_interface_names_.size(); ++i)
132213
{
133-
state_interfaces.emplace_back(hardware_interface::StateInterface(
134-
get_node()->get_name(), exported_state_interface_names_[i], &state_interfaces_values_[i]));
214+
state_interfaces.emplace_back(
215+
get_node()->get_name(), exported_state_interface_names_[i], &state_interfaces_values_[i]);
135216
}
136217
return state_interfaces;
137218
}

controller_interface/src/controller_interface.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ ControllerInterface::ControllerInterface() : ControllerInterfaceBase() {}
2222

2323
bool ControllerInterface::is_chainable() const { return false; }
2424

25-
std::vector<hardware_interface::StateInterface> ControllerInterface::export_state_interfaces()
25+
std::vector<hardware_interface::StateInterface::SharedPtr>
26+
ControllerInterface::export_state_interfaces()
2627
{
2728
return {};
2829
}
2930

30-
std::vector<hardware_interface::CommandInterface> ControllerInterface::export_reference_interfaces()
31+
std::vector<hardware_interface::CommandInterface::SharedPtr>
32+
ControllerInterface::export_reference_interfaces()
3133
{
3234
return {};
3335
}

controller_interface/test/test_chainable_controller_interface.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "test_chainable_controller_interface.hpp"
1616

1717
#include <gmock/gmock.h>
18+
#include <memory>
1819

1920
using ::testing::IsEmpty;
2021
using ::testing::SizeIs;
@@ -48,10 +49,10 @@ TEST_F(ChainableControllerInterfaceTest, export_state_interfaces)
4849
auto exported_state_interfaces = controller.export_state_interfaces();
4950

5051
ASSERT_THAT(exported_state_interfaces, SizeIs(1));
51-
EXPECT_EQ(exported_state_interfaces[0].get_prefix_name(), TEST_CONTROLLER_NAME);
52-
EXPECT_EQ(exported_state_interfaces[0].get_interface_name(), "test_state");
52+
EXPECT_EQ(exported_state_interfaces[0]->get_prefix_name(), TEST_CONTROLLER_NAME);
53+
EXPECT_EQ(exported_state_interfaces[0]->get_interface_name(), "test_state");
5354

54-
EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE);
55+
EXPECT_EQ(exported_state_interfaces[0]->get_value(), EXPORTED_STATE_INTERFACE_VALUE);
5556
}
5657

5758
TEST_F(ChainableControllerInterfaceTest, export_reference_interfaces)
@@ -68,10 +69,10 @@ TEST_F(ChainableControllerInterfaceTest, export_reference_interfaces)
6869
auto reference_interfaces = controller.export_reference_interfaces();
6970

7071
ASSERT_THAT(reference_interfaces, SizeIs(1));
71-
EXPECT_EQ(reference_interfaces[0].get_prefix_name(), TEST_CONTROLLER_NAME);
72-
EXPECT_EQ(reference_interfaces[0].get_interface_name(), "test_itf");
72+
EXPECT_EQ(reference_interfaces[0]->get_prefix_name(), TEST_CONTROLLER_NAME);
73+
EXPECT_EQ(reference_interfaces[0]->get_interface_name(), "test_itf");
7374

74-
EXPECT_EQ(reference_interfaces[0].get_value(), INTERFACE_VALUE);
75+
EXPECT_EQ(reference_interfaces[0]->get_value(), INTERFACE_VALUE);
7576
}
7677

7778
TEST_F(ChainableControllerInterfaceTest, interfaces_prefix_is_not_node_name)
@@ -88,10 +89,15 @@ TEST_F(ChainableControllerInterfaceTest, interfaces_prefix_is_not_node_name)
8889
controller.set_name_prefix_of_reference_interfaces("some_not_correct_interface_prefix");
8990

9091
// expect empty return because interface prefix is not equal to the node name
91-
auto reference_interfaces = controller.export_reference_interfaces();
92-
ASSERT_THAT(reference_interfaces, IsEmpty());
92+
std::vector<hardware_interface::CommandInterface::SharedPtr> exported_reference_interfaces;
93+
EXPECT_THROW(
94+
{ exported_reference_interfaces = controller.export_reference_interfaces(); },
95+
std::runtime_error);
96+
ASSERT_THAT(exported_reference_interfaces, IsEmpty());
9397
// expect empty return because interface prefix is not equal to the node name
94-
auto exported_state_interfaces = controller.export_state_interfaces();
98+
std::vector<hardware_interface::StateInterface::SharedPtr> exported_state_interfaces;
99+
EXPECT_THROW(
100+
{ exported_state_interfaces = controller.export_state_interfaces(); }, std::runtime_error);
95101
ASSERT_THAT(exported_state_interfaces, IsEmpty());
96102
}
97103

@@ -114,8 +120,7 @@ TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)
114120
EXPECT_FALSE(controller.is_in_chained_mode());
115121

116122
// Fail setting chained mode
117-
EXPECT_EQ(reference_interfaces[0].get_value(), INTERFACE_VALUE);
118-
EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE);
123+
EXPECT_EQ(reference_interfaces[0]->get_value(), INTERFACE_VALUE);
119124

120125
EXPECT_FALSE(controller.set_chained_mode(true));
121126
EXPECT_FALSE(controller.is_in_chained_mode());
@@ -124,11 +129,11 @@ TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)
124129
EXPECT_FALSE(controller.is_in_chained_mode());
125130

126131
// Success setting chained mode
127-
reference_interfaces[0].set_value(0.0);
132+
reference_interfaces[0]->set_value(0.0);
128133

129134
EXPECT_TRUE(controller.set_chained_mode(true));
130135
EXPECT_TRUE(controller.is_in_chained_mode());
131-
EXPECT_EQ(exported_state_interfaces[0].get_value(), EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE);
136+
EXPECT_EQ(exported_state_interfaces[0]->get_value(), EXPORTED_STATE_INTERFACE_VALUE_IN_CHAINMODE);
132137

133138
controller.configure();
134139
EXPECT_TRUE(controller.set_chained_mode(false));

controller_manager/src/controller_manager.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -771,15 +771,28 @@ controller_interface::return_type ControllerManager::configure_controller(
771771
get_logger(),
772772
"Controller '%s' is chainable. Interfaces are being exported to resource manager.",
773773
controller_name.c_str());
774-
auto state_interfaces = controller->export_state_interfaces();
775-
auto ref_interfaces = controller->export_reference_interfaces();
776-
if (ref_interfaces.empty() && state_interfaces.empty())
774+
std::vector<hardware_interface::StateInterface::SharedPtr> state_interfaces;
775+
std::vector<hardware_interface::CommandInterface::SharedPtr> ref_interfaces;
776+
try
777777
{
778-
// TODO(destogl): Add test for this!
779-
RCLCPP_ERROR(
780-
get_logger(),
781-
"Controller '%s' is chainable, but does not export any state or reference interfaces.",
782-
controller_name.c_str());
778+
state_interfaces = controller->export_state_interfaces();
779+
ref_interfaces = controller->export_reference_interfaces();
780+
if (ref_interfaces.empty() && state_interfaces.empty())
781+
{
782+
// TODO(destogl): Add test for this!
783+
RCLCPP_ERROR(
784+
get_logger(),
785+
"Controller '%s' is chainable, but does not export any reference interfaces. Did you "
786+
"override the on_export_method() correctly?",
787+
controller_name.c_str());
788+
return controller_interface::return_type::ERROR;
789+
}
790+
}
791+
catch (const std::runtime_error & e)
792+
{
793+
RCLCPP_FATAL(
794+
get_logger(), "Creation of the reference interfaces failed with following error: %s",
795+
e.what());
783796
return controller_interface::return_type::ERROR;
784797
}
785798
resource_manager_->import_controller_reference_interfaces(controller_name, ref_interfaces);

0 commit comments

Comments
 (0)