GeekyEggo / SharpDeck

A .NET wrapper for the official Elgato Stream Deck SDK.
MIT License
25 stars 9 forks source link

SetStateAsync sometimes does not work #15

Closed oddbear closed 2 years ago

oddbear commented 2 years ago

Hi.

Not sure if I am using the plugin wrong here, or this is a bug.

I have a button, when I press it, it calls a service and changes a value. When this value is changed, the service will send a callback that triggers an event, that then calls SetStateAsync from a method inside the Action.

I have added Trace.WriteLine that writes out the state I want, but sometimes this is not updated on the button.

If I change the value in the application, without pressing the button, I have not seen this fail (yet).

GeekyEggo commented 2 years ago

Hey @oddbear, action states are an odd one. Depending on how they're used, I often find they can produce unexpected behaviour, with that said however the behaviour should always be consistent, i.e. with a little trial and error in code they'll work.

One thing to note is; payload.state is the current state of the action, whereas payload.userDesiredState is the state the action is becoming. The latter of the two might simplify things.

The plugin "Sound Deck" uses Sharp Deck, and consumes states in two different ways:

  1. Record - This uses state as a "toggle", i.e. the recording starts and stops based on the state.
  2. Sampler - Controls the state based on whether an audio file is associated with an action.

Do you have a code snippet available? I'd be happy to take a look.

oddbear commented 2 years ago

I will look into it. This is my first Stream Deck plugin (only made Touch Portal and Loupedeck before), so there might be a couple of things I have misunderstood. The code can be found at: https://github.com/oddbear/Revelator.io24.Api/blob/master/Revelator.io24.StreamDeck/Actions/FatChannelToggleAction.cs

GeekyEggo commented 2 years ago

Learning from pain points I encountered with Sound Deck, my only suggestion would be to resist the urge to persist things in memory (unless absolutely necessary). Originally I was persisting things like settings in memory against the action instance, and quickly found it became difficult to synchronize. Other than that, it looks like you're certainly on the right path.

Consider removing:

private FatChannelToggleSettings _settings;
private int? _lastState = null;

Which would also allow you to remove:

protected override async Task OnDidReceiveSettings(ActionEventArgs<ActionPayload> args)

You can then access the settings directly from the payload, which makes the OnKeyDown look like this:

protected override async Task OnKeyDown(ActionEventArgs<KeyPayload> args)
{
    await base.OnKeyDown(args);

    var settings = args.Payload.GetSettings<FatChannelToggleSettings>();
    var action = Action();

    _updateService.SetRouteValue(_settings.Route, action);
}

Calling SetStateAsync is minimal, so I think it should also be safe to remove the guard in UpdateState:

private async Task StateUpdated()
{
    var state = GetCurrentState() ? 0 : 1;

    Trace.WriteLine($"Fat state: {state}, count: {_count}, context: {base.Context}");
    await SetStateAsync(state);
}
oddbear commented 2 years ago

Closing this one, as it doesn't seem to be a issue with the SDK. Thanks for the help.