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.

Sign up for GitHub

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

Jump to bottom

Jog arm dimensions #1724

Merged
merged 10 commits into from
May 8, 2020
Merged

Jog arm dimensions #1724

merged 10 commits into from
May 8, 2020

Conversation

AdamPettinger
Copy link
Contributor

@AdamPettinger AdamPettinger commented Nov 11, 2019 •
edited
Loading

Description

Adds a feature to moveit_jog_arm to allow you to "disable" specific directions when jogging. This is particularly useful for delicate (e.g. contact) tasks, so the user can remove unwanted small commands in directions they want to be stationary in. Examples: in an insertion task you can restrict motion to the EE forward/backward direction, you can only allow pure translation/rotation, or you can keep the EE in a specific plane.

The active directions can be changed live with a service call (change_control_dimensions). The input is a 6-long boolean array [translation-x, translation-y, translation-z, rotation-x, rotation-y, rotation-z] where passing a false sets the commanded velocity in that direction to 0, regardless of the user input. Note that the directions are defined in the commanded velocity frame

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

@welcome
Copy link

welcome bot commented Nov 11, 2019

Thanks for helping in improving MoveIt

@AndyZe
Copy link
Member

AndyZe commented Nov 14, 2019 •
edited
Loading

Cool, I've been wanting this feature for awhile.

Another related feature would be to allow the arm to drift in specified dimensions. Useful for passing through a singularity if you don't care about, say, wrist rotation. But we can add that later, and your PR will make it easier, I think.

AndyZe
AndyZe reviewed Nov 14, 2019
View reviewed changes
Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

The build error that Travis has turned up is:

fatal error: moveit_jog_arm/ChangeControlDimensions.h: No such file or directory
#include <moveit_jog_arm/ChangeControlDimensions.h>

@AndyZe
Copy link
Member

AndyZe commented Nov 20, 2019 •
edited
Loading

Note that CI passes except for catkin_lint. The catkin_lint error is:

moveit_ros_planning_interface: CMakeLists.txt(11): error: 'eigenpy' in find_package(catkin) is not a catkin package

So it seems to be an issue with moveit_ros_planning_interface, not with Adam's changes.

Related issue: #1536

AndyZe
AndyZe approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

I tested this locally and it works fine. The integration test still passes. I hope the catkin_lint issue from the other package doesn't hold this up from getting merged but I'm not sure how to resolve that issue.

@gavanderhoorn
Copy link
Contributor

I tested this locally and it works fine. The integration test still passes. I hope the catkin_lint issue from the other package doesn't hold this up from getting merged but I'm not sure how to resolve that issue.

A rebase should fix this, as #1737 was merged recently.

davetcoleman
davetcoleman reviewed Nov 20, 2019
View reviewed changes
Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Let's move the msgs out of here

moveit_experimental/moveit_jog_arm/CMakeLists.txt Outdated Show resolved Hide resolved
moveit_experimental/moveit_jog_arm/README.md Outdated Show resolved Hide resolved
moveit_experimental/moveit_jog_arm/package.xml Outdated Show resolved Hide resolved
rhaschke
rhaschke requested changes Nov 20, 2019
View reviewed changes
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.

Please move the service message into moveit_msgs.

moveit_experimental/moveit_jog_arm/package.xml Outdated Show resolved Hide resolved
AdamPettinger added a commit to UTNuclearRoboticsPublic/moveit_tutorials that referenced this pull request Nov 25, 2019
@AdamPettinger
Update for moveit PR #1724: Jog Arm Dimensions
7363efc 
See more here: moveit/moveit#1724
This was referenced Nov 25, 2019
Merged
Merged
AdamPettinger
AdamPettinger commented Nov 25, 2019
View reviewed changes
Copy link
Contributor Author

@AdamPettinger AdamPettinger left a comment

Choose a reason for hiding this comment

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

I have moved the service file to moveit_msgs, updated the tutorial, and made a PR for both.

@rhaschke
Copy link
Contributor

@AdamPettinger, when merging with master, you removed some changes to the jog_arm package.
I suggest to reset your branch to the state before the merge and rebase instead onto master.
Please, carefully consider changes in the master branch this time.

@AdamPettinger
Copy link
Contributor Author

Oops, sorry about that @rhaschke! Was confused...

I rebased onto master, kept all the master changes, and moved some of the changes made for this PR into the JogInterfaceBase class to match the master branch code structure

davetcoleman
davetcoleman reviewed Jan 21, 2020
View reviewed changes
Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

I wonder if we can make this more realtime performant.

moveit_experimental/moveit_jog_arm/src/jog_calcs.cpp Outdated Show resolved Hide resolved
moveit_experimental/moveit_jog_arm/src/jog_interface_base.cpp Outdated Show resolved Hide resolved
@v4hn
Copy link
Contributor

v4hn commented Apr 23, 2020

@davetcoleman @tylerjw @AndyZe @AdamPettinger
This request was reviewed quite a lot by now, CI only failed with clang-format issues (that need to be cleaned up) and by now it actually conflicts with upstream.

@AdamPettinger could you please rebase this to master and fix the code formatting?

Could we please get this merged to avoid yet another dead request lying around?

@AdamPettinger
Copy link
Contributor Author

@v4hn Sorry about that, this fell off my radar recently.

I rebased and ran the clang formatting suggestion from the CI error

tylerjw
tylerjw approved these changes May 7, 2020
View reviewed changes
Copy link
Member

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

This looks good to go. @AdamPettinger @AndyZe correct?

If so, can we merge it?

@AdamPettinger
Copy link
Contributor Author

It is ready to be merged as far as I am concerned!

@tylerjw tylerjw mentioned this pull request May 7, 2020
Closed
2 tasks
@AndyZe
Copy link
Member

AndyZe commented May 7, 2020

I rebased on master and tested locally. Still seems fine to me

@henningkayser
Copy link
Member

@rhaschke good to go?

@davetcoleman
Copy link
Member

Let's give @rhaschke a day to contest, and if not let's merge.

@rhaschke rhaschke merged commit aba6959 into moveit:master May 8, 2020
@welcome
Copy link

welcome bot commented May 8, 2020

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

@tylerjw
Copy link
Member

tylerjw commented May 8, 2020

🎉

tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
@AdamPettinger @tylerjw
Jog arm dimensions (moveit#1724)
5a7432a 
Added a service to change which directions jogging is allowed in
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
@AdamPettinger @tylerjw
Jog arm dimensions (moveit#1724)
783ee03 
Added a service to change which directions jogging is allowed in
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
@AdamPettinger @tylerjw
Jog arm dimensions (moveit#1724)
3f9720b 
Added a service to change which directions jogging is allowed in
davetcoleman pushed a commit to moveit/moveit_tutorials that referenced this pull request Jun 2, 2020
@AdamPettinger
Update for moveit PR #1724: Jog Arm Dimensions (#433)
40d7ed4 
* Update for moveit PR #1724: Jog Arm Dimensions

See more here: moveit/moveit#1724

* Fix dumb copy-paste formatting error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gavanderhoorn gavanderhoorn gavanderhoorn left review comments

@AndyZe AndyZe AndyZe approved these changes

@davetcoleman davetcoleman davetcoleman approved these changes

@tylerjw tylerjw tylerjw approved these changes

@henningkayser henningkayser henningkayser approved these changes

@rhaschke rhaschke rhaschke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@AdamPettinger @AndyZe @gavanderhoorn @rhaschke @v4hn @henningkayser @davetcoleman @tylerjw