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

Forward trajectory controller #944

Open
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

URJala
Copy link
Collaborator

@URJala URJala commented Mar 11, 2024

The controller should be able to forward a cartesian tracjectory to the robot for interpolation.
Creates errors on compilation due to multiple definition of functions, when inheriting from the joint_trajectory_controller. But looking at the passthrough controllers from ROS1 that may not be the right controller to use anyway?
All inputs are welcome.

closes #839

@RobertWilbrandt
Copy link
Collaborator

I cannot reproduce your build errors, are you up-to-date with the packages listed in Universal_Robots_ROS2_Driver.rolling.repos?

As for the general idea, i don't think the forwarding controller should be derived from the JTC at all as it doesn't do any spline interpolation itself. You can just derive directly from controller_interface::ControllerInterface.

The main problem we will have is encoding the trajectory in a way that it reaches the hardware interface. In ROS 1, we could just define TrajectoryInterface and use it in the hardware and controller. This is not really possible in ROS 2, as you access LoanedStateInterfaces from the controller that only give you access to one double value each. I know that some people are working on allowing more complex interface types, but am also not sure if this would help us (as we ideally want to use data that does not have a statically defined size). @fmauch do you know about the current state of these efforts?

@URJala
Copy link
Collaborator Author

URJala commented Mar 14, 2024

I am up-to-date on everything, as far as i can tell, but I have started working while deriving from ControllerInterface and that works fine, so i will just continue on that.
Mads and i talked about making an action_server in the controller that will send over one point at a time to the robot, that way we can have a variable input, while only using the space required for one point in the state interface. This sounds like a good way of doing it to me.

@URJala
Copy link
Collaborator Author

URJala commented May 2, 2024

The controller is now functional, and you should be able to test it out yourself. A user can give the controller a trajectory to forward via the action server running on the controller. It is possible to define trajectories with just positions, or with both positions and velocities and/or accelerations. The trajectory is transferred to the hardware interface via the command interfaces, before the hardware interface then transfers it to the robot.
Only one trajectory will be accepted at a time, if one is executing others will be denied.
The controller monitors the execution time of the trajectory, and gives a warning if it takes too long, although it isn't counting correctly currently (It warns too early). This has no effect on the execution of the trajectory.
When the robot driver is launched this controller is not running, and needs to be loaded/activated manually, while the scaled joint trajectory controller is unloaded, as they cannot run at the same time. The example load_switch_controller_example.py does this. The robot needs to be running while this is done, so the controller can be loaded correctly. (This should be fixed, so it doesn't need to run, but i don't know how currently.)
The examples example_movej.py and example_spline.py send a trajectory with only positions and one with positions, velocities and accelerations, respectively.
Let me know what you think of the construction of the controller and the communication between the controller and the hardware interface.

@fmauch fmauch self-requested a review May 3, 2024 10:00
@URJala URJala marked this pull request as ready for review June 14, 2024 14:05
@URJala URJala force-pushed the forward_controller branch 2 times, most recently from 20fe4a6 to a8b83ff Compare July 16, 2024 07:29
Copy link
Collaborator

@fmauch fmauch 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 looking very promising but I think we have to iterate this a couple of times, since this is a rather big change :-)

examples/passthrough_example.py Outdated Show resolved Hide resolved
examples/scaled_joint_example.py Outdated Show resolved Hide resolved
examples/robot_class.py Outdated Show resolved Hide resolved
ur_controllers/src/passthrough_trajectory_controller.cpp Outdated Show resolved Hide resolved
ur_controllers/src/passthrough_trajectory_controller.cpp Outdated Show resolved Hide resolved
ur_controllers/src/passthrough_trajectory_controller.cpp Outdated Show resolved Hide resolved
ur_controllers/src/passthrough_trajectory_controller.cpp Outdated Show resolved Hide resolved
ur_controllers/src/passthrough_trajectory_controller.cpp Outdated Show resolved Hide resolved
Creates errors on compilation due to multiple definition of funtions.
…upports passing through accelerations and velocities.
The number of joints in the controller is variable and received from the hardware interface. (It is not variable there though).
The internal action server uses the action type FollowJointTrajectory. Both goal time tolerance and goal position tolerance are checked upon completion of trajectory,
and feedback is sent to the user.
The integration test has also been updated to test the controller.
Did some cleanup of comments and print statements.
Improved the examples, so they are easier to use.
Moved functionality of passthrough controller from separate execute loop in to controller's update method.
Also changed how the controller checks validity of a received trajectory, and how it communicates that to the hardware interface.
Command and state interfaces are now only interacted with within the update method.
The update method still triggers some action server callbacks, so it is not threadsafe yet, but i dont know how to trigger the action server without having them in the update method.
@URJala URJala requested a review from fmauch September 18, 2024 02:28
Comment on lines +16 to +17
url: https://github.com/pal-robotics-forks/ros2_control.git
version: fix/controller_update_period
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
url: https://github.com/pal-robotics-forks/ros2_control.git
version: fix/controller_update_period
url: https://github.com/ros-controls/ros2_control.git
version: master

