Skip to content

Commit 0ffaacc

Browse files
authored
Merge branch 'master' into fix/on_error/controller_deactivation
2 parents 57c4f56 + a6e44ab commit 0ffaacc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+2852
-316
lines changed

.pre-commit-config.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ repos:
6363

6464
# CPP hooks
6565
- repo: https://github.com/pre-commit/mirrors-clang-format
66-
rev: v18.1.8
66+
rev: v19.1.0
6767
hooks:
6868
- id: clang-format
6969
args: ['-fallback-style=none', '-i']
@@ -109,7 +109,7 @@ repos:
109109

110110
# Docs - RestructuredText hooks
111111
- repo: https://github.com/PyCQA/doc8
112-
rev: v1.1.1
112+
rev: v1.1.2
113113
hooks:
114114
- id: doc8
115115
args: ['--max-line-length=100', '--ignore=D001']
@@ -133,7 +133,7 @@ repos:
133133
exclude: CHANGELOG\.rst|\.(svg|pyc|drawio)$
134134

135135
- repo: https://github.com/python-jsonschema/check-jsonschema
136-
rev: 0.29.2
136+
rev: 0.29.3
137137
hooks:
138138
- id: check-github-workflows
139139
args: ["--verbose"]

controller_interface/CHANGELOG.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22
Changelog for package controller_interface
33
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44

5+
4.18.0 (2024-10-07)
6+
-------------------
7+
* Adapt controller Reference/StateInterfaces to New Way of Exporting (variant support) (`#1689 <https://github.com/ros-controls/ros2_control/issues/1689>`_)
8+
* [ControllerInterface] Fix to properly propagate the controller NodeOptions (`#1762 <https://github.com/ros-controls/ros2_control/issues/1762>`_)
9+
* [Controller Interface] Make assign and release interfaces virtual (`#1743 <https://github.com/ros-controls/ros2_control/issues/1743>`_)
10+
* Contributors: Manuel Muth, Sai Kishor Kothakota
11+
512
4.17.0 (2024-09-11)
613
-------------------
714
* Rename `get_state` and `set_state` Functions to `get/set_lifecylce_state` (variant support) (`#1683 <https://github.com/ros-controls/ros2_control/issues/1683>`_)

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::ConstSharedPtr> 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::ConstSharedPtr> 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: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,14 +185,15 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
185185
CONTROLLER_INTERFACE_PUBLIC
186186
virtual rclcpp::NodeOptions define_custom_node_options() const
187187
{
188+
rclcpp::NodeOptions node_options;
188189
// \note The versions conditioning is added here to support the source-compatibility with Humble
189190
#if RCLCPP_VERSION_MAJOR >= 21
190-
return rclcpp::NodeOptions().enable_logger_service(true);
191+
node_options.enable_logger_service(true);
191192
#else
192-
return rclcpp::NodeOptions()
193-
.allow_undeclared_parameters(true)
194-
.automatically_declare_parameters_from_overrides(true);
193+
node_options.allow_undeclared_parameters(true);
194+
node_options.automatically_declare_parameters_from_overrides(true);
195195
#endif
196+
return node_options;
196197
}
197198

198199
/// Declare and initialize a parameter with a type.
@@ -234,7 +235,8 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
234235
* \returns list of command interfaces for preceding controllers.
235236
*/
236237
CONTROLLER_INTERFACE_PUBLIC
237-
virtual std::vector<hardware_interface::CommandInterface> export_reference_interfaces() = 0;
238+
virtual std::vector<hardware_interface::CommandInterface::SharedPtr>
239+
export_reference_interfaces() = 0;
238240

239241
/**
240242
* Export interfaces for a chainable controller that can be used as state interface by other
@@ -243,7 +245,8 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
243245
* \returns list of state interfaces for preceding controllers.
244246
*/
245247
CONTROLLER_INTERFACE_PUBLIC
246-
virtual std::vector<hardware_interface::StateInterface> export_state_interfaces() = 0;
248+
virtual std::vector<hardware_interface::StateInterface::ConstSharedPtr>
249+
export_state_interfaces() = 0;
247250

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

