Skip to content
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

NativeApi.InitializeAsync() not really async #274

Closed
poveden opened this issue Sep 3, 2019 · 5 comments
Closed

NativeApi.InitializeAsync() not really async #274

poveden opened this issue Sep 3, 2019 · 5 comments

Comments

@poveden
Copy link

poveden commented Sep 3, 2019

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?

@poveden poveden changed the title NativeApi.InitializeAsync() not really async NativeApi.InitializeAsync() not really async Sep 3, 2019
@Sharparam
Copy link
Member

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

@Sharparam Sharparam added this to the v6.1 milestone Sep 3, 2019
@Sharparam Sharparam added this to Backlog in v6.1 via automation Sep 3, 2019
@Sharparam
Copy link
Member

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?

@poveden
Copy link
Author

poveden commented Jan 29, 2021

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.

@Sharparam
Copy link
Member

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).

@Sharparam
Copy link
Member

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.

v6.1 automation moved this from Backlog to Done Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v6.1
  
Done
Development

No branches or pull requests

2 participants