Cached KeyImages cannot be stored

[wojciechsura]

I created and stored a number of KeyImage images to improve performance during switching screens. However, during using such images, I ran into an exception:

Type:        System.ArgumentNullException
Message:     Value cannot be null. (Parameter 'key')
Call stack:     at System.Runtime.CompilerServices.ConditionalWeakTable`2.GetValue(TKey key, CreateValueCallback createValueCallback)
   at StreamDeckSharp.Internals.CachedHidClient.SetKeyBitmap(Int32 keyId, KeyBitmap bitmapData)
   at OpenMacroKeyboard.Drivers.StreamDeckV2.Driver.UpdateVisuals(List`1 buttons, List`1 encoders)
   at OpenMacroKeyboard.BusinessLogic.Services.DriverRepository.MacroKeyboard.UpdateKeyboardVisuals() in D:\Dokumenty\Dev\Pico\OpenMacroKeyboard\Desktop\OpenMacroKeyboard.BusinessLogic\Services\DriverRepository\MacroKeyboard.cs:line 48
   at OpenMacroKeyboard.BusinessLogic.Services.DriverRepository.MacroKeyboard.SetActiveScreen(Screen newScreen) in D:\Dokumenty\Dev\Pico\OpenMacroKeyboard\Desktop\OpenMacroKeyboard.BusinessLogic\Services\DriverRepository\MacroKeyboard.cs:line 76
   at OpenMacroKeyboard.BusinessLogic.Services.DriverRepository.MacroKeyboard.ExecuteMacro(BaseMacroInfo macro) in D:\Dokumenty\Dev\Pico\OpenMacroKeyboard\Desktop\OpenMacroKeyboard.BusinessLogic\Services\DriverRepository\MacroKeyboard.cs:line 104
   at OpenMacroKeyboard.BusinessLogic.Services.DriverRepository.MacroKeyboard.<>c__DisplayClass18_0.<OpenMacroKeyboard.API.V1.Drivers.IDriverHandler.HandleEvent>b__0() in D:\Dokumenty\Dev\Pico\OpenMacroKeyboard\Desktop\OpenMacroKeyboard.BusinessLogic\Services\DriverRepository\MacroKeyboard.cs:line 208
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

Workaround: Disable caching when creating Stream Deck device instance. Also. don't store KeyBitmaps, create them every time they are needed.

[wischi-chr]

Can you post some soft of reproducible example (maybe a little one-file console application?)

[wojciechsura]

I don't have minimal example, but you can reproduce the issue in the following way:

• Clone the repository https://gitlab.com/spook/OpenMacroKeyboard.git
• Switch to commit 9907a9209b8936d6b95edde0d2db0c8ea434f845
• You can notice in Desktop/Drivers/OpenMacroKeyboard.Drivers.StreamDeckV2/Driver.cs the following code:

        public override void UpdateVisuals(List<KeyboardElementVisuals> buttons, List<KeyboardElementVisuals> encoders)
        {
            if (deck == null)
                return;

            if (sleep)
            {
                deck.SetBrightness(brightness);
                sleep = false;
            }

            // Prepare images to display            

            for (int i = 0; i < buttons.Count; i++)
            {
                deck.SetKeyBitmap(i, buttons[i].CustomCache as KeyBitmap);
            }
        }

The last line is the place where buttons are set from local (application) cache.

• Rebuild whole solution (important!)
• Run the application
• Add a Stream Deck v2 keyboard
• Open layout configuration
• Add images to two buttons
• Set a button to a folder
• Add an image to a button in the folder (double-click the folder button preview to enter the folder).
• Confirm the layout
• Press the button on the Stream Deck, which corresponds to the folder
• You should get the said exception.

[wojciechsura]

I know the project is moderately big, but there are only two files you may be interested in, related to the issue.

• OpenMacroKeyboard.Drivers.StreamDeck\Driver.cs - contains the code, which creates KeyImage caches and later uses them to set images to buttons (methods BuildElementCustomCache, which creates the cache and UpdateVisuals, which uses it);
• OpenMacroKeyboard.BusinessLogic\Services\DriverRepository\MacroKeyboard.cs, method BuildCustomCacheRecursive - initiates the cache building process and stores objects returned by the driver for later usage.

[wischi-chr]

I haven't had time to try it with my stream deck v2 but I read a bit through the code and I'm pretty sure that you pass something that's actually null (which is not allowed). There is currently not an explicit null-check for this method (I should probably add one) but it will crash later down the line if you pass null to SetKeyBitmap.

// if buttons[i].CustomCache is not a KeyBitmap or null than this will throw!
deck.SetKeyBitmap(i, buttons[i].CustomCache as KeyBitmap);

CustomCache is if type object? so basically anything can be stored in that property. It's only assigned in the constructor of KeyboardElementVisuals but everything in this class is nullable, and there are multiple instances that instantiate the class with new KeyboardElementVisuals(null, null, null, null) and the only instantiation where there even could be a non-null value is inside ElementInfo where another CustomCache property of type object? is assigned by a constructor with a value b?.CustomCache with a null-conditional operator.

I actually stopped at that point because there are so many possibilities where this value could end up being null.

My guess is that something inside your caching code doesn't work as expected/intended and leads to null values inside CustomCache. A "quick-fix" would be to take the code inside the driver and handle the null case there.

// Note: just typed this in GitHub and have not compiled/tested it.
// Prepare images to display            

for (int i = 0; i < buttons.Count; i++)
{
    if (buttons[i].CustomCache is KeyBitmap kb)
    {
        deck.SetKeyBitmap(i, kb);
    }
    else
    {
        // CustomCache was null or not of type KeyBitmap
        // throw Exception here or set break-point to diagnose the issue
        // or log the error event
        // or fall back to a different (known) KeyBitmap that is actually non-null
    }
}

On the long run you should try to be way stricter with all of your types. There should be very good reasons to use object? and if you do should should take extra precautions that the type and value stored in such properties/variables are the ones you expect. You've enabled nullable-reference types (which is great) but you should try to keep most things non-nullable and design abstractions differently.