ros-industrial / abb_libegm

A C++ library for interfacing with ABB robot controllers supporting Externally Guided Motion (689-1)
BSD 3-Clause "New" or "Revised" License
93 stars 53 forks source link

Protobuf headers not found when compiling with CMI #65

Closed jbeck28 closed 4 years ago

jbeck28 commented 4 years ago

I'm trying to run catkin_make_isolated, and a package which depends on abb_libegm leads to:

/home/joshua/catkin_ws/src/abb_libegm/include/abb_libegm/egm_base_interface.h:42:10: fatal error: egm.pb.h: No such file or directory
 #include "egm.pb.h"         // Generated by Google Protocol Buffer compiler protoc

Not sure how to fix!

Edit, one such example would be the egm samples package.

gavanderhoorn commented 4 years ago

This is likely a build ordering issue due to the fact that those files are generated (here) and CMake doesn't know that, so it cannot pass that information on to whatever build scripts it generates (be it Makefiles or something else).

This in turn causes make (if you're using that, which CMI by default does) to wrongly assume that it can parallelise certain targets which should really be built after one another.

Compare this to ROS 1 message/service/action headers and how you have to use add_depencies(..) to force correct build order.

It will probably work if you first build just abb_libegm and then the rest of the workspace. That is not a solution, but would confirm what I wrote above.

A quick look at the protobuf CMake integration doesn't show any targets we could depend on directly. Perhaps something like this could be used:

add_custom_target(abb_libegm_proto_generated DEPENDS ${EgmProtoSources} ${EgmProtoHeaders})

and then:

list(APPEND ${PROJECT_NAME}_EXPORTED_TARGETS abb_libegm_proto_generated)

This would be ROS 1 / Catkin specific, but I'm not sure at this point whether we'd need anything for this in #63.

jbeck28 commented 4 years ago

By chance, abb_libegm is built before abb_libegm_samples. Forgive my ignorance, how would I go about building only abb_libegm, and then the rest of the workspace?

Also, to be clear, both packages build fine using catkin_make

gavanderhoorn commented 4 years ago

I haven't checked or diagnosed, so my response above was just some thinking-out-loud after reading your OP.

This could also be another instance of #7 (PR: #23 and #43) or a problem with how we export the include directories in this package.

Are you using an install space with CMI?

gavanderhoorn commented 4 years ago

It's likely a problem with how we export the include dirs.

In a build with catkin_tools (which works in ways similar to CMI), the protobuf headers are not located in a directory that is on the include path.

Could you try using an install space with CMI? I believe things should work then. That is not a solution, just a test.


Finally: can you tell me why you want to use CMI?

jbeck28 commented 4 years ago

I wanted to use CMI because the latest version of opw_kinematics seems to require it. I also found a bunch of mistakes in my cmake files which were uncovered by CMI... which seemed like a benefit to have found and fixed.

I'll do some reading on how to use an install space, I believe this:

https://catkin-tools.readthedocs.io/en/latest/verbs/catkin_config.html

provides the relevant information, right?

gavanderhoorn commented 4 years ago

I wanted to use CMI because the latest version of opw_kinematics seems to require it.

No, it doesn't. It's just that catkin_make cannot build workspaces with non-Catkin packages in it, and opw_kinematics is a plain CMake one since a few weeks.

So using CMI is one option, but it's not a direct requirement.

catkin_tools can build mixed workspaces as well.

I'll do some reading on how to use an install space, I believe this:

https://catkin-tools.readthedocs.io/en/latest/verbs/catkin_config.html

provides the relevant information, right?

That would be the documentation for catkin_tools, not catkin_make_isolated.

You should just be able to run catkin_make_isolated --install.

jbeck28 commented 4 years ago

That makes sense about opw-kinematics, thank you for all the help!

However, I encounter the same problem when running catkin_make_isolated --install.

gavanderhoorn commented 4 years ago

Ok, so the Catkin approach here would be to provide an CFG_EXTRAs file which exports the correct include path, depending on whether a devel or install space is used.

But seeing as we have #63 in the pipeline (which will do things differently anyway) I'm hesitant to start adding that.

I've just verified locally that using an install space results in the Protobuf generated headers getting installed in the correct location (ie: catkin_ws/install/include/abb_libegm). That should definitely be on the include path.

However, I encounter the same problem when running catkin_make_isolated --install.

Please make sure there are no cached paths anywhere in your workspace by removing the build, devel and install spaces (and their _isolated variants).

gavanderhoorn commented 4 years ago

The issue could be these #include statements:

https://github.com/ros-industrial/abb_libegm/blob/142f5a526b5db2865fb88fca1775b4c84d8031bc/include/abb_libegm/egm_base_interface.h#L42-L43

they don't prefix the package name, and only $install_space/include is on the include path, not $install_space/include/abb_libegm.

As a quick work-around, you could prefix those #include lines with abb_libegm/ and things should start working, but only if you use an install space.

For a devel space, you shouldn't add that prefix.

This is exactly what the CFG_EXTRAs file would fix.


alternatively, you could create a Catkin underlay with just opw_kinematics in it (and all your other non-Catkin packages) and build that with either catkin_make_isolated or catkin_tools. Then create an overlay with all your other packages. In that overlay you should be able to use catkin_make again.

jbeck28 commented 4 years ago

Few things: 1) I'd use #include<abb_libegm/egm.pb.h> rather than #include "abb_libegm/egm.pb.h" ? 2) I tried both and still had the same issue, though I only changed it on one of the header files in abb_libegm, though many of them have #include "egm.pb.h".

