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

CreateNativeAsync needs to be called twice #279

Closed
Shadowtail117 opened this issue Dec 31, 2019 · 5 comments
Closed

CreateNativeAsync needs to be called twice #279

Shadowtail117 opened this issue Dec 31, 2019 · 5 comments
Milestone

Comments

@Shadowtail117
Copy link

Shadowtail117 commented Dec 31, 2019

I'm following the Getting Started tutorial to set up basic key modifications, but whenever I run it, I have to click the button twice to get any changes. The first time it is clicked, my Chroma Studio effects freeze, and then clicking it again will set the colors correctly.
As demonstrated in #253, it looks like in order to successfully connect with Chroma, you have to initialize twice (since they had CreateNativeAsync followed by InitializeAsync), which can be a problem. Below is my code for reference, just in case I simply mistyped something (irrelevant usings are omitted). Anyone have a potential solution to this other than running the methods twice?

using Colore;
using ColoreColor = Colore.Data.Color;
using ColoreKey = Colore.Effects.Keyboard.Key;

///

private async void button1_Click(object sender, EventArgs e)
{
    var chroma = await ColoreProvider.CreateNativeAsync();
    await chroma.SetAllAsync(ColoreColor.White);
    await chroma.Keyboard.SetKeysAsync(ColoreColor.Red, ColoreKey.W, ColoreKey.A, ColoreKey.S, ColoreKey.D);
}
@Sharparam
Copy link
Member

This is likely related to #274. Some thinking needed on how best to deal with this.

@Sharparam Sharparam added this to the v6.1 milestone Jan 8, 2020
@poveden
Copy link

poveden commented Mar 13, 2020

After some testing related with #274, I've found that the call to ColoreProvider.CreateNativeAsync() does return immediately, but the Chroma SDK seems to perform some validation tasks (lasting around 250 ms - 750 ms) before it starts processing commands.

I've discovered that the Chroma SDK will actually signal when this validation has completed by emitting the WM_CHROMA_EVENT event with wParam = 2 (Access to device) and lParam = 1 (Access granted).

Colore provides an event handler for catching that event (IChroma.DeviceAccess) but, in order to get that event triggered, you need to:

  1. Set up a Win32 event pump (available in .NET with WinForms, WPF, or subclassing NativeWindow), and
  2. Override WndProc(ref Message m) and carefully pass the events to IChroma.HandleMessage().

By "carefully", I mean that the current implementation of HandleMessage (as of 6.0.0-rc0005) will throw if you haven't previously called IChroma.Register() or it doesn't like the hWnd handle, or if you close the window and forget to call IChroma.Unregister().

(@Sharparam please change this to just returning false, as throwing exceptions within an event pump it's not really a good idea; if you want, I can set up a PR for you 😜)

As an example, I'm trying something like this (untested):

public sealed class ChromaWindow : NativeWindow // Or System.Windows.Forms.Form, or WPF
{
    private IChroma _chroma;
    private bool _chromaMsgHandlerSet;
    private TaskCompletionSource<bool> _chromaAccessGranted = new TaskCompletionSource<bool>();

    protected override void WndProc(ref Message m)
    {
        if (_chromaMsgHandlerSet && _chroma.HandleMessage(m.HWnd, m.Msg, m.WParam, m.LParam))
        {
            return;
        }

        base.WndProc(ref m);
    }

    private async Task Init()
    {
        var hWnd = this.Handle; // Should be available after the HandleCreated event.

        _chroma = await ColoreProvider.CreateNativeAsync().ConfigureAwait(false);
        _chroma.Register(hWnd);
        _chromaMsgHandlerSet = true;
        _chroma.DeviceAccess += Chroma_DeviceAccess;

        var granted = await _chromaAccessGranted.Task.ConfigureAwait(false);

        if (!granted)
        {
            throw new Exception("Chroma SDK has denied access.");
        }
    }

    private void Chroma_DeviceAccess(object sender, Colore.Events.DeviceAccessEventArgs e)
    {
        _chromaAccessGranted.SetResult(e.Granted);
    }
}

Hope that it helps.

@Sharparam
Copy link
Member

@poveden Thanks for the input!

Apologies for the late response, have been absent from here for a while. I can change the behaviour of the handler to just log a warning and return false.

That would enable people to check it if they have access to the Win32 event system. For the cases where they don't have it available though I'm unsure how to best handle it. Got any ideas?

diff --git a/src/Colore/Implementations/ChromaImplementation.cs b/src/Colore/Implementations/ChromaImplementation.cs
index f1ee6b01..5934ea69 100644
--- a/src/Colore/Implementations/ChromaImplementation.cs
+++ b/src/Colore/Implementations/ChromaImplementation.cs
@@ -373,13 +373,20 @@ namespace Colore.Implementations
         public bool HandleMessage(IntPtr handle, int msgId, IntPtr wParam, IntPtr lParam)
         {
             if (!_registered)
-                throw new InvalidOperationException("Register must be called before event handling can be performed.");+            {
+                Log.Warn($"{nameof(HandleMessage)} called without event handling being registered");
+
+                return false;
+            }

             if (handle != _registeredHandle)
             {
-                throw new ArgumentException(
-                    "The specified window handle does not match the currently registered one.",
-                    nameof(handle));
+                Log.Warn(
+                    $"Unexpected handle passed to {nameof(HandleMessage)}. Expected 0x{{0}} but was 0x{{1}}",
+                    _registeredHandle.ToString("X"),
+                    handle.ToString("X"));
+
+                return false;
             }

             if (msgId != Constants.WmChromaEvent)

Sharparam added a commit that referenced this issue Jun 17, 2020
Based on feedback from @poveden in #279, the implementation of the Win32
event handling method (ChromaImplementation.HandleMessage) has been
changed to return `false` if event handling has not been registered,
or the handle not having the expected value, as opposed to throwing an
exception. This is to avoid unexpected behaviour in event processing.
@poveden
Copy link

poveden commented Jul 3, 2020

@Sharparam Sorry for my late response. 😛

I'd actually drop the log calls. You want event pumps to be as fast as possible. By the way Windows works, any delay here will delay all the messages that come after (this is the UI thread, after all). The logic is simple: If the message is not addressed to you, just return false.

That would enable people to check it if they have access to the Win32 event system.

I don't understand what you mean by this. Whoever links to Colore will do so either from a Win32 window (which needs an event pump) or from something else (which doesn't have an event pump). They should know in which context they are running.

This HandleMessage() method will only be called in the context of an event pump. If that pump doesn't receive any messages is probably because the app is not running in a Win32 window context (which would make having a pump useless in the first place).

I'd say that you leave it as is, unless you

  • a) Implement an event loop listener in Colore (which would make you depend on Microsoft.NET.Sdk.WindowsDesktop), or
  • b) Provide a helper library that does this job (I've built something like this, precisely to link with Colore, if you're interested).

Hope that it helps.

@Sharparam
Copy link
Member

As noted in #274 this behaviour is now documented in the getting started guide. Users need to create an artifical delay after creating the Chroma instance to allow the internal SDK to finish initializing, or wait for the DeviceAccess event to be generated with Granted set to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants