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 vel and acc scaling to computeCartesianPath in Humble #3048

Closed
wants to merge 1 commit into from
Closed

Add vel and acc scaling to computeCartesianPath in Humble #3048

wants to merge 1 commit into from

Conversation

liamcarlos
Copy link

Hi there,

I updated to Iron to get this capability but noticed Iron's EOL is Friday, so I thought I'd suggest this change for Humble since it has another couple years to EOL

Description

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

…le branch move_group_interface.cpp

I updated to Iron to get this capability but noticed Iron's EOL is Friday, so I thought I'd suggest this change for Humble since it has another couple years to EOL
Copy link

mergify bot commented Oct 30, 2024

Please target the main branch for development, we will backport the changes to humble for you if approved and if they don't break API.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

I don't think, computeCartesianPath handles timing at all. It just computes path waypoints. Hence, forwarding the scaling factors does not change anything.
Did you observe any change?

@mikeferguson
Copy link
Contributor

This appears to be a back port of #1968? But maybe is missing a line from that PR? And also depends on changes to moveit_msgs that haven't been back ported to humble either AFAICT?

@liamcarlos liamcarlos changed the base branch from humble to main October 30, 2024 14:34
Copy link

mergify bot commented Oct 30, 2024

This pull request is in conflict. Could you fix it @liamcarlos?

@liamcarlos
Copy link
Author

@rhaschke It does, I tested it with a UR10e in Iron. Those lines just seem to be missing from the Humble branch.. Also should I switch base back to humble branch? The bot threw me off..

@liamcarlos liamcarlos changed the base branch from main to humble October 30, 2024 14:40
Copy link

mergify bot commented Oct 30, 2024

Please target the main branch for development, we will backport the changes to humble for you if approved and if they don't break API.

@rhaschke
Copy link
Contributor

As pointed out in #3048 (comment), your PR is missing another line from #1968. Moreover, the corresponding message fields are missing. Probably, those API changes are the reason, why this PR wasn't backported to Humble.

@liamcarlos
Copy link
Author

@rhaschke Yes I noticed that, I'm making that change right now! Thanks @mikeferguson !

Also found the srv file(GetCartesianPath.srv) that is missing those request fields. It's missing some other fields i.e. cartesian_speed_limited_link and max_cartesian_speed, but I don't know where those are being used.. I really just want to add velocity and acceleration control to the Humble branch since it will having longer support.

@liamcarlos liamcarlos closed this by deleting the head repository Oct 30, 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