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

ENSENSO_INSTALL Required #58

Open
yotabits opened this issue Apr 22, 2021 · 4 comments
Open

ENSENSO_INSTALL Required #58

yotabits opened this issue Apr 22, 2021 · 4 comments

Comments

@yotabits
Copy link

https://github.com/ensenso/ros_driver/blob/42014bd0fa58ef4840a1e319c533a23958b0f9d1/ensenso_camera/CMakeLists.txt#L56-57

It seems that the environment variable ENSENSO_INSTALL is required to find FindEnsenso.cmake file
installed by the ensenso-sdk.
The ENSENSO_INSTALL variable is set by /etc/environment
This is not convenient for systems not relying on /etc/environment. Would it be possible to put the FindEnsenso.cmake file in a place where ENSENSO_INSTALL wouldn't be necessary?

@saierd
Copy link
Member

saierd commented Apr 22, 2021

What system do you have in mind and how did you install the SDK there? What prevents you from setting up the environment variable to the place where you installed the SDK?

Generally, Debian and Ubuntu are the only officially supported platforms for the Ensenso SDK. On those systems the deb package should set everything up so that it works. On other systems, you can copy the files wherever you want, but the ENSENSO_INSTALL environment variable is always necessary for a working installation of the NxLib. It is not only used for finding the CMake file, but also e.g. for storing caches and other configuration files at runtime. This is also documented in the tar archive used to install the SDK on those systems.

@yotabits
Copy link
Author

The SDK is installed using the Debian package. We are building in CI ensenso_camera and various packages in a ubuntu docker which by default does not use /etc/environement so it forces us to export the ENSENSO_INSTALL we would like to avoid having to do that as it seems using a cmake-config file in a default location would remove the necessity for CMake to know the package install path.

@saierd
Copy link
Member

saierd commented Apr 22, 2021

I just searched a bit on how this should be done and it doesn't seem like there is a standard path for CMake module files where I could put the existing script.

We already have an internal ticket for improving and modernizing the CMake integration of the NxLib. I will see that we consider this use case when doing that. We could add a workaround to the CMake script of the ROS driver, but I guess the easiest solution for now is to just export the variable.

@haudren
Copy link

haudren commented Apr 23, 2021

Thank you for answering. While trying to figure this issue out with @yotabits I stumbled upon the official CMake documentation:
https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#config-file-packages

Indeed, there is no standard location for the FindPackage.cmake, as it seems like (paraphrasing a bit):

  • Packages are supposed to provide their own package-config.cmake (can be mostly auto-generated)
  • For packages that depend on non-CMake packages, you should write the FindPackage.cmake corresponding to that dependency

So it seems that the recommended course of action is to either:

  • Modernize CMake in NxLib to provide ensenso-config.cmake in a standard system directory
  • Import FindEnsenso.cmake into this repository (ros_driver), and not rely on the environment variable

We will export the environment variable manually for the time being, but I would in general try to avoid relying on environment variables as there are a lot of headless use cases out there that will not load any default environment variables. For example robot_upstart uses setuidgid to have roslaunch as a daemon process.

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

No branches or pull requests

3 participants