controller_interface/package.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
33
<package format="2">
44
<name>controller_interface</name>
5-
<version>4.17.0</version>
5+
<version>4.18.0</version>
66
<description>Description of controller_interface</description>
77
<maintainer email="[email protected]">Bence Magyar</maintainer>
88
<maintainer email="[email protected]">Denis Štogl</maintainer>

controller_interface/src/chainable_controller_interface.cpp

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

47-
std::vector<hardware_interface::StateInterface>
47+
std::vector<hardware_interface::StateInterface::ConstSharedPtr>
4848
ChainableControllerInterface::export_state_interfaces()
4949
{
5050
auto state_interfaces = on_export_state_interfaces();
51+
std::vector<hardware_interface::StateInterface::ConstSharedPtr> 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(
92+
std::const_pointer_cast<const hardware_interface::StateInterface>(state_interface));
6693
}
6794

68-
return state_interfaces;
95+
return state_interfaces_ptrs_vec;
6996
}
7097

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

76122
// check if the names of the reference interfaces begin with the controller's name
77-
for (const auto & interface : reference_interfaces)
123+
const auto ref_interface_size = reference_interfaces.size();
124+
for (auto & interface : reference_interfaces)
78125
{
79126
if (interface.get_prefix_name() != get_node()->get_name())
80127
{
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;
128+
std::string error_msg = "The name of the interface " + interface.get_name() +
129+
" does not begin with the controller's name. This is mandatory for "
130+
"reference interfaces. Please "
131+
"correct and recompile the controller with name " +
132+
get_node()->get_name() + " and try again.";
133+
throw std::runtime_error(error_msg);
90134
}
135+
136+
hardware_interface::CommandInterface::SharedPtr reference_interface =
137+
std::make_shared<hardware_interface::CommandInterface>(std::move(interface));
138+
const auto inteface_name = reference_interface->get_name();
139+
// check the exported interface name is unique
140+
auto [it, succ] = reference_interfaces_ptrs_.insert({inteface_name, reference_interface});
141+
// either we have name duplicate which we want to avoid under all circumstances since interfaces
142+
// need to be uniquely identify able or something else really went wrong. In any case abort and
143+
// inform cm by throwing exception
144+
if (!succ)
145+
{
146+
std::string error_msg =
147+
"Could not insert Reference interface<" + inteface_name +
148+
"> into reference_interfaces_ map. Check if you export duplicates. The "
149+
"map returned iterator with interface_name<" +
150+
it->second->get_name() +
151+
">. If its a duplicate adjust exportation of InterfacesDescription so that all the "
152+
"interface names are unique.";
153+
reference_interfaces_.clear();
154+
exported_reference_interface_names_.clear();
155+
reference_interfaces_ptrs_vec.clear();
156+
throw std::runtime_error(error_msg);
157+
}
158+
ordered_reference_interfaces_.push_back(reference_interface);
159+
exported_reference_interface_names_.push_back(inteface_name);
160+
reference_interfaces_ptrs_vec.push_back(reference_interface);
91161
}
92162

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

96178
bool ChainableControllerInterface::set_chained_mode(bool chained_mode)
@@ -130,8 +212,8 @@ ChainableControllerInterface::on_export_state_interfaces()
130212
std::vector<hardware_interface::StateInterface> state_interfaces;
131213
for (size_t i = 0; i < exported_state_interface_names_.size(); ++i)
132214
{
133-
state_interfaces.emplace_back(hardware_interface::StateInterface(
134-
get_node()->get_name(), exported_state_interface_names_[i], &state_interfaces_values_[i]));
215+
state_interfaces.emplace_back(
216+
get_node()->get_name(), exported_state_interface_names_[i], &state_interfaces_values_[i]);
135217
}
136218
return state_interfaces;
137219
}

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::ConstSharedPtr>
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
}

0 commit comments

Comments
 (0)