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

Set rotation value of cartesian MaxEEFStep by default #2614

Merged
merged 2 commits into from
Apr 21, 2021

Conversation

felixvd
Copy link
Contributor

@felixvd felixvd commented Apr 20, 2021

I was trying to plan a pure rotation today and noticed that max_step did not apply to the rotation anymore. If the motion is a pure rotation, move_group.computeCartesianPath() returns no intermediate points. It looks like this bug was introduced when we added CartesianInterpolator, which sets the rotation value of MaxEEFStep to zero by default.

This PR fixes the constructor of MaxEEFStep and sets the rotation to a reasonable value if it is not set. I chose a value of 1 cm = 2 deg, but I would be fine with 1 cm = 1 deg as well.

@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@224129a). Click here to learn what that means.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2614   +/-   ##
=========================================
  Coverage          ?   60.49%           
=========================================
  Files             ?      402           
  Lines             ?    29614           
  Branches          ?        0           
=========================================
  Hits              ?    17912           
  Misses            ?    11702           
  Partials          ?        0           
Impacted Files Coverage Δ
...nclude/moveit/robot_state/cartesian_interpolator.h 85.72% <0.00%> (ø)
...al_motion_planner/src/trajectory_generator_ptp.cpp 98.94% <75.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 224129a...9510d2d. Read the comment docs.

Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

looks good. Please consider my comment.

@v4hn v4hn merged commit a3a4d8f into moveit:master Apr 21, 2021
@felixvd felixvd deleted the limit-rotation-in-cartesian-plan branch April 23, 2021 05:33
@tylerjw tylerjw mentioned this pull request Apr 29, 2021
v4hn added a commit to tylerjw/moveit that referenced this pull request Apr 29, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request Apr 29, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request May 3, 2021
tylerjw pushed a commit that referenced this pull request May 3, 2021
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

2 participants