Fix interactive markers not visible in motion planning plugin #299
Conversation
Codecov Report
@@ Coverage Diff @@
## main #299 +/- ##
==========================================
+ Coverage 47.94% 48.00% +0.06%
==========================================
Files 157 157
Lines 14840 14843 +3
==========================================
+ Hits 7114 7124 +10
+ Misses 7726 7719 -7
Continue to review full report at Codecov.
|
| // read the list of plugin names for possible kinematics solvers | ||
| for (const srdf::Model::Group& known_group : known_groups) | ||
| { | ||
| std::string base_param_name = known_group.name_; | ||
| std::string ksolver_param_name = base_param_name + ".kinematics_solver"; | ||
| RCLCPP_DEBUG(LOGGER, "Looking for param %s ", ksolver_param_name.c_str()); | ||
| bool found = node_->has_parameter(ksolver_param_name); | ||
| if (!found) | ||
| rclcpp::Parameter ksolver_param = declare_parameter(node_, ksolver_param_name); |
There was a problem hiding this comment.
Why do we need these additional checks? Are we running multiple IK solver instances in the same node namespace? Otherwise, just calling Node::declare_parameter() should be sufficient.
moveit_ros/planning/kinematics_plugin_loader/src/kinematics_plugin_loader.cpp
Outdated
Show resolved
Hide resolved
| double ksolver_timeout; | ||
| rclcpp::Parameter parameter = node_->get_parameter(ksolver_timeout_param_name); | ||
| if (parameter.get_type() == rclcpp::ParameterType::PARAMETER_DOUBLE) | ||
| if (ksolver_timeout_param.get_type() == rclcpp::ParameterType::PARAMETER_DOUBLE) |
There was a problem hiding this comment.
Shouldn't we just declare the parameter as double? That way we don't have to check the different cases
There was a problem hiding this comment.
It will throw the following exception rclcpp::exceptions::InvalidParameterTypeException if the loaded parameter is integer
JafarAbdi
force-pushed
the
pr-fix_interactive_markers
branch
from
8b8bf33
to
0f7576c
Compare
|
I reverted the last commit because the function with one parameter will throw an exception if the parameter is declared and not set https://github.com/ros2/rclcpp/blob/master/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp#L680, this's why the tests were throwing exceptions |
|
This still seems overly complicated to me. A whole lot of code just for loading 4 parameters. But this is an issue that affects a lot of our classes currently. I'll merge this for now as it fixes the interactive marker, but we should still simplify and refactor parameter usage overall. |
Description
Please explain the changes you made, including a reference to the related issue if applicable
Checklist