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

Add hold_joints parameter #251

Merged

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Dec 31, 2023

@HPCLOL posted a use case of gazebo in ros-controls/ros2_controllers#875:
If the simulation is started with an inactive controller, the robot should move with gravity as if the joints were passive.

As discussed with #250 this is currently not possible. This PR adds an parameter to enable this behavior (again).

  • I'm not sure if this should be default true or false.
  • I haven't found a different solution to tell GazeboSystem this parameter parsed from the gazebo plugin without changing API. Therefore, I used a ROS parameter of the gazebo_ros2_control node to pass the information.

The result looks like this if no controller was spawned:

passive_rrbot.mp4

@christophfroehlich christophfroehlich changed the title Hold joints parameter Add hold_joints parameter Dec 31, 2023
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Thanks for the change! LGTM!

After discussion with @christophfroehlich
We can add a new option for now, as we are limited to one control mode per run of the simulation. Ultimately, I think it will make sense to be able to switch the control modes dynamically once the simulation starts. We can try to aim this for Jazzy. If we can get this switching done, then we can remove this option, as we can achieve the same by switching the control mode to effort.

Ideally, the behavior would be at startup we will have the robot maintaining the position, and then when you switch from position or velocity active controllers to inactive mode, they maintain the old reference, and for the effort based controller, the reference will be set to zero.

What do you think about this new approach for future?

@@ -128,6 +131,18 @@ bool GazeboSystem::initSim(
return false;
}

try {
this->dataPtr->hold_joints_ = this->nh_->get_parameter("hold_joints").as_bool();
} catch (const std::exception & e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Include exception

@@ -306,6 +312,11 @@ void GazeboRosControlPlugin::Load(gazebo::physics::ModelPtr parent, sdf::Element
RCLCPP_DEBUG(
impl_->model_nh_->get_logger(), "Loaded hardware interface %s!",
robot_hw_sim_type_str_.c_str());
try {
node_ros2->declare_parameter("hold_joints", rclcpp::ParameterValue(hold_joints));
} catch (const std::exception & e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Include exception

@christophfroehlich
Copy link
Contributor Author

@ahcorde not exactly sure what you requested, I added now explicit catch statements for the exceptions written in the docstring of the rclcpp functions. Or do you have another resource on how to write that properly?

@ahcorde
Copy link
Collaborator

ahcorde commented Jan 1, 2024

@ahcorde not exactly sure what you requested, I added now explicit catch statements for the exceptions written in the docstring of the rclcpp functions. Or do you have another resource on how to write that properly?

My suggestion was about

#include <exception>

But this change is also fine

@ahcorde ahcorde merged commit 7682dce into ros-controls:master Jan 1, 2024
4 of 5 checks passed
@christophfroehlich christophfroehlich deleted the hold_joints_parameter branch January 1, 2024 20:45
@christophfroehlich
Copy link
Contributor Author

@ahcorde not exactly sure what you requested, I added now explicit catch statements for the exceptions written in the docstring of the rclcpp functions. Or do you have another resource on how to write that properly?

My suggestion was about

#include <exception>

But this change is also fine

Ah 😁 I thought too complicated. Thx!

@christophfroehlich
Copy link
Contributor Author

@ahcorde Could we backport this please? rolling won't be released any more.

@christophfroehlich
Copy link
Contributor Author

@Mergifyio backport humble iron

Copy link
Contributor

mergify bot commented Jul 6, 2024

backport humble iron

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jul 6, 2024
(cherry picked from commit 7682dce)

# Conflicts:
#	gazebo_ros2_control/src/gazebo_system.cpp
mergify bot pushed a commit that referenced this pull request Jul 6, 2024
(cherry picked from commit 7682dce)

# Conflicts:
#	gazebo_ros2_control/src/gazebo_system.cpp
ahcorde pushed a commit that referenced this pull request Jul 8, 2024
ahcorde pushed a commit that referenced this pull request Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants