Description
moveit_ros/moveit_servo/config/panda_simulated_config.yaml reads like a normal ROS 2 parameter file, but it cannot be used as one. Two separate things are missing, and both only show up at runtime.
1. No ros__parameters / /** wrapper.
The file is a flat list of keys:
publish_period: 0.01
max_expected_latency: 0.1
command_in_type: "speed_units"
...
Passing it the obvious way —
Node(package="moveit_servo", executable="servo_node", parameters=[servo_yaml_path])
— fails, because there is no node-name or /** key at the top level. The demo launch files avoid this by going through launch_param_builder:
servo_params = {
"moveit_servo": ParameterBuilder("moveit_servo")
.yaml("config/panda_simulated_config.yaml")
.to_dict()
}
which does the namespacing for you. That is a reasonable design, but nothing in the file says so, and the resulting error does not point at it.
2. The smoothing plugin the file enables needs a parameter the file does not contain.
Lines 30-31 of the same file:
use_smoothing: true
smoothing_filter_plugin_name: "online_signal_smoothing::AccelerationLimitedPlugin"
AccelerationLimitedPlugin requires update_period (params_.update_period, used throughout moveit_core/online_signal_smoothing/src/acceleration_filter.cpp, e.g. lines 285, 294-296, 316, 322). That parameter is not in panda_simulated_config.yaml. Every demo launch file supplies it separately, at node level:
# moveit_ros/moveit_servo/launch/demo_twist.launch.py:26
acceleration_filter_update_period = {"update_period": 0.01}
moveit_core/online_signal_smoothing/test/test_acceleration_filter.cpp:85 confirms the plugin is expected to fail without it ("fail because the update_period parameter is not set").
So the config enables a plugin whose required parameter lives only in the launch files next to it. Anyone starting from this config in their own launch file — which is the natural thing to do, since it is the only complete servo config in the tree — gets a failure whose cause is in a different file.
Suggested fix
Either would resolve it; the first is less surprising:
- Make the file self-contained: wrap it in
/**: ros__parameters: and add update_period alongside the smoothing plugin name that requires it.
- Or keep it as-is and add a header comment stating that the file must be loaded via
ParameterBuilder and that update_period has to be supplied separately when AccelerationLimitedPlugin is enabled.
Happy to send a PR for whichever you prefer.
Related
#3008 also involves the smoothing plugin selected in this file, but is about the Ruckig plugin's runtime behaviour rather than the config being loadable.
ROS Distro
Jazzy
OS and version
Ubuntu 24.04 (container)
Source or binary build?
Binary
If binary, which release version?
The exact patch version was not recorded. The report does not depend on it: I verified the file, the plugin's update_period use and the launch-file workaround directly in the upstream tree on both main (current) and jazzy (4d84106) — identical on both.
Which RMW are you using?
Default (rmw_fastrtps_cpp)
Steps to Reproduce
- Write a launch file that starts
servo_node and passes the shipped config the direct way, rather than through ParameterBuilder:
servo_yaml = os.path.join(
get_package_share_directory("moveit_servo"), "config", "panda_simulated_config.yaml"
)
Node(package="moveit_servo", executable="servo_node", parameters=[servo_yaml, ...])
-
Launch it. It fails on the missing top-level key.
-
Fix that by wrapping the file in /**: ros__parameters:, and launch again. It now gets further and fails inside AccelerationLimitedPlugin, because update_period is still unset — that value only exists in the demo launch files.
Expected behavior
Either the shipped config loads as a parameter file and brings up servo with the smoothing plugin it selects, or the file states that it is not a standalone parameter file and lists what has to be supplied alongside it.
Actual behavior
The file cannot be loaded directly, and once that is worked around, the smoothing plugin it enables by default fails on a parameter the file does not contain.
Description
moveit_ros/moveit_servo/config/panda_simulated_config.yamlreads like a normal ROS 2 parameter file, but it cannot be used as one. Two separate things are missing, and both only show up at runtime.1. No
ros__parameters//**wrapper.The file is a flat list of keys:
Passing it the obvious way —
— fails, because there is no node-name or
/**key at the top level. The demo launch files avoid this by going throughlaunch_param_builder:which does the namespacing for you. That is a reasonable design, but nothing in the file says so, and the resulting error does not point at it.
2. The smoothing plugin the file enables needs a parameter the file does not contain.
Lines 30-31 of the same file:
AccelerationLimitedPluginrequiresupdate_period(params_.update_period, used throughoutmoveit_core/online_signal_smoothing/src/acceleration_filter.cpp, e.g. lines 285, 294-296, 316, 322). That parameter is not inpanda_simulated_config.yaml. Every demo launch file supplies it separately, at node level:moveit_core/online_signal_smoothing/test/test_acceleration_filter.cpp:85confirms the plugin is expected to fail without it ("fail because theupdate_periodparameter is not set").So the config enables a plugin whose required parameter lives only in the launch files next to it. Anyone starting from this config in their own launch file — which is the natural thing to do, since it is the only complete servo config in the tree — gets a failure whose cause is in a different file.
Suggested fix
Either would resolve it; the first is less surprising:
/**: ros__parameters:and addupdate_periodalongside the smoothing plugin name that requires it.ParameterBuilderand thatupdate_periodhas to be supplied separately whenAccelerationLimitedPluginis enabled.Happy to send a PR for whichever you prefer.
Related
#3008 also involves the smoothing plugin selected in this file, but is about the Ruckig plugin's runtime behaviour rather than the config being loadable.
ROS Distro
Jazzy
OS and version
Ubuntu 24.04 (container)
Source or binary build?
Binary
If binary, which release version?
The exact patch version was not recorded. The report does not depend on it: I verified the file, the plugin's
update_perioduse and the launch-file workaround directly in the upstream tree on bothmain(current) andjazzy(4d84106) — identical on both.Which RMW are you using?
Default (rmw_fastrtps_cpp)
Steps to Reproduce
servo_nodeand passes the shipped config the direct way, rather than throughParameterBuilder:Launch it. It fails on the missing top-level key.
Fix that by wrapping the file in
/**: ros__parameters:, and launch again. It now gets further and fails insideAccelerationLimitedPlugin, becauseupdate_periodis still unset — that value only exists in the demo launch files.Expected behavior
Either the shipped config loads as a parameter file and brings up servo with the smoothing plugin it selects, or the file states that it is not a standalone parameter file and lists what has to be supplied alongside it.
Actual behavior
The file cannot be loaded directly, and once that is worked around, the smoothing plugin it enables by default fails on a parameter the file does not contain.