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

moveit_ros_perception: make OpenGL parts optional #698

Merged
merged 1 commit into from
Mar 29, 2018

Conversation

bulwahn
Copy link
Contributor

@bulwahn bulwahn commented Nov 29, 2017

Description

We make OpenGL parts optional, but build everything by default.

This commit is motivated by the work on the OpenEmbedded layer for ROS, a cross-compilation tool chain environment for ROS packages. We would like to allow to use moveit_ros_perception in embedded systems without OpenGL support.

Dmitry Rozhkov is the original author of this patch. I ported the patch from indigo to the current kinetic branch and elaborated the commit message.

Checklist

  • Code Formatting: clang-format does not apply to CMakeLists.txt. clang-format clearly states that this is limited to .h, .hpp and .cpp files. There is no enforced syntax convention for CMakeLists.txt.
  • Documentation: I checked the existing moveit documentation and they are focussed on its use. So, the best documentation to consider documenting the build configuration would be http://moveit.ros.org/install/source/. However, I think the user base of the feature is small, and developers are likely going to look into the CMakeLists.txt file when they encounter the issue and then they will find the configuration option quite immediately.
  • Screenshot: Not applicable.
  • Testing: It is special requirement from only a few users so far, so a test is not strictly needed. I will check the complexity of adding to compile it twice with and without the BUILD_OPENGL feature.
  • Backporting: It is special requirement from only a few users so far, so this does not cherry-picked to other current ROS branches (Indigo, Jade, Kinetic).

set(gl_LIBS ${gl_LIBS} ${OPENGL_LIBRARIES})
endif(OPENGL_FOUND)
set(perception_GL_INCLUDE_DIRS "mesh_filter/include" "depth_image_octomap_updater/include")
set(SYSTEM_GL_INCLUDE_DIRS ${GLEW_INCLUDE_DIR} ${GLUT_INCLUDE_DIR})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line seems unused, could you please explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used in line 91. If BUILD_OPENGL is not set, SYSTEM_GL_INCLUDE_DIRS is empty; if BUILD_OPENGL is set, the paths are set as SYSTEM include directories.

@bulwahn
Copy link
Contributor Author

bulwahn commented Dec 2, 2017

I just want to inform @rojkov that I am providing his patch upstream.

@bulwahn
Copy link
Contributor Author

bulwahn commented Dec 2, 2017

In this discussion, I came up with the idea to actually rename the option BUILD_OPENGL to the more standard option WITH_OPENGL. @rojkov Any objections to that?

But build everything by default.

This commit is motivated by the work on the OpenEmbedded layer
for ROS, a cross-compilation tool chain environment for ROS
packages. We would like to allow to use moveit_ros_perception
in embedded systems without OpenGL support.

Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>

[Ported Dmitry's patch from indigo to current kinetic branch,
renamed build config, elaborated commit message]

Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
@rojkov
Copy link
Contributor

rojkov commented Dec 2, 2017

Sure, WITH_OPENGL is definitely better. I should have used that name from the beginning.

@davetcoleman davetcoleman merged commit 8aa8e1f into moveit:kinetic-devel Mar 29, 2018
@davetcoleman
Copy link
Member

Merging because minor change.

rhaschke pushed a commit that referenced this pull request Jul 12, 2018
rhaschke pushed a commit that referenced this pull request Jul 17, 2018
dg-shadow pushed a commit to shadow-robot/moveit that referenced this pull request Jul 30, 2018
mayman99 pushed a commit to mayman99/moveit that referenced this pull request Aug 25, 2018
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.

3 participants