Using your device object as your hardware detect functor

[rpavlik]

I suppose it's not strictly forbidden, but it's not how the API was designed: https://github.com/OSVR/OSVR-Leap-Motion/blob/master/com_osvr_LeapMotion.cpp#L6

You're creating a device object unconditionally, passing it as the hardware detect callback functor, then finishing initializing it when the function-call operator (hardware detect) is called, rather than providing actual "detection of hardware" functionality in a functor that then creates a device object if needed.

The current structure will also make it difficult for the server to clean up in case of device failure (in future API extensions), since the hardware detection functor registration method is taking ownership of the device object at registration.

[zachkinstner]

The com_osvr_LeapMotion.cpp code...

context.registerHardwareDetectCallback(new LeapOsvr::ControllerDevice());

...matches the com_osvr_example_DummyDetectAndCreateAsync.cpp example code:

context.registerHardwareDetectCallback(new HardwareDetection());

Is there a different example available that shows the correct way to do this?

[zachkinstner]

I'm seeing that some examples (like com_osvr_example_AnalogSync.cpp don't use registerHardwareDetectCallback() at all. Does the Leap Motion plugin need this step? The Leap Controller class can exist even if the device is connected (there's an IsConnected property to check for this).

[rpavlik]

Syntactically, yes, those lines are similar, but the HardwareDetection object actually performs detection then creates a device object.

The step is highly recommended, since it keeps devices from being created in the path tree (and having their automatic aliases populate, etc) unless there's actual hardware backing them.

[rpavlik]

(The reason some examples don't use it is because I tried to make the examples each show exactly one thing, as much as possible - too many "examples" for SDKs cram every feature in so you can't see the forest for the trees.)

[zachkinstner]

Please see the commit referenced above.

I have added a HardwareDetection layer, whose only job is to create/send a new ControllerDevice to OSVR via registerObjectForDeletion(). The ControllerDevice now does all the setup within its constructor, and its destructor cleans up the new "module" objects that it creates.

How does this solution look? Is it possible to test for the "clean up in case of device failure" scenario at this point?

[zachkinstner]

@rpavlik wrote:

you could load the library here and see if a device is actually connected

I have added this condition to HardwareDetection in my local build:

Leap::Controller controller;

if ( !controller.isConnected() ) {
    mFound = false;
    return OSVR_RETURN_FAILURE;
}

If I return OSVR_RETURN_FAILURE here, does OSVR delete the object that was given to registerObjectForDeletion()?

[zachkinstner]

So far, the above approach seems to work. I'm not sure whether setting mFound = false is correct, since that assumes that the previously-created ControllerDevice object has been destroyed and will need to be replaced once the device is connected again.

In my tests with the test Unity client, while keeing the OSVR Server running, I'm connecting/disconnecting the device and then stopping/starting the Unity scene. Whenever the scene is started, OSVR Server reports "Performing hardware auto-detection" and then runs the plugin's HardwareDetection callback.

This is working as expected, except that the Unity scene sometimes shows the most recent images (the last "camera" and "distortion" frames before device disconnection) while the device is disconnected. I can also reproduce this via Imaging_cpp.exe. This behavior may mean that the plugin's ControllerDevice object is still alive and sending stale image data, or that the OSVR Server is caching/sending stale images.

[rpavlik]

Returning failure from a hardware detect callback does not presently change anything. Returning failure from a device update callback signals that the device is disconnected/no longer active, but there's currently not lifecycle code to dispose of devices returning failure there. (as you noted, a plain registerObjectForDeletion() doesn't provide the core with enough high-level information to do that). As far as that goes, this is fine for now.

mFound is a pattern I accidentally introduced through my desire to show a hardware detection callback working with a dummy example plugin, without having the device instantiated anew each time the hardware detection was called. Unfortunately it has now spread into a number of device plugins (as I should have expected), despite usually having a better way of knowing if you've already handled a given device when you're enumerating them. If you have a better way of tracking if you've already created a device, please just use that exclusively.