From c9534155e6849442b3e76033235ed5f73f127fce Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Thu, 28 Jan 2021 09:32:21 -0600 Subject: [PATCH] Clarify the Macros plugin `Key` value range This replaces the magic number `24576` with the old definition of the start of the Macros plugin's `Key` range, to make it clear that it will match bitwise tests for "not a plugin key" (the two high bits are both zeros) and the `SYNTHETIC` flags bit. I also elaborated on the reasons for keeping the existing `Key` values stable and added a comment addressing the more likely case where new values are being added, rather than fighting the last war and focusing only on the one legacy plugin range that exists outside the canonical plugin range, whose values have already been incorporated here. Signed-off-by: Michael Richters --- .../src/Kaleidoscope-Ranges.h | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/plugins/Kaleidoscope-Ranges/src/Kaleidoscope-Ranges.h b/plugins/Kaleidoscope-Ranges/src/Kaleidoscope-Ranges.h index d60ff94d..75a87388 100644 --- a/plugins/Kaleidoscope-Ranges/src/Kaleidoscope-Ranges.h +++ b/plugins/Kaleidoscope-Ranges/src/Kaleidoscope-Ranges.h @@ -17,6 +17,9 @@ #pragma once +// Included for definition of legacy Macros plugin key range: +#include "kaleidoscope/key_defs.h" + namespace kaleidoscope { namespace ranges { @@ -24,13 +27,25 @@ namespace ranges { // // When adding, removing, or changing ranges, make sure that existing ranges are // never accidentally moved. If migrating keycodes that weren't previously using -// the rang system, make sure you keep the old keycodes, even if they short -// before ranges::FIRST, for the sake of remaining backwards compatible with -// existing keymaps. +// the range system, make sure you don't change the `Key` values. If an existing +// `Key` value is changed, it won't be a problem for the Kaleidoscope sketch +// itself, but it will cause problems for keymap entries stored in EEPROM +// (i.e. Chrysalis keymap layers), which will not get updated by flashing the +// firmware. +// +// When adding new `Key` values for plugins, to keep them backwards-compatible, +// they must be added at the end of the range below (just before `SAFE_START`), +// even if those values are logically related to existing ones. This is +// important for compatibility with existing Chrysalis keymaps, despite the fact +// that it makes the code more obtuse here. + enum : uint16_t { // Macro ranges pre-date Kaleidoscope-Ranges, so they're coming before // ranges::FIRST, because we want to keep the keycodes backwards compatible. - MACRO_FIRST = 24576, + // This is undesirable, because it prevents us from making a clear distinction + // between plugin key values and core key values. The magic number + // `0b00100000` is the old `IS_MACRO` key flags bit. + MACRO_FIRST = (SYNTHETIC | 0b00100000) << 8, MACRO_LAST = MACRO_FIRST + 255, FIRST = 0xc000,