OSVR / OSVR-Core

The core libraries, applications, and plugins of the OSVR software platform.
Apache License 2.0
329 stars 124 forks source link

Auto-start server API/functionality #377

Open JeroMiya opened 8 years ago

JeroMiya commented 8 years ago

This issue is to discuss/implement OSVR server auto-start functionality/API. For this issue, we could focus initially on the primary use case: Android, where it is critically needed, and finally on more open platforms, where it is just nice to have.

New API vs auto-start behind the scenes

There has been some discussion as to whether there needs to be a new API to initiate server auto-start explicitly by the client application, or whether a client context should just initiate the auto-start behind the scenes, opaque to the client. Here are some of the arguments, but please discuss in the comments:

Pro new API:

Con new API:

I am leaning towards the new API approach. It gives the clients flexibility without overly burdening client applications with extra code.

godbyk commented 8 years ago

Just leaving a few links here for future reference.

Automatic service activation on Linux

You can trigger an OSVR server startup using D-Bus activation or socket activation using systemd.

Once configured, any time a client attempts to connect to the OSVR TCP socket, systemd will start up the OSVR server (if it's not already running). This requires no special handling within the OSVR API, it just needs to be configured with systemd.

russell-taylor commented 8 years ago

It seems like we're going to have to provide the name of the server to connect to whether we have a separate call or not, so that all client contexts will talk to the same server and so that server can be remote if needed (or on a non-standard port). Given than, using a URL for the server name as is done in VRPN provides us flexibility for auto-starting or not. Using an approach like VRPN's where you get handed back the same server if you ask for it twice with the same name will provide the ability to internally avoid trying to spin up multiple servers.

On the other hand, adding an OSVR_Init() with an optional server name is not a large burden on the community and could be added to our example programs. It would mean changing code, so we should go ahead and make it part of the upcoming breaking change if we do.

JeroMiya commented 8 years ago

We will need both an osvrInit() and osvrShutdown(). On Android osvrShutdown() actually needs shut down the server thread. Calling osvrInit should be optional, and the behavior without it should match the current behavior - assume the server is already running, and fail if it isn't. Thus existing applications shouldn't break so long as the server is running already.

It looks like osvrClientInit already looks at an environment variable for the host: image

Note that this should work on Android as well. Android does allow you to set and read environment variables, although they are only visible to the application and its child processes.

rpavlik commented 8 years ago

The existing environment variable to connect to a non-standard server is an undocumented hack that few know of and that I'd like to be rid of.

To give you a bit of back-story, the initial design intention was that configuration was outside the scope of concern of the application (and that making configuration something that applications had to deal with would be an impediment to VR adoption - nobody wants to pass a bunch of config files as parameters to their application, esp. if they're always the same for that system), and that included configuration of "where the server was", hence why opening a client context took no location parameter: it was assumed there'd be something locally, if nothing else to say "the real server is over there".

The formative pain here was me dealing with VR Juggler, which was fairly and over time increasingly tightly coupled to the concept that it had first dibs on your command line arguments: it expected one or more config files, and/or flags indicating that process' role in the cluster, and heaven forbid you want to pass your own arguments to your program: it would often try to parse whatever you passed as a config file and promptly die when it wasn't one. So running something by just double clicking it would never work by default (VR JuggLua worked around it by faking a command line and passing that to the underlying VR Juggler code), nor would opening some non-config-file document.

russell-taylor commented 8 years ago

Vlib at UNC used configuration files; first searching in the current directory, then the user's home directory, then in a system-wide directory, then in an environment variable. I could never remember the order and your app would behave differently depending on where you were when you ran it, and it got to be a confusing mess.

VRPN finessed the whole thing by encoding "where is the server and how do I run it" into a URI format within the name of the device itself. Then, however you described your device to your application, VRPN could do the right thing. Some used command-line arguments, some used environment variables, some hard-coded them, but it was not VRPN's job to figure that out.

We could leave this to the client app to determine in OSVR using the same mechanism. You can only standardize what nobody cares about. People seem to care about how they pass configuration info into their apps. There should of course be a default that works. That default might be a separate thread using loopback.

russell-taylor commented 8 years ago

(network loopback, not the new networkless loopback that can't run in a separate thread)

JeroMiya commented 8 years ago

The auto-start functionality is not intended to allow clients control over where the server is, etc... It's meant to be a platform specific black box.

Here is my current prototype (coding blind for the moment):

static const char SERVER_ENV_VAR[] = "OSVR_SERVER";

OSVR_ReturnCode osvrServerInit()
{
    // @todo start the server.
#if defined(OSVR_ANDROID)
    // @todo implement auto-start for android
#else
    auto server = osvr::common::getEnvironmentVariable(SERVER_ENV_VAR);
    if (server.is_initialized()) {
        OSVR_DEV_VERBOSE("Attempting to auto-start OSVR server from path " << *server);

#if defined(OSVR_WINDOWS)
        LPCTSTR appName = server->c_str();
        LPSTARTUPINFO startupInfo;
        LPPROCESS_INFORMATION processInfo;
        memset(&startupInfo, 0, sizeof(startupInfo));
        memset(&processInfo, 0, sizeof(processInfo));
        if (!CreateProcess(appName, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
            startupInfo, processInfo)) {
            OSVR_DEV_VERBOSE("Could not auto-start OSVR server.");
            return OSVR_RETURN_FAILURE;
        }
        return OSVR_RETURN_SUCCESS;
#else
        pid_t pid = fork();
        if (pid == -1) {
            OSVR_DEV_VERBOSE("Could not fork the OSVR server process.");
            return OSVR_RETURN_FAILURE;
        } else if (pid == 0) {
            execl(server->c_str(), NULL);
            OSVR_DEV_VERBOSE("Could not start OSVR server process.");
            return OSVR_RETURN_FAILURE;
        } else {
            OSVR_DEV_VERBOSE("Started OSVR server process successfully.");
            return OSVR_RETURN_SUCCESS;
        }
#endif
    } else {
        OSVR_DEV_VERBOSE("Environment variable " << SERVER_ENV_VAR << " not defined. Assuming server is already running.");
    }
#endif
}

OSVR_ReturnCode osvrServerRelease()
{
#if defined(OSVR_ANDROID)
    // @todo cleanup server thread
#else
    // no-op. Leave the server running.
#endif
    return OSVR_RETURN_SUCCESS;
}
JeroMiya commented 8 years ago

pull request for this issue is open (https://github.com/OSVR/OSVR-Core/issues/380) and could use a review. :)