I had failed to check that the queue length was non-zero before checking release delay
timeouts, causing reading past the end of the `key_queue_` array an repeatedly sending
essentially random input to the host.
In the process of fixing that bug, I realized that I was also assuming that layer changes
weren't happening earlier in the queue and checking whether or not a key is a qukey when
it wasn't the head of the queue. Now we only enact a release delay when `flushKey()` is
called, and always call `setQukeyState()` when enqueuing new keys so that we can
distinguish between keys that should be immediately flushed in the primary state, and ones
that should have keypresses delayed.
When releasing a qukey, allow a short timeout in case a subsequent key was released
effectively simultaneously, treating that near-simultaneous release as intended to use the
alternate (i.e. modifier) keycode.
Any keyswitch events without real physical addresses were injected by other plugins and
should not be processed by Qukeys. There was an interaction with OneShot that prevented
the two plugins from working together because OneShot sends events with a (row, col)
address of `UNKNOWN_KEYSWITCH_LOCATION` (i.e. 255, 255). This meant that if a OneShot
modifier was on when a Qukey was pressed, it would fill up the queue with bogus-address
versions of the Qukey, which would then get flushed in the primary state, cancelling the
OneShot and producing an un-modified, repeating primary keycode, causing both plugins to
fail.
Since keyboardio/Kaleidoscope-Hardware-Model01#23 we do not call
`handleKeyswitchEvent` for keys that are idle. Document this in the comments.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
When a keyswitch has been off in the previous cycle and is still off now, do not
call `handleKeyswitchEvent` on it. As an extension, when the whole column is
idle, skip the whole thing.
In practice, handling fully idle keys is not useful. There are many plugins
which explicitly look for this case and return early, because it isn't an
interesting event. As such, not calling the event handler in this case makes
sense, as we save not only a few needless checks in plugins, but our performance
improves greatly too.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
In `actOnMatrixScan`, there is no need to use temporary variables, we can just
pass the data directly to `actOnHalfRow`, and doing so makes the code easier to
follow.
In `actOnHalfRow`, we can further optimize things if instead of reading the Nth
bit, we always read the first, and shift the byte at the end.
All of these optimizations were done by @obra, I just wrote the commit message.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
When there are no state changes, and no keys pressed on a row, instead of
iterating through a byte bit-by-bit, just fire idle events without checking the
bits. In all other cases, do the bit-walking like we did before.
The reason this is useful is because bit-walking is costly, and slow. If we can
avoid that, we win quite a lot of performance. Since rows being idle is the most
common case on a keyboard, this is a huge net win. Even in the worst case, where
no rows are idle, this is just one byte comparison and a branch slower than our
previous implementation.
As part of this optimization, `actOnHalfRow` was lifted out into its own
function, to reduce code duplication.
Many thanks to @gedankenexperimenter for the original idea!
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
I made a bunch of changes to improve SpaceCadet for the better.
1. I resolved issue #9 ( https://github.com/keyboardio/Kaleidoscope-SpaceCadet/issues/9 ) to support mappings on Alt keys under Windows. This change in behavior means that we don't send the initial key value (the key with a mapping) until we hit the timeout (if held) or if we hit another key in combination in the mean time (to keep modifiers working as expected in combination with other keys). This means that when you place a mapping on Alt, we don't send Alt if you are just tapping -- we only send the other key value. This prevents Alt capturing the menu bar in Windows apps, and probably means we can better support SpaceCadet on non-modifier keys.
2. I added support for enabling, disabling, and determining if SpaceCadet is currently enabled. This allows other plugins and macros to better interact with SpaceCadet, and allows us to temporarily disable the behavior if that's desired.
3. I added two new virtual Key entries for placing on the user's keymap. One key disables SpaceCadet, and the other key enables SpaceCadet.
I also updated the README.md with all of the relevant changes.
It seems that if we inject the release of the mapped modifier key before injecting the press of the alternate key, we avoid the need to call `hid::sendKeyboardReport`, which in turn fixes#7. Which is nice.
Closes#7
It seems that if we inject the release of the mapped modifier key before injecting the press of the alternate key, we avoid the need to call `hid::sendKeyboardReport`, which in turn fixes#7. Which is nice.
Closes#7
Allow user to specify custom boot greeting key by Key_* or by specific row and column. Add ability to define custom duration of boot greet breathing effect, and add ability to change color hue of breathing effect. Finally, rework logic that happens when plugin is loaded to allow all user custom settings to be properly read and applied as expected.
add hue to the header
Updated readme to support new features
astyle + change to allow custom settings
There are a number of false-positives, where ShellCheck warns about behaviour we
do want, or are otherwise intentional.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
If the user has missed the step about setting up their account
with the right group membership, they would get a cryptic failure
from stty, so catch this and explain the problem.
This should never happen, but could if something goes badly wrong
in the device detection code, e.g. someone changing it in a way
which caused extra output on STDOUT.
7bd2e9fbb7/udev/99-hdmi2usb-mm-blacklist.rules (L4)
says:
It should be enough to set ID_MM_DEVICE_IGNORE:="1" but it seems many
versions of modem manager have a bug which makes them ignore that value.
Setting ID_MM_CANDIDATE:="0" has the same effect but is an "internal
implementation detail" of modem manager.
so set both to be safe. Also use := rather than =, in order to prevent
any later rules from overriding the settings.
In `hexToKeysWithNumpad`, have a default case, a pointless one, because we never
take that branch, but it silences a warning.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
Michael Richters
Gergely Nagy
Jesse Vincent
Gergely Nagy
Marty Gentillon
Ben Gemperline
Piers Cawley
Jesse Vincent
Chris King-Parra
Maxime de Roucy
Adam Spiers