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

Iron kinetic devel #96

Closed
wants to merge 3 commits into from
Closed

Iron kinetic devel #96

wants to merge 3 commits into from

Conversation

ahundt
Copy link

@ahundt ahundt commented Mar 10, 2017 •
edited
Loading

Merge master with kinetic patch:
#58
https://github.com/iron-ox/ur_modern_driver/tree/iron-kinetic-devel

May want to simply put thes into a new kinetic-devel branch until the restructured update is ready, for that reason edits from maintainers are enabled.

It at least compiles successfully, not yet tested actually running on a ur5.

jonbinney and others added 3 commits July 6, 2016 13:07
@jonbinney
Fix for hardware_interface ABI changes in kinetic
883070d
@jonbinney
Relax current state check on trajectories
45b3c34 
A lot of our trajectories in pick and place were failing because they
didn't start close enough to the robot's state, even though they were
very close.
@ahundt
merge master to https://github.com/iron-ox/ur_modern_driver/tree/iron…
836f212 
…-kinetic-devel
@ahundt ahundt mentioned this pull request Mar 10, 2017
Closed
@jacknlliu
Copy link

jacknlliu commented Jul 3, 2017 •
edited
Loading

This PR did not work for my UR5 robot. Using moveit_rviz with ros_control and vel_based_pos_traj_controller failed.

@ahundt
Copy link
Author

ahundt commented Jul 4, 2017

ah that's unfortunate :-/

v4hn
v4hn reviewed Jul 17, 2017
View reviewed changes
src/ur_ros_wrapper.cpp
@@ -344,7 +344,7 @@ class RosWrapper {

reorder_traj_joints(goal.trajectory);

if (!start_positions_match(goal.trajectory, 0.01)) {
if (!start_positions_match(goal.trajectory, 0.04)) {
Copy link

Choose a reason for hiding this comment

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

Could you please change this to use a ros parameter with default 0.04 instead?
That way the user can adjust the limit in launch files if required.

For our arm, we had to increase this limit above .08 and depending on the state of the arm other groups might need to increase it further.

@v4hn
Copy link

v4hn commented Jul 17, 2017

We don't use the ros_control interface.
But otherwise this request allows us to compile and use this package with our UR5 arm.

Thank you @jonbinney , @ahundt .
Please see my inline comment for a small request.

@ThomasTimm The original request for kinetic is more than a year old by now. Pretty Please make upstream compatible with ROS kinetic. If there are further issues with the ros_control integration, the community will figure them out once the module is officially supposed to support kinetic.
If you require it, we can also add a bit of preprocessor magic to make this patch compatible with kinetic & indigo in the same master branch.

@ahundt
Copy link
Author

ahundt commented Jul 18, 2017

#120 was just submitted and claims to be a full refactor supporting kinetic!

@nano-meter nano-meter mentioned this pull request Sep 21, 2017
Closed
@jacobdeery jacobdeery mentioned this pull request Oct 3, 2017
Closed
@gavanderhoorn
Copy link
Member

Seeing as #120 got merged I believe we can close this.

If you feel that is not the case please re-open and clarify.

Thanks for the PR, that is always appreciated.

@ahundt
Copy link
Author

ahundt commented Sep 30, 2018

sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v4hn v4hn v4hn left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ahundt @jacknlliu @v4hn @gavanderhoorn @jonbinney