Skip to content
This repository has been archived by the owner on Nov 13, 2017. It is now read-only.

make iterative time parameterization tool use KinematicTrajectory #35

Closed
isucan opened this issue Jan 30, 2013 · 11 comments
Closed

make iterative time parameterization tool use KinematicTrajectory #35

isucan opened this issue Jan 30, 2013 · 11 comments
Assignees

Comments

@isucan
Copy link
Contributor

isucan commented Jan 30, 2013

No description provided.

@ghost ghost assigned adamantivm May 3, 2013
@isucan
Copy link
Contributor Author

isucan commented May 3, 2013

In the moveit_core package, the trajectory_processing library includes the IterativeParabolicTimeParameterization class; That class computes time staps for the waypoints of the robot along a given trajectory.

The issue that needs to be resolved is that the computeTimeStamps() function should operate directly on the robot_trajectory::RobotTrajectory class; currently, it instead converts the trajectory to a message (trajectory_msgs::JointTrajectory) then calls the computeTimeStamps() variant that operates on the message and then converts back to a RobotTrajectory.

What needs to be done is that the computation needs to be performed directly on the RobotTrajectory class, in place (no copying of states) and the version of computeTimeStamps() that operates on the message should be implemented in terms of the function that operates directly on RobotTrajectory, using conversion functions.

tulku added a commit to tulku/moveit_core that referenced this issue May 10, 2013
Added the RobotTrajectory object to the methods that need to be
converted to test the new code against the results obtained from using
JointTrajectory.

Converted applyVelocityConstraints to use only data from the
RobotTrajectory object.

Made the JointTrajectory versions of computeTimeStamps private to ensure
that they are not used anywhere.
@tulku
Copy link
Contributor

tulku commented May 10, 2013

We have started working on this and committed the first changes to my fork.

I have a question, in the bug report you state that the original JointTrajectory should still be available as wrappers of the RobotTrajectory ones, whoever, they are not being use by anyone right one. Are they really necessary? (I've made them private and everything compiles fine).

@aleeper
Copy link

aleeper commented May 10, 2013

I think we should keep them; it makes it easier for folks who might already have a JointTrajectory and want to manually call the time parameterization in their own code (which I have done sometimes).

@isucan
Copy link
Contributor Author

isucan commented May 12, 2013

sorry for the late reply - pretty bad internet at the conf and the schedule was a bit hectic.
the code looks great, thanks! the functions you mention are indeed not used anywhere, but i agree with adam about keeping them. the implementation should be simple anyway, so there is no problem having them. we will just include in the docs the fact that conversions are performed and the use is not encouraged.

@tulku
Copy link
Contributor

tulku commented May 12, 2013

Ok, I'll leave them public using the conversions. Just asked to avoid possible dead code. Thanks!

tulku added a commit to tulku/moveit_core that referenced this issue May 14, 2013
When updating the "updateTrajectory" function to not use
trajectory_msgs::JointTrajectory I ended up in some code that seems old
and not longer in use.  I think that velocities and accelerations
calculated in this function are not used anywhere, that's why I created
a new updateTrajectory which just updates the time between waypoints
using the previously calculated time_diff vector.
@tulku
Copy link
Contributor

tulku commented May 14, 2013

Hi!

I have been spending quite some time now finishing this patch and run into a problem.

Part of the process that's done to the trajectory_msgs::JointTrajectory is calling a function called updateTrajectory which is suppose to "Takes the time differences, and updates the values in the trajectory". However this function also makes some velocity (although the code is commented) and acceleration calculations. This new values are stored in the JointTrajectory message.

It is very difficult to track where the velocity and acceleration information is stored in the way back from a trajectory_msgs::JointTrajectory to a robot_trajectory::RobotTrajectory. It actually seems that the values are discarded and that the only important result from the entire computeTimeStamps() method is, not surprisingly, to calculate new, smoother timestamps and modify the robot_trajectory::RobotTrajectory. The velocity values in a point are added to the JointStates, using some functions that say that are old and should not be used.

I have just pushed a new changeset with a new updateTrajectory that only updates the timestamps of the waypoints according to the previously calculated time_diff. If this is correct I should finish this issue tomorrow. (I still need to update the computeTimeStamps() functions.)

@isucan
Copy link
Contributor Author

isucan commented May 15, 2013

@tulku The velocities are stored for a point indeed, and previously they were stored in the message, for each trajectory point. When using the RobotTrajectory class, you will have to store the velocities in the RobotState instances, using the functions you found. The reason they are marked for removal is because they are horribly inefficient in terms of memory allocation for states. Nevertheless, the functions will remain there in the updated version of RobotState, so you should use them and we should not discard computed velocities.

tulku added a commit to tulku/moveit_core that referenced this issue May 15, 2013
Discarded the timestamps-only updateTrajectory and updated the original
one to update the time_from_previous waypoints in the RobotTrajectory
and the velocities and accelerations in every joint / waypoint.
@tulku
Copy link
Contributor

tulku commented May 15, 2013

@isucan Thanks for that information. If I understood you correctly, to update the RobotTrajectory we should:

  • Update the duration_from_previous_ queue using the robot_trajectory::setWayPointDurationFromPrevious() taking the values from the time_diff vector.
  • Update the velocities of each joint in each state of the waypoints_ queue asking for the velocities vector of each joint using joint_state::getVelocities() and then changing it's values.
  • Update the accelerations in the same way as the velocities but using joint_state::getAccelerations().

This is done in updateTrajectory function of latest commit, which compiles but at the planner wont work correctly because the JointTrajectory msg is not correctly set and the later transformations fail.

@isucan
Copy link
Contributor Author

isucan commented May 15, 2013

Yep, this sounds good.

tulku added a commit to tulku/moveit_core that referenced this issue May 15, 2013
This commit should finish the transition from the previous
Joint Trajectory based timestamps calculations to making the calculations
in place in RobotTrajectory.

The new wrappers which take a JointTrajectory are still missing.
tulku added a commit to tulku/moveit_core that referenced this issue May 16, 2013
While working on issue moveit#35, three functions generated the active_joints
string vector. Now it is only generated in updateTimestamps and passed
as reference to the functions that use it.
isucan added a commit that referenced this issue May 20, 2013
@acornacorn
Copy link

Is this issue complete? Can I close it?

@isucan
Copy link
Contributor Author

isucan commented Nov 26, 2013

This has been done for a while now, but I forgot to close it.

@isucan isucan closed this as completed Nov 26, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants