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

remove left-over tf artifacts #1170

Merged
merged 4 commits into from Oct 29, 2018

Conversation

Projects
None yet
3 participants
@rhaschke
Copy link
Collaborator

commented Oct 29, 2018

Alternative fix of Melodic release build (instead of #1169).
Remove all left-over tf1 stuff:

$ grep -r "<tf/" .
./moveit_ros/planning/planning_request_adapter_plugins/src/chomp_optimizer_adapter.cpp:#include <tf/transform_listener.h>

$ grep -r ">tf[^2]" .
./moveit_commander/package.xml:  <run_depend>tf</run_depend>
remove left-over tf artifacts
... as we migrated to tf2

@rhaschke rhaschke referenced this pull request Oct 29, 2018

Closed

Add tf dependency to package.xml #1169

0 of 7 tasks complete
@IanTheEngineer

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

This spurious tf/transform_listener.h include was introduced in #1012

@IanTheEngineer

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

I found a file I seem to have missed in #830 with regards to the moveit_setup_assistant template:

$ grep -r "\"tf[^ _2]"
moveit_setup_assistant/src/widgets/configuration_files_widget.cpp:      vjb << "  <node pkg=\"tf\" type=\"static_transform_publisher\" name=\"virtual_joint_broadcaster_" << i

should be

moveit_setup_assistant/src/widgets/configuration_files_widget.cpp:      vjb << "  <node pkg=\"tf2_ros\" type=\"static_transform_publisher\" name=\"virtual_joint_broadcaster_" << i

@rhaschke, would you like to fix that here as well, or a should I open a separate PR?

@rhaschke

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 29, 2018

@130s Travis passed. You could proceed. Thanks!

@rhaschke

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 29, 2018

@IanTheEngineer Just push here as well.

@@ -28,7 +28,7 @@
<run_depend>rospy</run_depend>
<run_depend>sensor_msgs</run_depend>
<run_depend>shape_msgs</run_depend>
<run_depend>tf</run_depend>

This comment has been minimized.

Copy link
@IanTheEngineer

IanTheEngineer Oct 29, 2018

Member

Ah, this removal will cause problems unfortunately. tf2 python bindings don't have the conversions we apparently depend on:

~/dev/moveit_dev/src/moveit/moveit_commander$ grep -r tf
src/moveit_commander/conversions.py:import tf
src/moveit_commander/conversions.py:        q = tf.transformations.quaternion_from_euler(pose_list[3], pose_list[4], pose_list[5])
src/moveit_commander/interpreter.py:import tf
src/moveit_commander/interpreter.py:                    q = tf.transformations.quaternion_from_euler(float(clist[4]), float(clist[5]), float(clist[6]))
src/moveit_commander/move_group.py:import tf
src/moveit_commander/move_group.py:                (r, p, y) = tf.transformations.euler_from_quaternion(pose[3:])
package.xml:  <run_depend>tf2</run_depend>

If you'd like to leave this change off for now, I can develop a longer-term fix for this python package.

This comment has been minimized.

Copy link
@rhaschke

rhaschke Oct 29, 2018

Author Collaborator

Thanks for spotting this! Are you going to revert this?

This comment has been minimized.

Copy link
@IanTheEngineer

IanTheEngineer Oct 29, 2018

Member

Sure. I'll push that now

@IanTheEngineer
Copy link
Member

left a comment

Looks good. We'll just need to figure out the python tf conversions for tf2 for the future.

@rhaschke rhaschke referenced this pull request Oct 29, 2018

Closed

2018 Fall Release Kinetic and Melodic #1083

11 of 11 tasks complete
@clalancette

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

@rhaschke @IanTheEngineer Thanks for pushing this forward!

@IanTheEngineer IanTheEngineer merged commit d52f8c7 into ros-planning:melodic-devel Oct 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@IanTheEngineer

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

@130s @clalancette We're all merged in here. Things are ready for a rebuild & release

@rhaschke rhaschke deleted the ubi-agni:remove-tf1 branch Oct 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.