Comment on lines +16 to +17
url: https://github.com/pal-robotics-forks/ros2_control.git
version: fix/controller_update_period
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
url: https://github.com/pal-robotics-forks/ros2_control.git
version: fix/controller_update_period
url: https://github.com/ros-controls/ros2_control.git
version: master

Make passthrough command interfaces part of the joints and fix claiming conflicts
@fmauch fmauch requested a review from VinDp October 3, 2024 12:18
If we want to use other types for forwarding such as Cartesian setpoints
or motion primitives it doesn't really make sense to have this as a part
of the joints.
This should be thread-safe in contrast to calling it from the AsyncIO
thread.
Copy link
Collaborator

@VinDp VinDp left a comment

Choose a reason for hiding this comment

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

Works nicely and looks good, I just have some minor suggestions.


.. _passthrough_trajectory_controller:

ur_controlers/PassthroughTrajectoryController
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ur_controlers/PassthroughTrajectoryController
ur_controllers/PassthroughTrajectoryController

Comment on lines +224 to +229
<command_interface name="setpoint_velicities_0"/>
<command_interface name="setpoint_velicities_1"/>
<command_interface name="setpoint_velicities_2"/>
<command_interface name="setpoint_velicities_3"/>
<command_interface name="setpoint_velicities_4"/>
<command_interface name="setpoint_velicities_5"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<command_interface name="setpoint_velicities_0"/>
<command_interface name="setpoint_velicities_1"/>
<command_interface name="setpoint_velicities_2"/>
<command_interface name="setpoint_velicities_3"/>
<command_interface name="setpoint_velicities_4"/>
<command_interface name="setpoint_velicities_5"/>
<command_interface name="setpoint_velocities_0"/>
<command_interface name="setpoint_velocities_1"/>
<command_interface name="setpoint_velocities_2"/>
<command_interface name="setpoint_velocities_3"/>
<command_interface name="setpoint_velocities_4"/>
<command_interface name="setpoint_velocities_5"/>

"""""""""""""""""""""""""""""""""

* A trajectory passed to the controller will be sent to the hardware component one by one.
* The controller will send one setpooint and then wait for the hardware to acknowledge that it can
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* The controller will send one setpooint and then wait for the hardware to acknowledge that it can
* The controller will send one setpoint and then wait for the hardware to acknowledge that it can


/*
* 0.0: No trajectory to forward, the controller is idling and ready to receive a new trajectory.
* 1.0: The controller has received and accepted a new trajecotry. When the state is 1.0, the controller will write a
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* 1.0: The controller has received and accepted a new trajecotry. When the state is 1.0, the controller will write a
* 1.0: The controller has received and accepted a new trajectory. When the state is 1.0, the controller will write a

Comment on lines +151 to +156
} else if (interface_it->get_interface_name() == "velocity") {
joint_velocity_state_interface_.emplace_back(*interface_it);
}
} else if (interface_it->get_interface_name() == "acceleration") {
joint_acceleration_state_interface_.emplace_back(*interface_it);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there one unwanted extra parenthesis before the acceleration case or is it intended?

Suggested change
} else if (interface_it->get_interface_name() == "velocity") {
joint_velocity_state_interface_.emplace_back(*interface_it);
}
} else if (interface_it->get_interface_name() == "acceleration") {
joint_acceleration_state_interface_.emplace_back(*interface_it);
}
} else if (interface_it->get_interface_name() == "velocity") {
joint_velocity_state_interface_.emplace_back(*interface_it);
} else if (interface_it->get_interface_name() == "acceleration") {
joint_acceleration_state_interface_.emplace_back(*interface_it);
}


This controller uses a ``control_msgs/FollowJointTrajectory`` action but instead of interpolating
the trajectory on the ROS pc it forwards the complete trajectory to the robot controller for
interpolation and execution. This way, the realtime requirements for the control PC.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"This way, the realtime requirements for the control PC." → isn't the sentence missing something?


if (current_index_ == 0 && current_transfer_state == TRANSFER_STATE_IDLE) {
active_trajectory_elapsed_time_ = rclcpp::Duration(0, 0);
rclcpp::Duration::from_seconds(duration_to_double(active_joint_traj_.points.back().time_from_start));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line useful? Because it is not assigned to anything and then it's repeated immediately after but assigned to max_trajectory_time

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.

Passthrough controllers
5 participants