-
Notifications
You must be signed in to change notification settings - Fork 19
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
Linux/MacOSX port of DriverLoader #10
Conversation
@@ -100,7 +100,7 @@ add_library(ViveLoaderLib STATIC | |||
VRSettings.h) | |||
target_link_libraries(ViveLoaderLib | |||
PUBLIC | |||
OpenVRDriver osvr::osvrUtil linkable_into_dll | |||
OpenVRDriver osvr::osvrUtil linkable_into_dll ${CMAKE_DL_LIBS} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer for this to be conditional on if(NOT WIN32)
, if you don't mind. It might also be able to be a PRIVATE
library dependency, rather than public, not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Far As I Know, CMake will return a null string on Win32.
And -ldl on OSX/Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$CMAKE_DL_LIBS
is set to an empty string on platforms that don't need to link to a library for dlopen()
/dlclose()
.
Wow, great, thanks for the contribution, it's really appreciated! I just have two little tweaks to suggest (the one in the build file, then the one to remove the unnecessary ifdefs around constructors), and then just a general question about libdl usage, since that's something I write libraries to keep me from dealing with too often... then I'll be more than happy to merge this! FWIW, you can actually "test" and run this without Vive hardware, as long as you have SteamVR installed, it'll have the driver in the expected location, so running the ViveLoader app (built with the project, just not installed) should take it through its basic paces. It won't get too far without the hardware but you'll be able to see if the loading of the library and such works properly. |
Yes, I have installed SteamVR. Ok, I will try to run it and fix all issues tomorrow. |
CouldNotLoadDriverModule(const char *errString) | ||
: std::runtime_error( | ||
"Could not load driver module." + | ||
std::string(errString)) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably add a space between the hard-coded C-string and the appended error string.
Finally, got it running. Also I fixed some mistypes in code, forgotten includes. NOTE: it requires setting LD_LIBRARY_PATH variable(SteamVR dependencies). It may be useful for they, who have Vive and want use OSVR.
NOTE2: For some reason, Valve have an invalid GCC detection macro. Here is a confirmation: ValveSoftware/openvr#58
|
…pening, if there is unresolved symbols. Use RTLD_GLOBAL, so other libraries can access the driver, if it need
Ah... I forgot a charger for my laptop in home. If there is some issues, I will fix in next Monday. |
Is anyone alive here? |
Sorry, I thought we were waiting for you to test this again. |
@@ -40,14 +41,26 @@ | |||
namespace osvr { | |||
namespace vive { | |||
struct CouldNotLoadDriverModule : std::runtime_error { | |||
CouldNotLoadDriverModule() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually meant leave both constructors - a default constructor and one that takes an extra error message, with no platform ifdefs in this header at all...
FWIW, |
Okay, I will look for it.
|
Sorry, I was been busy for a week. There is a problem with I see some workarounds:
|
That's fine, I've been busy too. Think I may as well merge this as-is, but would be good to figure out how to improve the libdl situation. I wonder what SteamVR does, since they have the same problem: they have to add each driver's directory to the library search path. In libfunctionality, which is basically the only other place I've done substantial dlopening of libraries myself cross-platform (Unity/.NET doesn't really count), I'd contemplated/thought we might need/want a In any case, while option 2 in the general case is difficult, for the specific case of just loading the Vive driver, we could manually look up and hard-code its dependencies and preload them - that's essentially what we had to do with Unity loading OSVR... |
Yeah, I'll just go ahead and merge this as-is, after skimming some related things, if that sounds good with you - let me know if you're ready for me to merge it! If you wouldn't mind, could you submit a pull request updating this page with the LD_LIBRARY_PATH workaround info under a Linux section? https://github.com/OSVR/OSVR-Docs/blob/master/Configuring/HTC-Vive.md |
I think it does the same thing as Steam itself. Just setting LD_LIBRARY_PATH before starting program. It's most easy and secure way. I may write an example shell script which will detect steam-runtime, add it to LD_LIBRARY_PATH and start the app. But I little busy now. I will update both |
reset(); | ||
throw CouldNotLoadEntryPoint(dlerror()); | ||
} | ||
factory_ = reinterpret_cast<DriverFactory>(proc); \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a backslash at the end of this line (line 108)? Seems to break the build for me
Merging this - thanks for your contribution! |
Just looked at the commits again - you got a lot more in than Linux support, thanks for catching my bugs! thank you so much! |
Not tested this at all. But this must work on Linux and OSX too, since they are provide same POSIX dlfcn.h.
Also here is little changes in exceptions, because it's too important to know why library isn't loaded.
This must be compiled with
-ldl
. (${CMAKE_DL_LIBS}
variable in CMake)