Skip to content
This repository was archived by the owner on Jul 10, 2025. It is now read-only.

Port wheel slip plugin to ros2 (eloquent)#1099

Merged
scpeters merged 12 commits intoros-simulation:eloquentfrom
scpeters:wheel_slip_plugin_eloquent
Aug 13, 2020
Merged

Port wheel slip plugin to ros2 (eloquent)#1099
scpeters merged 12 commits intoros-simulation:eloquentfrom
scpeters:wheel_slip_plugin_eloquent

Conversation

@scpeters
Copy link
Copy Markdown
Member

@scpeters scpeters commented May 7, 2020

This ports the wheel slip plugin from melodic (#995) to ros2. It uses ros2 parameters instead of dynamic reconfigure.

An example world is provided that shows two trisphere_cycle models that each have two instances of the wheel slip plugin, allowing separate parameters for the front and rear wheels. The demo world has some example ros2 param commands to adjusting the parameters dynamically.

@scpeters scpeters requested a review from chapulina May 7, 2020 01:17
@chapulina chapulina added the ros2 label May 7, 2020
@scpeters
Copy link
Copy Markdown
Member Author

I may need to rework this a bit to match the changes in #1111

@scpeters
Copy link
Copy Markdown
Member Author

@jacobperron considering the change in #1126 in noetic that attempts to use the parameter value specified in the launch file, what would the equivalent be for ros2? I'm guessing this PR might need a change to match that behavior

@scpeters
Copy link
Copy Markdown
Member Author

I can make a new branch and target to foxy as well

@chapulina
Copy link
Copy Markdown
Contributor

I can make a new branch and target to foxy as well

Note that technically Foxy has already been released (bloomed but not synced yet, still in ros2-testing). So if you really want to break behaviour on Foxy, I'd recommend doing it soon. Otherwise, targeting Galactic.

@scpeters
Copy link
Copy Markdown
Member Author

Note that technically Foxy has already been released (bloomed but not synced yet, still in ros2-testing). So if you really want to break behaviour on Foxy, I'd recommend doing it soon. Otherwise, targeting Galactic.

This is adding a new plugin, so I don't think it breaks anything

@jacobperron
Copy link
Copy Markdown
Collaborator

jacobperron commented Jun 25, 2020

@jacobperron considering the change in #1126 in noetic that attempts to use the parameter value specified in the launch file, what would the equivalent be for ros2? I'm guessing this PR might need a change to match that behavior

I don't think we have the same problem in ROS 2, since native parameters are reconfigurable. A parameter passed to the node in a launch file should override the default value in declare_parameter.

scpeters added 6 commits July 14, 2020 13:32
Similar to the change in ros-simulation#1111, moves the WheelSlipPlugin::Load
call before the parameter callback so that the values from SDF/URDF
are set first.
Then the callback is changed to ignore negative values, and the
default slip parameter values are set to -1, so that the SDF/URDF
values are still preferred unless a different parameter value
is specified in a launch file.
@scpeters scpeters force-pushed the wheel_slip_plugin_eloquent branch from 129b6bb to 00257a6 Compare July 14, 2020 21:36
@scpeters
Copy link
Copy Markdown
Member Author

I rebased and changed the behavior in 00257a6 to match the change in #1111 / #1126.

scpeters added 4 commits July 15, 2020 14:41
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
return result;
};

impl_->ros_node_->add_on_set_parameters_callback(param_change_callback);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to double-check, but I think we should maintain a reference to the returned callback handle, otherwise the callback may be unregistered.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it looks like this was done in ros2/rclcpp#1136

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Copy Markdown
Member Author

@osrf-jenkins run tests please

2 similar comments
@scpeters
Copy link
Copy Markdown
Member Author

@osrf-jenkins run tests please

@nuclearsandwich
Copy link
Copy Markdown

@osrf-jenkins run tests please

@scpeters
Copy link
Copy Markdown
Member Author

some unrelated test failures, but CI is finally working

@scpeters scpeters merged commit eea082d into ros-simulation:eloquent Aug 13, 2020
@scpeters scpeters deleted the wheel_slip_plugin_eloquent branch August 13, 2020 05:42
scpeters added a commit to scpeters/gazebo_ros_pkgs that referenced this pull request Aug 13, 2020
Initial port of gazebo_ros_wheel_slip_plugin to ros2.

It includes a similar change to ros-simulation#1111, which moved the
WheelSlipPlugin::Load call before the parameter callback
so that the values from SDF/URDF are set first.
Then the callback is changed to ignore negative values, and the
default slip parameter values are set to -1, so that the SDF/URDF
values are still preferred unless a different parameter value
is specified in a launch file.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit that referenced this pull request Aug 17, 2020
Forward port of wheel slip plugin from eloquent (#1099)
to foxy.

It includes a similar change to #1111, which moved the
WheelSlipPlugin::Load call before the parameter callback
so that the values from SDF/URDF are set first.
Then the callback is changed to ignore negative values, and the
default slip parameter values are set to -1, so that the SDF/URDF
values are still preferred unless a different parameter value
is specified in a launch file.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants