search

moveit / moveit_ikfast

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
12 stars 20 forks source link

Fix issue #18: remove install rule for ikfast.h from generated package CMakeLists.txt #19

Closed gavanderhoorn closed 10 years ago

gavanderhoorn commented 10 years ago

AFAICT the ikfast.h header is only needed for compilation of the MoveIt IKFast plugin, not by 'end-users' of the resulting binary (library or package).

gavanderhoorn commented 10 years ago

ping?

davetcoleman commented 10 years ago

Inside the generated ROBOT_ARM_ikfast_solver.cpp there is the line:

#include "ikfast.h" // found inside share/openrave-X.Y/python/ikfast.h

I'm not sure if the comment is actually correct though, or if it is referencing the one in /include/ikfast.h that you are proposing to remove. Have you tested this?

gavanderhoorn commented 10 years ago

@davetcoleman: my question in #18 was whether the header file needs to be installed, so included in a binary distribution of an IKFast plugin. I was under the impression that the header was only needed at compile time, thus no install rule necessary.

All my uses of IKFast plugins have been without that header installed, and they worked fine. I'm not sure if it would be needed to use an IKFast plugin stand-alone though.

davetcoleman commented 10 years ago

The package moveit_ikfast is never needed for running a robot, it is only a tool for developers to create their own custom ikfast packages. If you install moveit_ikfast from debian the only usefulness of it is if you have the header file needed to generate the solution cpp file. Therefore, I'm pretty sure the ikfast.h should be included with the debian.

What I'm not sure about is where it should be installed to. Is the ROS include directory the correct place? I'm not an expert on this.

gavanderhoorn commented 10 years ago

Therefore, I'm pretty sure the ikfast.h should be included with the debian.

Sure, but the pull request changes the CMakeLists.txt template, which is AFAICT used to generate the CMakeLists.txt of the resulting MoveIt IKFast plugin package. So this is not about moveit_ikfast's CMakeLists.txt, but that of the generated package.

davetcoleman commented 10 years ago

Ah I see. I've never created debians out of the generated custom ikfast packages for a robot, so I haven't run into this. Yes, I think you are right that it is only needed at compile time, good catch.