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

Fix controller parameter loading issue in different cases #1293

Merged
68 changes: 32 additions & 36 deletions controller_manager/controller_manager/spawner.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import sys
import time
import warnings
import io
from contextlib import redirect_stdout, redirect_stderr

from controller_manager import (
configure_controller,
Expand All @@ -37,7 +35,6 @@
from rclpy.parameter import get_parameter_value
from rclpy.signals import SignalHandlerOptions
from ros2param.api import call_set_parameters
from ros2param.api import load_parameter_file

# from https://stackoverflow.com/a/287944

Expand Down Expand Up @@ -266,6 +263,38 @@ def main(args=None):
)
return 1

if param_file:
destogl marked this conversation as resolved.
Show resolved Hide resolved
parameter = Parameter()
parameter.name = prefixed_controller_name + ".params_file"
parameter.value = get_parameter_value(string_value=param_file)

response = call_set_parameters(
destogl marked this conversation as resolved.
Show resolved Hide resolved
node=node, node_name=controller_manager_name, parameters=[parameter]
)
assert len(response.results) == 1
result = response.results[0]
if result.successful:
node.get_logger().info(
bcolors.OKCYAN
+ 'Set controller params file to "'
+ param_file
+ '" for '
+ bcolors.BOLD
+ prefixed_controller_name
+ bcolors.ENDC
)
else:
node.get_logger().fatal(
bcolors.FAIL
+ 'Could not set controller params file to "'
+ param_file
+ '" for '
+ bcolors.BOLD
+ prefixed_controller_name
+ bcolors.ENDC
)
return 1

ret = load_controller(node, controller_manager_name, controller_name)
if not ret.ok:
node.get_logger().fatal(
Expand All @@ -284,39 +313,6 @@ def main(args=None):
+ bcolors.ENDC
)

if param_file:
# load_parameter_file writes to stdout/stderr. Here we capture that and use node logging instead
with redirect_stdout(io.StringIO()) as f_stdout, redirect_stderr(
io.StringIO()
) as f_stderr:
load_parameter_file(
node=node,
node_name=prefixed_controller_name,
parameter_file=param_file,
use_wildcard=True,
)
if f_stdout.getvalue():
node.get_logger().info(bcolors.OKCYAN + f_stdout.getvalue() + bcolors.ENDC)
if f_stderr.getvalue():
node.get_logger().error(bcolors.FAIL + f_stderr.getvalue() + bcolors.ENDC)
node.get_logger().info(
bcolors.OKCYAN
+ 'Loaded parameters file "'
+ param_file
+ '" for '
+ bcolors.BOLD
+ prefixed_controller_name
+ bcolors.ENDC
)
# TODO(destogl): use return value when upstream return value is merged
# ret =
# if ret.returncode != 0:
# Error message printed by ros2 param
# return ret.returncode
node.get_logger().info(
"Loaded " + param_file + " into " + prefixed_controller_name
)

if not args.load_only:
ret = configure_controller(node, controller_manager_name, controller_name)
if not ret.ok:
Expand Down
27 changes: 25 additions & 2 deletions controller_manager/src/controller_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,23 @@ controller_interface::ControllerInterfaceBaseSharedPtr ControllerManager::load_c
controller_spec.next_update_cycle_time = std::make_shared<rclcpp::Time>(
0, 0, this->get_node_clock_interface()->get_clock()->get_clock_type());

// We have to fetch the params_file at the time of loading the controller, because this way we can
// load them at the creating of the LifeCycleNode and this helps in using the features such as
saikishor marked this conversation as resolved.
Show resolved Hide resolved
// read_only params, dynamic maps lists etc
// Now check if the params_file parameter exist
const std::string param_name = controller_name + ".params_file";
std::string params_file;

// Check if parameter has been declared
if (!has_parameter(param_name))
{
declare_parameter(param_name, rclcpp::ParameterType::PARAMETER_STRING);
christophfroehlich marked this conversation as resolved.
Show resolved Hide resolved
}
if (get_parameter(param_name, params_file) && !params_file.empty())
{
controller_spec.info.params_file = params_file;
}

return add_controller_impl(controller_spec);
}

Expand Down Expand Up @@ -1300,10 +1317,16 @@ controller_interface::ControllerInterfaceBaseSharedPtr ControllerManager::add_co
return nullptr;
}

rclcpp::NodeOptions controller_node_options = rclcpp::NodeOptions().enable_logger_service(true);
if (controller.info.params_file.has_value())
{
controller_node_options = controller_node_options.arguments(
{"--ros-args", "--params-file", controller.info.params_file.value()});
}
if (
controller.c->init(
controller.info.name, robot_description_, get_update_rate(), get_namespace()) ==
controller_interface::return_type::ERROR)
controller.info.name, robot_description_, get_update_rate(), get_namespace(),
controller_node_options) == controller_interface::return_type::ERROR)
destogl marked this conversation as resolved.
Show resolved Hide resolved
{
to.clear();
RCLCPP_ERROR(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef HARDWARE_INTERFACE__CONTROLLER_INFO_HPP_
#define HARDWARE_INTERFACE__CONTROLLER_INFO_HPP_

#include <optional>
#include <string>
#include <vector>

Expand All @@ -32,6 +33,9 @@ struct ControllerInfo
/// Controller type.
std::string type;

/// Controller param file
std::optional<std::string> params_file;
destogl marked this conversation as resolved.
Show resolved Hide resolved

/// List of claimed interfaces by the controller.
std::vector<std::string> claimed_interfaces;
};
Expand Down
Loading