OSVR / OSVR-Leap-Motion

OSVR Leap Motion plugin
Apache License 2.0
18 stars 14 forks source link

PluginKit API usage error: DeviceInitOptions not valid after you have a device token #11

Closed rpavlik closed 9 years ago

rpavlik commented 9 years ago

I'm surprised you've apparently gotten this to work (right?) - but here it looks like you're using the OSVR_DeviceInitOptions after you have a DeviceToken - though now I realize that might be the C++ device token holder just not yet initialized.

https://github.com/OSVR/OSVR-Leap-Motion/blob/master/Imaging.cpp#L12

I'm trying to think of a better, safer way of handling that since ideally you'd just pass the constructor the imaging interface you get from imaging configure, but you might consider hiding that osvrDeviceImagingConfigure call in a static method of your Imaging class that just returns that imaging interface.

The device token parameters to the send functions will become optional soon (they're already not used, IIRC) - as soon as we either decide to break the ABI or add alternate functions that don't have that parameter.

zachkinstner commented 9 years ago

The OSVR_DeviceInitOptions object is initialized with osvrDeviceCreateInitOptions() in ControllerDevice (here), then passed to each of the "module" classes (Analog, Imaging, etc.) as a constructor parameter. This seems to match the order things are done in the com_osvr_example_AnalogSync.cpp example. Is it a problem to pass around the options object in this way?

The Imaging class was using of both the C and C++ interfaces. I was able to remove the osvrDeviceImagingConfigure() call completely (see the commit referenced above). The imaging worked as expected before and after this change -- I can see the images in both Imaging_cpp.exe and the test Unity client.

rpavlik commented 9 years ago

I think that looks good. One of those bugs that becomes less of a bug as I write the issue, forgot that the C++ API device token object can exist, for the convenience of device classes, before the "device token" internally and in the C API exists.