In the past, we were using `Focus.handleHelp()` to see if we're handling a
`help` command, and print the commands supported by the handler. We also used
`strcmp_P` directly to compare (parts of) our input against command supported by
the handler. This approach works, but had multiple major disadvantages: it
duplicated strings between `handleHelp` and the `strcmp_P` calls, and it relied
on fragile substring pointers to save space.
To replace all that, this patch implements a different approach. Help handling
is split between a check (`Focus.inputMatchesHelp()`) and a
reply (`Focus.printHelp()`), the latter of which takes a list of `PSTR()`
strings, rather than one single string. This allows us to reuse the same
strings for comparing against the handler's input.
The new approach no longer uses the fragile substring pointers, nor does it use
`strcmp_P` directly, but goes through a wrapper (`Focus.inputMatchesCommand()`)
instead.
These changes lead to a more readable pattern. While we do use longer strings as
a result, there is less duplication, and the new patterns also require less
code, so we end up with saving space, at least on AVR devices.
The old methods are still available and usable, but they're deprecated.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
Our lookup table should have 32 entries, not 31, as Kaleidoscope-Ranges gives
DynamicMacros 32 entries.
Thanks @gedankenexperimenter for spotting this!
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
There were a number of problems with how we updated and handled our cache.
First of all, we did not support empty macros that consist only of a single
`MACRO_ACTION_END`: we returned immediately as soon as we encountered one such.
This is undesirable, we want to support such empty macros.
Seconds, we did _not_ bail out early when encountering an unknown step. We
continued reading from storage until we reached our slice's end. That's also
undesirable, because data past an unknown step is not something we can reliably
parse. We should have bailed out here.
On top of that, we did not keep our id->location map in good shape. The initial
cache update did the right thing, but if we did an update where we ended up with
less macros, our map would have dangling pointers for macro ids that no longer
exist. That's not a problem when our update clears the rest of the storage
slice, but if it doesn't, the results of trying to run an unknown macro would be
unpredictable. Even if we don't care about that, it's still very inefficient,
especially when we have large macro storage.
So, this update does a whole lot of things to improve the situation:
We now keep track of how many macros we find during a cache update, and will
refuse to play macro ids that are beyond our count. This improves efficiency,
and protects us from trying to run garbage.
We also support empty macros now, and return early from a cache update when we
encounter an unknown step type.
Fixes#1177.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
When updating our map via Focus, do not read past `storage_size_`, because we do
not want to clobber storage space past our slice by accident.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
DynamicMacros was missing a necessary `beforeReportingState()` handler that is
responsible for adding keys held by an active macro to the HID report. This
handler is identical to the one used by the Macros plugin for the same purpose.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
Both Macros and DynamicMacros were only reading one byte for each `Key` object
in a tap sequence, so it would first read the flags byte of each key in the
sequence and treat it as a keycode byte, using a flags byte of `0`. As soon as
an unmodified keyboard key was encountered, this would be recognized as the end
of the sequence. This change fixes the bug by reading and using the flags byte
of each key in the sequence as intended.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
This is the result of running the `include-what-you-use` wrapper, followed by
the `clang-format` wrapper on the Kaleidoscope codebase. It is now safe to use
both without needed any manual corrections after the fact, but it's still
necessary to run clang-format after IWYU, because the two differ in the way they
indent comments after header files.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
DynamicMacros reads sequences from EEPROM, not PROGMEM, so it needs to call
`Runtime.storage().read()` instead. The code was copied from Macros, but a
couple of spots were missed.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
In some cases, we still need preprocessor macros to preserve the same keymap
markup as before, because they convert `X` to `Key_X` (for example).
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
This removes the `key_events.*` files that once contained the main
`handleKeyswitchEvent()` function, and all references to it. Because
`key_events.h` was included in the main `Kaleidoscope.h` header file,
`key_defs.h` and `keyswitch_state.h` were added to that header so that other
code that relies on those things being included via `Kaleidoscope.h` will
continue to work.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
This implements a new `FocusSerial` command: `plugins`. The `plugins` command
will reply back with a list of plugins enabled in the firmware. The list is not
exhaustive, only plugins that opt-in to this mechanism will be listed. It is
opt-in, because for a lot of plugins, having them listed isn't useful in a
practical sense.
The goal with this feature is to allow Chrysalis to detect plugins that would
affect what keys it offers, or which additional settings it displays, and do so
in a consistent way. This is why IdleLEDs has an `onNameQuery` handler too, even
though it can be detected otherwise: for consistency.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
Gergely Nagy
Taylor Yu
Michael Richters
Jesse Vincent