# Extra key reports (potentially) causing errors, and how to improve that **URL:** \ **Category:** Kaleidoscope Internals\ **Created:** [December 7, 2017, 5:01pm UTC](https://community.keyboard.io/t/extra-key-reports-potentially-causing-errors-and-how-to-improve-that/830 "2017-12-07T17:01:09Z")\ **Posts on this page:** 4\ **Page:** 1
**Author:** ![merlin](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/merlin/32/3416_2.png) [@merlin](https://community.keyboard.io/u/merlin)\ **Post date:** [December 7, 2017, 5:01pm UTC](https://community.keyboard.io/t/extra-key-reports-potentially-causing-errors-and-how-to-improve-that/830/1 "2017-12-07T17:01:09Z")
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-report `loopHook` pass, 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: 1. Is this a good idea? 2. 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? ---
**Author:** ![merlin](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/merlin/32/3416_2.png) [@merlin](https://community.keyboard.io/u/merlin)\ **Post date:** [December 7, 2017, 5:02pm UTC](https://community.keyboard.io/t/extra-key-reports-potentially-causing-errors-and-how-to-improve-that/830/2 "2017-12-07T17:02:06Z")
Third question: Is this a real problem, or is this just excessive perfectionism on my part? ---
**Author:** ![algernon](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/algernon/32/4404_2.png) [@algernon](https://community.keyboard.io/u/algernon)\ **Post date:** [December 7, 2017, 5:49pm UTC](https://community.keyboard.io/t/extra-key-reports-potentially-causing-errors-and-how-to-improve-that/830/3 "2017-12-07T17:49:03Z")
I think this is a real problem, there are a number of plugins that could benefit from this, or something like this. I’m not entirely convinced that what you proposed above is something that covers all the cases I have in mind, but I need to spend a few more brain cycles to properly understand both your proposal, and @noseglasses’s PR. (ETA: tomorrow) ---
**Author:** ![jesse](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jesse/32/361_2.png) [@jesse](https://community.keyboard.io/u/jesse)\ **Post date:** [December 7, 2017, 11:13pm UTC](https://community.keyboard.io/t/extra-key-reports-potentially-causing-errors-and-how-to-improve-that/830/4 "2017-12-07T23:13:46Z")
> [@merlin](#): > > 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? > [@merlin](#): > > Third question: Is this a real problem, or is this just excessive perfectionism on my part? Given the two of these, I _think_ it makes sense to hold off. What _would_ be useful is figuring out specific failure cases. Exact sequences of events that would cause the problem you’re worried about. These are the sorts of test cases that we should be thinking about adding once we have a decent test harness (Do note that we have guards in the HID layer to prevent sending duplicate key reports.)