chroma-sdk/Colore

NativeApi.InitializeAsync() not really async

poveden opened this issue · 5 comments

Currently NativeApi.InitializeAsync() wraps NativeSdkMethods.Init() to return a Task. However, NativeSdkMethods.Init() does take a while to start (about 300ms on my laptop), blocking the calling thread.

I'd suggest to spin a new thread doing something like this:

return Task.Run(_nativeSdkMethods.Init)
   .ContinueWith(/* error handling */);

Thoughts?

This is perhaps something we should do for all the native calls?

Coming back to this after a too long hiatus.

It seems the recommended way to wrap sync calls in async contexts is to do:

// Custom delegates not supported so have to wrap in a lambda
var result = await Task.Run(() => _nativeSdkMethods.Init());
// handle result

Updating all the methods in NativeApi to use this style might be optimal then?

After a long hiatus myself, I've found myself acquiring a better understanding on how async/await works.

Given that _nativeSdkMethods.Init() will block the thread whether we want it or not, spinning another thread makes no difference as the thread that will be blocked will be the new one, so frankly I would leave it as it is.

Besides this, there's the fact that the Chroma SDK will take way longer (around 1 second on my machine) to get initialized internally and be ready to accept commands. The only way to check on this, though, is by listening for the WM_CHROMA_EVENT message (or waiting for the IChroma.DeviceAccess event), and for that you need to be hooked up in a Windows event loop (and call `IChroma.HandleMessage). Considering that Colore can deal with both the native API and the REST API, it would be a bad idea to bake in all this logic only for the native implementation. I'd rather document this clearly so users know what to expect.

I'd rather document this clearly so users know what to expect.

Hm, that might be the way to go. Document it and then users can either use the event loop if they have it available (optimal), or insert an artificial delay of a second or two that should leave enough time to init (not as accurate, but probably good enough if it's not feasible to get the event loop).

Latest docs in release branch now have a note in the getting started guide about needing to wait for a couple seconds or use the method of listening for the DeviceAccess event.