This is mostly a theoretical problem; I don’t know if anyone has actually observed this in the field yet, but I’ve just recently released a plugin that might replace Kaleidoscope-DualUse at some point, and it’s much more likely than most plugins to trigger this potential bug. Here’s a summary:
When the event handler hooks are being called by handleKeyswitchEvent() from the main loop (via KaleidoscopeHardware.actOnMatrixScan()), a plugin might try to guess (it’s hard to know for sure) if – for example – a modifier key is active. To do this, it’s better to check the previous HID report using wasModifierActive() because the current report is incomplete. If that plugin makes a decision based on that information – and let’s suppose that information is correct – it could be invalidated by a different plugin acting on a later key which triggers an injected hid::sendKeyboardReport() before the main report is sent by Kaleidoscope at the end of the main loop.
Now, I’ve made a plugin that sends such reports, and it currently works by copying the previous report, and modifying it by calling handleKeyswitchEvent(), then sending the modified report, then restoring the state of the in-progress report before returning and letting the main loop proceed. To make matters worse, it is quite likely to change the modifier state of not just the next (in-progress) report, but also the state of the (new) previous report, since it’s whole raison d’être is to overload letter keys with modifiers. This could retroactively make prior calls to wasModifierActive() during the same scan cycle incorrect.
The only solution I’ve come up with is to get Kaleidoscope to re-start the event handler loop, stopping when it gets to the key currently being handled. This would give other plugins a chance to get the correct information about the previous keyboard report, but it has a couple of potential drawbacks:
- Lots of extra calls to
handleKeyswitchEvent(), the vast majority of which won’t produce any different result - A plugin could save state based on an aborted call to
handleKeyswitchEvent()without realizing it, possibly resulting in very unpredictable behaviour (e.g. if it increments a counter, then acts on that counter during the regular pre-reportloopHookpass, the counter would have the wrong value). - Memory limits could be exceeded by the recursive nature of calling
handleKeyswitchEvent()from itself, possibly even triggering infinite (until we run out of memory) recursion.
I have a much deeper solution in mind, but it depends on some much bigger architectural changes in Kaleidoscope, beginning with @noseglasses’s changes to the plugin hook interface, but that’s a bit of a separate discussion.
I have a working version of Kaleidoscope that implements the changes necessary to make it possible to restart the event handler pass. It changes the hardware modules:
- Kaleidoscope-Hardware
- Kaleidoscope-Hardware-Model01
- Kaleidoscope-Hardware-Shortcut
I added a function that’s basically a version of actOnMatrixScan(), but which takes row & column parameters, and stops after the given coordinates. This could be done with less code by modifying actOnMatrixScan() to take those parameters and test the conditionals for every key on every scan. I don’t know whether the code size matters more than the speed difference.
Two big questions, then:
- Is this a good idea?
- Even if it is an improvement, should we bother with this kind of change, or should we drop it because there are better ways to make it irrelevant (I have one, I think) once the plugin hook interface improvements land?