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

Ubuntu 16.04 compilation #26

Merged
merged 3 commits into from
Aug 20, 2016
Merged

Conversation

bmagyar
Copy link
Contributor

@bmagyar bmagyar commented Aug 18, 2016

Hi guys,

I'm running iCub stuff on Ubuntu 16.04 with MoveIt as soon as I get everything to compile.
In accordance with this I may be submitting a couple of PR-s that fix the code for this platform.

None of these changes should affect functionality on other platforms.

This PR does 3 things.

  • static_pointer_cast had a clash appearing in several namespaces. Since boost:: was used everywhere consistently, I added that namespace to occurences where it was unspecified.
/home/bence/icub-ws/icub-model-generator/dh/generator/urdf_utils.cpp:264:155: error: call of overloaded ‘static_pointer_cast(urdf::GeometrySharedPtr&)’ is ambiguous
                         if( verbose ) std::cout << "Copyng visual mesh with filename " << (static_pointer_cast<urdf::Mesh>(mesh_link_ptr->visual->geometry))->filename << std::endl;

To add to the story: the namespaces urdf, boost and std (with C++11) all have static_pointer_cast. This file happened to have a using namespace to all of the above which brings us to the next item.

  • It is a form of courtesy to avoid using using namespace in general, especially to avoid putting more than one. I removed all 4, and fixed the few lines that were affected.
  • There was a typo in the CMake variable name of KDL header location compared to what KDL defines here

@traversaro
Copy link
Member

Hi @bmagyar ,
thanks a lot for the PR!

It is a form of courtesy to avoid using using namespace in general, especially to avoid putting more than one. I removed all 4, and fixed the few lines that were affected.

Yep, sorry on this one. Actually all the code of the dh generation pipeline was meant to be a temporary setup to generate models while waiting for the clean approach using simmechanics2urdf, and that is the reason why there is some much cruft around.
Unfortunately for some iCub models (i.e. v1) we are stuck on using the dh generation pipeline.

In general the generation process (for both dh and simmechanics) is quite experimental, but I just realized that this is not extremely clear from the README, I can clarify this in the README tomorrow, linking some issues that are still open.

There are some travis failures not related to this PR (see #27), I plan to fix those problem and come back to this PR.

In the meanwhile, if you are interested in iCub <--> MoveIt! integration you could check out the work done by the people in VisLab in Lisboa : https://github.com/vislab-tecnico-lisboa/icub-moveit .

@bmagyar
Copy link
Contributor Author

bmagyar commented Aug 19, 2016

Thanks for the great advice on MoveIt, I'm checking it out!

Once you release something it will never be temporary I guess, haha. New or old generation aside, if the compilation of a package fails people will assume nothing will work. Frankly I didn't give a single thought of the 2 different pipelines and which one I need when I got the compilation errors, just went ahead and fixed it. Others may get stuck there...

Yeah, fixing those issues will make travis green again 👍

@traversaro traversaro closed this Aug 20, 2016
@traversaro traversaro reopened this Aug 20, 2016
@traversaro
Copy link
Member

Travis problems solved in b6f24b0 , README fixed to clarify (hopefully) the scope of the repo in 50e6742 .
I tested locally and everything seems ok, merging.
Thanks @bmagyar !
OT : Considering the other open issues (such as #28 ) I think you could have issues in use one of this models in MoveIt! , so feel free to ping us if we can help in you in getting a good model for the iCub you are interested in.

@traversaro traversaro merged commit 6d7d145 into robotology:master Aug 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants