The plugin was both more complex and less accurate than it could have been. For
simplicity, it used a weighted average, with each cycle getting twice the weight
of the previous one. As a result, the reported average really only took into
account the last three or four cycles. On a keyboard with LEDs, some cycles
take much longer than others because of relatively rare updates, so this could
lead to misleading results, with the "average" cycle time usually being reported
as lower than it really should have been, and occasionally much higher.
This new version computes an evenly-weighted mean cycle time for each interval,
and runs more efficiently, by dividing the total elapsed time by the number of
cycles that has passed since the last report, rather than computing the time for
each individual cycle.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
We'd like to be able to set the default LED mode via Focus, so it can be
configured via Chrysalis. However, we may not want auto-save, so make that
configurable too.
To preserve the EEPROM layout, the highest bit of the previous led mode index
setting was repurposed for the auto save setting. This lets us set the default
mode to anything between 0 and 126 (or 127, if auto save is turned off).
While there, we also add an `onNameQuery` handler, to make it easier for
Chrysalis to detect if the plugin is available.
This addresses the Kaleidoscope parts of keyboardio/Chrysalis#846.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
I think it's reasonable to assume that other plugins won't be bad actors and
remove an active Turbo key from the live keys array unceremoniously, so this
check is really unnecessary.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
Previously, the "sticky" state was simply ignored. Now it's handled properly,
leaving the "sticky" active Turbo key in the live keys array.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
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>
The plugin was restricted to the Model01, because it depends on a very specific
key coordinate -> geometric shape mapping. Because the Model 01 and the Model
100 share this mapping, we can safely enable the plugin for the latter, too.
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>
We should be treating the end of data the same way we treat a newline.
A common issue that affects a good number of plugins is that we don't deal with
trailing space well. For example, most plugins that respond with a large list of
values, where we iterate over an array or list, or something else, they usually
end up responding with a trailing space before the newline.
If we feed that same string back as an update, we can end up in a situation
where we lock up (or become very, very slow), because we want to read more data
than is available. Why? Because `Serial.parseInt()` (used by Focus under the
hood) will swallow up any leading whitespace. So if we have "255 \n" as an
argument list, we'll parse the first number, and the second `parseInt()` will
return 0, because it times out waiting for a number, consuming both the space
and the newline in the process. Thus, the next `::isEOL()` will still return
false, because `peek()` returns `-1`, signaling no data.
That can confuse the heck out of our plugins. To combat that, we should treat
end of data the same as EOL, and return false if `peek()` returns -1, too.
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>
This matches the default EEPROM size of the underlying FlashStorage library, and
is substantially bigger than AVR-based keyboards, yet not _too_ big.
For various reasons, we're mirroring EEPROM into RAM 1:1, so we're constrained
by the size of that, too. That makes larger storage sizes undesirable at this
time. On top of this limitation, larger storage sizes also pose backup & restore
speed issues with Chrysalis, so lets settle for 16k, which is still very big,
all things considered, but not big enough to be a problem.
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>
Tested to work on both a Model 01 and a Model 100. This will become
unnecessary when the GD32 Arduino core grows the ability to autoflush on
SOF packets from the host
Switch the plugin to use the `F303CC_GENERIC` variant rather than `F303ZE_EVAL`,
to match the hardware more closely. Also drop the keyscanner, we do not need
that for most testing, and it was just complicating things.
On top of those simplifications, add a `serialPort()` method to make
`FocusSerial` work, and `rebootBootloader()` to make rebooting function as one
would expect.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
There are a number of places in our code where clang-format tries too hard, and
destroys human readability, so I protected them with `clang-format off`
directives.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
Three plugins (`AutoShiftConfig`, `EscapeOneShotConfig` and `TypingBreaks`) that
used the same checker pattern to see if their storage slice is uninitialized now
use the new `storage().isSliceUninitialized()` method instead.
This reduces code duplication, among other things.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
This standardizes namespace closing brackets for namespace blocks. Each one is
on its own line, with a comment clearly marking which namespace it closes.
Consecutive lines closing namespace blocks have no whitespace between them, but
there is one blank line before and after a set of namespace block closing lines.
To generate the namespace comments, I used clang-format, with
`FixNamespaceComments: true`. But since clang-format can't exactly duplicate
our astyle formatting, it made lots of other changes, too. To isolate the
namespace comments from the other formatting changes, I first ran clang-format
with `FixNamespaceComments: false`, committed those changes, then ran it again
to generate the namespace comments. Then I stashed the namespace comments,
reset `HEAD` to remove the other changes, applied the stashed namespace
comments, and committed the results (after examining them and making a few minor
adjustments by hand).
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>
To make it easier - or in some cases, even possible - to put the keyboard into
programmable mode, we need to be able to initiate a reset from the host side.
For example, ynlike on AVR, where we can send a HUP to the serial port, on GD32,
we need to do it a bit differently: by sending the keyboard a Focus request to
reboot itself.
This implements the command itself.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
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>
Instead of hardcoding 255 in the `eeprom.erase` handler, use a constant instead,
to make it clear what the number is.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
The `eeprom.erase` command makes it easier to erase the whole of EEPROM, and in
addition, it will reboot the keyboard so that the changes are picked up by every
single plugin.
Fixes#1134.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
When handling the update variant of `eeprom.contents`, we need to commit the
update storage, too. This does not affect AVR that doesn't need an explicit
commit, but it does affect ARM, where `eeprom.contents` likely didn't work at
all in update mode.
Signed-off-by: Gergely Nagy <algernon@keyboard.io>
* Move MouseWrapper class into the `plugin::mousekeys` namespace
This helper class is meant to be internal to MouseKeys, so it would be best to
sequester it from the shared `kaleidoscope::plugin` namespace.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Remove unnecessary explicit MouseWrapper constructor
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Remove trailing underscore from MouseWrapper class name
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Rename Mousewrapper methods to comply with coding style guide
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Make MouseWrapper warping utility methods private
MouseKeys doesn't call these functions directly, and making them private helps
make the API for MouseWrapper clearer.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Make MouseWrapper public variables comply with code style guide
Variables should be in `snake_case`. Since MouseKeys is the only client of
`MouseWrapper`, I don't feel it's necessary to go through a deprecation process
for the sake of this rename.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Remove unnecessary explicit MouseKeys constructor
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Remove trailing underscore from MouseKeys class name
Because the `MouseKeys_` class is now contained in the `kaleidoscope::plugin`
namespace, but the `MouseKeys` object is in the global namespace, we can safely
drop the trailing underscore from the class name, like most other plugins.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Add some namespace comments
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Make `MouseWrapper::subpixels_per_pixel` constexpr
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Change MouseKeys variables from `static` to member variables
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Change name of MouseWrapper object
Instead of `kaleidoscope::plugin::MouseWrapper`, it is now named like a
variable: `kaleidoscope::plugin::mousekeys::wrapper`.
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Downcase directory name to match namespace
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
* Adjust header includes to match new dirname
Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
Michael Richters
Gergely Nagy
Jesse Vincent
Jesse Vincent
Gergely Nagy