I'm going to get fresh copies of abb_libegm, as there's a chance I broke something when messing with the cmakelists.txt.

Thanks again for the assistance. I may attempt those underlays/overlays, though the specific implementation goes over my head, it's something I could probably learn.

I'll report back once I have a new package.

gavanderhoorn commented 4 years ago
  1. I'd use #include<abb_libegm/egm.pb.h> rather than #include "abb_libegm/egm.pb.h" ?

it doesn't matter. There is a slight difference in lookup order, but in the end it will find the same files.

2. I tried both and still had the same issue, though I only changed it on one of the header files in abb_libegm, though many of them have #include "egm.pb.h".

you'd have to chance it in all files that #include those headers.

I'm going to get fresh copies of abb_libegm, as there's a chance I broke something when messing with the cmakelists.txt.

I don't expect that to make any difference if you didn't change all files including those generated headers.

I may attempt those underlays/overlays, though the specific implementation goes over my head, it's something I could probably learn.

It would probably be the easiest workaround for now.

You could check this tutorial on overlaying and workspace chaining.

You'd essentially create a first workspace that overlays /opt/ros/$ROS_VERSION (as you'd normally do). In this workspace you'd place opw_kinematics and build it with CMI or catkin_tools.

Then source that workspace and create a second one. This would overlay the first workspace. Place all your other packages in the second workspace (or keep them there, if you already have a workspace you could use as the second workspace).

Now build the second workspace with regular catkin_make.

After you've sourced the second workspace you should be able to use everything as you'd normally do.

But instead of having the chain:

/opt/ros/$ROS_VERSION
  /path/to/your/catkin_ws

you'd now have:

/opt/ros/$ROS_VERSION
  /path/to/your/first_catkin_ws      <<-- contains opw_kinematics
    /path/to/your/second_catkin_ws   <<-- contains the rest

You only create the first workspace to get opw_kinematics out of the way.

jbeck28 commented 4 years ago

Your workaround (workspace overlay) worked perfectly. Can't thank you enough for your assistance!

gavanderhoorn commented 4 years ago

I'm pretty sure this is no longer an issue with the merge of #63.

@jbeck28: if you could verify that would be most appreciated.

I'm closing this in the meantime.

Feel free to keep commenting on the issue of course.