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

[melodic] IKFast compilation errors #241

Closed
VictorLamoine opened this issue Jul 2, 2018 · 8 comments
Closed

[melodic] IKFast compilation errors #241

VictorLamoine opened this issue Jul 2, 2018 · 8 comments

Comments

@VictorLamoine
Copy link
Contributor

VictorLamoine commented Jul 2, 2018

This change has to be applied on all IKFast generated code to be able to compile on melodic:

-  boost::shared_ptr<urdf::Link> link = boost::const_pointer_cast<urdf::Link>(robot_model.getLink(getTipFrame()));
+  std::shared_ptr<urdf::Link> link = std::const_pointer_cast<urdf::Link>(robot_model.getLink(getTipFrame()));
   while(link->name != base_frame_ && joint_names_.size() <= num_joints_)
   {
     ROS_DEBUG_NAMED("ikfast","Link %s",link->name.c_str());
     link_names_.push_back(link->name);
-    boost::shared_ptr<urdf::Joint> joint = link->parent_joint;
+    std::shared_ptr<urdf::Joint> joint = link->parent_joint;
     if(joint)
     {

Same goes for fanuc_experimental

Simple fix:

find . -type f -exec sed -i 's/boost\:\:shared_ptr/std\:\:shared_ptr/g' {} \;
find . -type f -exec sed -i 's/boost\:\:const_pointer_cast/std\:\:const_pointer_cast/g' {} \;
@gavanderhoorn
Copy link
Member

hm, bah.

Thanks though.

@simonschmeisser
Copy link
Contributor

Any news on this? The real fix would be to update the ikfast plugins I guess? There is now a urdf::LinkConstSharedPtr as can be seen here https://github.com/isys-vision/fanuc_experimental/blob/779b7b6d3fb390f8e074b76184ad6d06d574f00f/fanuc_m20ib25_moveit_plugins/src/fanuc_m20ib25_manipulator_ikfast_moveit_plugin.cpp#L383 as well as urdf::JointSharedPtr

Updating (all) ikfast plugins should probably be done before moveit/moveit#1166 will lead to further incompatibility. It has been merged and thus KinematicsBase is no longer ABI compatible but the ikfast template has not yet been updated which would create non-backwards compatible ikfast-"instances" (generated code) because it uses the new robot_model_ member variable https://github.com/ros-planning/moveit/blob/87482b291e2ed48fdf301d0d85d3f329ea333043/moveit_core/kinematics_base/include/moveit/kinematics_base/kinematics_base.h#L588

@gavanderhoorn do you have scripts and procedures for mass-upgrading?

@gavanderhoorn
Copy link
Member

I doubt I'll be upgrading the ikfast plugins: they have issues.

Either we migrate all pkgs to use trac_ik, or I might look into OPW #245.

@simonschmeisser
Copy link
Contributor

I doubt I'll be upgrading the ikfast plugins: they have issues.

Agreed, some (the MoveIt! API dependency) could be solved by making the wrapper even thinner (just a plugin interface to the generated code loaded by a generic ikfast-wrapper plugin). But you would still need the whole mess of openrave for just running the single python script that generates the code. Or do you have other non-maintenance issues in mind?

Would you accept a PR that updates the stuff?

Either we migrate all pkgs to use trac_ik, or I might look into OPW #245.

while being an impressive improvement to kdl, trac_ik is still just a last resort for us. It is just too slow. We need to run 1000+ ik computations as quickly as possible. OPW looks quite nice indeed!

I wonder if automatic parsing/setup is realistic and something to wait for: JeroenDM/moveit_opw_kinematics_plugin#3

@gavanderhoorn
Copy link
Member

I doubt I'll be upgrading the ikfast plugins: they have issues.

Agreed, some (the MoveIt! API dependency) could be solved by making the wrapper even thinner (just a plugin interface to the generated code loaded by a generic ikfast-wrapper plugin). But you would still need the whole mess of openrave for just running the single python script that generates the code.

Getting OpenRAVE to work is a non-issue. I always use a Docker image (this one).

Or do you have other non-maintenance issues in mind?

Yes. OPW is much simpler, depends on less infrastructure, is perfectly suited for our types of robots, should generate more accurate results (but I haven't tested/verified yet) and doesn't need 3k+ lines per a solver.

Would you accept a PR that updates the stuff?

Probably, but it seems a bit like a waste of time.

Either we migrate all pkgs to use trac_ik, or I might look into OPW #245.

while being an impressive improvement to kdl, trac_ik is still just a last resort for us. It is just too slow. We need to run 1000+ ik computations as quickly as possible.

That is indeed still an issue with trac_ik, and one that will most likely not change.

OPW looks quite nice indeed!

I wonder if automatic parsing/setup is realistic and something to wait for: JeroenDM/moveit_opw_kinematics_plugin#3

I doubt it: iirc the parameters needed for OPW are not present in the urdf.

@gavanderhoorn
Copy link
Member

@Levi-Armstrong: FYI.

@simonschmeisser
Copy link
Contributor

#262 provides a backward compatible fix until we agree on how to store opw parameters

@gavanderhoorn
Copy link
Member

Even compilation of the IKFast plugins results in a ton of warnings (mostly (all?) about deprecated MoveIt IK base class methods), all packages now do build on Melodic with the merge of #262, so I'm going to close this.

Thanks for the PR that made that possible @simonschmeisser.

Thanks @VictorLamoine for the initial report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants