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

Enable mdof trajectory execution #2740

Merged
merged 14 commits into from
Apr 30, 2024

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Mar 13, 2024

This implements preparative steps to enable mobile base trajectory execution using ros2_control JTC.

In particular, these changes convert mdof trajectories into joint trajectories using the local variables which in turn can be supported by ros2_control already.

An example for a functioning setup with the Stretch robot + JTC + chained DiffDrive is provided in PickNikRobotics/stretch_ros#48

Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Overall, I like this change, nice work!

}
}
// clear velocities if we have an incomplete specification
if (joint_trajectory.points[i].velocities.size() != onedof.size())
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe print and error message here, since this probably indicates a bug somewhere

@henningkayser henningkayser self-assigned this Mar 20, 2024
@henningkayser henningkayser changed the title WIP: Enable mdof trajectory execution Enable mdof trajectory execution Mar 26, 2024
@henningkayser henningkayser marked this pull request as ready for review March 26, 2024 14:27
@henningkayser
Copy link
Member Author

I've put some of the open requests into #2773, since they seem out of scope of the new free function to me. I'm happy to provide API cleanup after mdof support has been completed.

Comment on lines 748 to 753
if (!joint_trajectory.joint_names.empty())
{
joint_trajectory.header.frame_id = robot_model->getModelFrame();
joint_trajectory.header.stamp = rclcpp::Time(0, 0, RCL_ROS_TIME);
joint_trajectory.points.resize(trajectory.getWayPointCount());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!joint_trajectory.joint_names.empty())
{
joint_trajectory.header.frame_id = robot_model->getModelFrame();
joint_trajectory.header.stamp = rclcpp::Time(0, 0, RCL_ROS_TIME);
joint_trajectory.points.resize(trajectory.getWayPointCount());
}
if (joint_trajectory.joint_names.empty())
{
return std::nullopt;
}
joint_trajectory.header.frame_id = robot_model->getModelFrame();
joint_trajectory.header.stamp = rclcpp::Time(0, 0, RCL_ROS_TIME);
joint_trajectory.points.resize(trajectory.getWayPointCount());

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we just exit the function here if joint_names is empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

with the updated logic, I would just keep it the way it is probably

Copy link
Member

@EzraBrooks EzraBrooks left a comment

Choose a reason for hiding this comment

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

This is working in Pro and the CI failure appears to be a flaky test.

I will re-run the CI here a few times to see if it passes, but otherwise I propose we open another PR to temporarily disable the flaky test.

@EzraBrooks EzraBrooks mentioned this pull request Apr 30, 2024
6 tasks
@EzraBrooks EzraBrooks added this pull request to the merge queue Apr 30, 2024
Merged via the queue into moveit:main with commit 53627d3 Apr 30, 2024
4 of 8 checks passed
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.

None yet

5 participants