search

moveit / moveit_ikfast

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

IKTYPE not defined #5

Closed cheffe112 closed 10 years ago

cheffe112 commented 11 years ago

The treatment of different IK types introduced in commit 50a7e29348746245af66d39d1639f77643e65b34 spawns error messages like "You need to #define a IKTYPE_!" because nowhere in the process described in http://moveit.ros.org/wiki/Kinematics/IKFast any IKTYPE like IKTYPE_TRANSFORM_6D is defined in a generated source file.

I tried to track down the place where this define should be set, but couldn't figure out whether the error originally stems from the OpenRave IKFast package or from moveit_ikfast.

fsuarez6 commented 11 years ago

Using the same nomenclature as in http://moveit.ros.org/wiki/Kinematics/IKFast go to <moveit_ik_plugin_pkg>/src and in the file <yourobot_name>_moveit_ikfast_kinematics_plugin.cpp try adding #define IKTYPE_TRANSFORM_6D after line 59. That solved the problem in my case.

If you modify the template you should re-run: rosrun moveit_ikfast create_ikfast_moveit_plugin.py <yourobot_name> <planning_group_name> <moveit_ik_plugin_pkg> <ikfast_output_path>

cheffe112 commented 11 years ago

Thank you. Sure, I can do that and it worked for me as well as a workaround when I discovered the problem. However, it doesn't solve the issue that the IK type is then set manually, right? I thought the IKTYPE should not be hardcoded because that way it perverts the feature introduced in 50a7e29348746245af66d39d1639f77643e65b34, if I got it right.

fsuarez6 commented 11 years ago

Yes, it's manually set. That #define IKTYPE_TRANSFORM_6D should be included by the create_ikfast_moveit_plugin.py script. I'm not familiar with IKfast so I don't know how to determine the IKTYPE from the output_ikfast_XX.cpp

mintar commented 11 years ago

@davetcoleman I see that you merged 041a914db4a6a063749c6b65946061fc6692166e into master. This is not a good idea. As @jir-tobi correctly points out, this makes the feature introduced in 50a7e29348746245af66d39d1639f77643e65b34 rather pointless. Now, people who run the generator to generate a 5D IK plugin will see the same faulty behavior as before, but no error or warning.

The best solution would be indeed to insert the appropriate define in the generator script. But until then, we should restore the old behavior (force the user to manually insert the proper define). Maybe we should explain this better on the wiki page.

davetcoleman commented 10 years ago

I'm not sure I realized I had merged it, I might have pulled it in with some other fixes I was making.

What you are saying is that the user needs to manually add the appropriate define definition during the IKFast plugin creation, and we should not have a default one because that will will break any other iktypes. We certainly will need to document this in the wiki and anyone is welcome to edit it and do so.

I think a better solution would be to use the GetIkType() function that is included in the generated cpp solver code. It returns an int but looks like it does so in hex. If we could map its returned values for the different types of ik solutions, we could skip this editing the code step entirely with a much cleaner solution. Thoughts?

mintar commented 10 years ago

@davetcoleman Good catch! GetIkType() seems like it's the way to go. I remember looking for a function like that before I implemented the ugly #define ... thing and wondering why there was none. Turns out I must have been blind. I'll look into that and send you a patch.