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

ROBOTOLOGY_USES_CYBERITH_SDK variable does nothing #519

Closed
traversaro opened this issue Nov 18, 2020 · 4 comments · Fixed by #1436
Closed

ROBOTOLOGY_USES_CYBERITH_SDK variable does nothing #519

traversaro opened this issue Nov 18, 2020 · 4 comments · Fixed by #1436

Comments

@traversaro
Copy link
Member

Similar to what was spotted in #511, also the ROBOTOLOGY_USES_CYBERITH_SDK option is basically ignored, and it is not used by anything, see: https://github.com/robotology/robotology-superbuild/search?q=ROBOTOLOGY_USES_CYBERITH_SDK .

@kouroshD @GiulioRomualdi do you have any idea how this is possible?

@GiulioRomualdi
Copy link
Member

Actually, I do not have any idea. I hope @kouroshD can help you

@traversaro
Copy link
Member Author

Given #511 (comment), I guess that the option was problably never used, and what is happening is that the virtualized module (the one that requires Cyberith) is always enabled on Windows, see : https://github.com/robotology/walking-teleoperation/blob/master/modules/CMakeLists.txt#L13 .
This means that ROBOTOLOGY_ENABLE_TELEOPERATION set to ON on a Windows system without Cyberith (so with ROBOTOLOGY_USES_CYBERITH_SDK set to OFF) is basically broken, but we did not realized this because of #472 .

@kouroshD
Copy link
Contributor

kouroshD commented Nov 19, 2020

Sorry for my late reply!
I have checked the PR here https://github.com/robotology/robotology-superbuild/pull/168/files#diff-9b8f433cdd5b2117c6b8376b22470f2e4d3c332ad97323b76d82f6049e03685b

We have the two options in ROBOTOLOGY_USES_CYBERITH_SDK and ROBOTOLOGY_USES_OCULUS_SDK in the main CMakeLists of the superbuild, however these two option are not used as they are commented here. https://github.com/robotology/robotology-superbuild/pull/168/files#diff-9b8f433cdd5b2117c6b8376b22470f2e4d3c332ad97323b76d82f6049e03685b

I did a flashback to the old issue, but I could not find the reason we made this choice. However, as far as I remember, I had a discussion with @traversaro about this, and we decided to have these options even if not used, and later use these two options to make installation easier (I think we added them not by mistake but with this intention). For example look at this issue: #143 and https://github.com/dic-iit/lab-events-demos/issues/64#issuecomment-447928490 . Adding those two options were the first step toward making installation automatic!

However, things changed by time, and we kept the installation procedure as it was.
To recap, the current state is:

The actions it comes to me to take, in order to fix these issues are:

  • Update the documentation for the installation of the dependencies
  • Instead of deleting ROBOTOLOGY_USES_CYBERITH_SDK, we can use it for windows by passing to walking-teleoperation, in order to install Cyberith module. So, in this case, we can avoid error on CI for windows machine. The relevant PR opened by @traversaro Manage CybSDK as other dependencies walking-teleoperation#44
  • Instead of deleting ROBOTOLOGY_USES_OCULUS_SDK, we pass this option to vcpkg in order to install the glew and glfw3 automatically (I am not sure about the last suggestion since after updates on vcpkg I have not install on Windows machine, and I am not aware of the vcpkg procedure for installing dependencies, if not the case we can keep the option deleted). The option on yarp also can be enabled by this option. For example, using audio since we can use oculus for that as well. A relevant PR by @traversaro If ROBOTOLOGY_USES_OCULUS_SDK is enabled compile yarp-device-ovrheadset #518

I think this will address the relevant issues in #511 (however closed!), this issue, and robotology/walking-teleoperation#43.

Let me know what do you think @traversaro @GiulioRomualdi @S-Dafarra ?

@traversaro
Copy link
Member Author

Thanks for the clarification @kouroshD, I think that is clear and the issue you point to should solve the issue!

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