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

Allow setting minimum timestamp delta between trajectory points in Iterative Spline Parameterization #574

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

schornakj
Copy link
Contributor

@schornakj schornakj commented Jan 6, 2025

I ran into a situation where I needed to enforce that the differences between timestamps for consecutive trajectory points were all greater than some minimum threshold. Briefly, this is because some robot controllers can't execute trajectories with arbitrarily-fine time resolution and need to have (for example) at least a 1ms difference between each trajectory point.

One way to resolve this while ensuring that the trajectory is still smooth is to enforce this time delta threshold during the time parameterization stage. This lets us stretch out the trajectory between two waypoints if needed while ensuring that the robot's path for this trajectory segment follows the same positions but just at a slower speed.

I'm using ISP specifically so I only implemented this change there. I did add this parameter to the time parameterizer base class to allow implementing it elsewhere as well. Let me know if there's a better way to approach this, though.

I have to test this more thoroughly on my system and then update the branch to the latest state of main, so I'll keep this PR as a draft until I do both those things.

@rjoomen
Copy link
Contributor

rjoomen commented Jan 6, 2025

Looking at MoveIt, I noticed that the TOTG algorithm nowadays supports spacing waypoints equally in time. As the TOTG algorithm of Tesseract is forked from MoveIt, I think the best solution would be to bring Tesseract TOTG up to date with the MoveIt(1)/MoveIt2 version. This was on my wishlist anyway, as this would bring in some improvements/fixes as well (see here and here), so I'm willing to help.

Also, TOTG is proven to be superior to ISP, hence MoveIt2 dropped ISP, another reason for switching.

@gavanderhoorn
Copy link

@schornakj: is this related to Yaskawa-Global/motoros2#255?

@schornakj
Copy link
Contributor Author

schornakj commented Jan 7, 2025

@schornakj: is this related to Yaskawa-Global/motoros2#255?

@gavanderhoorn Yes, this PR is where I ended up after digging into my motion planning stack to diagnose the issue I described in my comment in motoros2. I'll keep discussion about that primarily here so I don't just totally hijack the other issue

Looking at MoveIt, I noticed that the TOTG algorithm nowadays supports spacing waypoints equally in time. As the TOTG algorithm of Tesseract is forked from MoveIt, I think the best solution would be to bring Tesseract TOTG up to date with the MoveIt(1)/MoveIt2 version. This was on my wishlist anyway, as this would bring in some improvements/fixes as well (see here and here), so I'm willing to help.

Also, TOTG is proven to be superior to ISP, hence MoveIt2 dropped ISP, another reason for switching.

@rjoomen I agree that ultimately it's a good idea to move over to TOTG. The MoveIt folks decided that it's a better use of maintainer effort to just have one time parameterization approach that works well, so they removed the alternatives. It's probably a good idea to do the same thing in Tesseract for the same reasons.

Aside from that, my main concern would be that enforcing equal time spacing of trajectory points is different from enforcing a minimum time spacing between points. To ensure that TOTG never outputs a trajectory with points spaced by less than 1ms I'd need to calculate the equal sampling parameters for each trajectory given the trajectory's total duration. I'd need to dig into MoveIt's TOTG implementation in more detail to know for sure but my suspicion would be that these are two related but unique problems which need different solutions.

@Levi-Armstrong do you think this is a useful setting to expose within the time parameterization profiles? As long as there's a way to make it internally default to epsilon it's essentially an optional setting that wouldn't require changes to user code.

@Levi-Armstrong
Copy link
Contributor

@marip8 What are your thoughts on this?

@schornakj schornakj force-pushed the joe/time-parameterization-minimum-delta branch from b599c31 to 2b3eee9 Compare February 7, 2025 20:33
@schornakj schornakj force-pushed the joe/time-parameterization-minimum-delta branch from 2b3eee9 to 386a096 Compare February 7, 2025 20:38
@schornakj schornakj marked this pull request as ready for review February 7, 2025 20:45
@schornakj
Copy link
Contributor Author

@Levi-Armstrong @marip8 alrighty, this is ready for review.

I've been running this branch on my robot (a Yaskawa GP180-120 with a YRC1000 controller) and it resolves the issue I described in my comment in Yaskawa-Global/motoros2#255. I added some tests to show that the new parameter does have an effect on the trajectory timestamps too.

MotoROS2 cannot execute trajectories with time deltas less than 1ms, so a change like this one is required to prevent the time parameterizer from generating a trajectory that the robot cannot execute.

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.

4 participants