Merge pull request #6 from keyboardio/f/new-api

Complete redesign of the plugin

README.md

@@ -18,88 +18,54 @@ This can be used to tie complex actions to key chords.
 
 ## Using the extension
 
-To use the extension, we must include the header, create an array of combos we
+To use the extension, we must include the header, create actions for the magic
-want to work with, let the plugin know we want to work with those, and then use
+combos we want to trigger, and set up a mapping:
-a special function to handle the combos:
 
 ```c++
 #include <Kaleidoscope.h>
 #include <Kaleidoscope-Macros.h>
 #include <Kaleidoscope-MagicCombo.h>
 
-void magicComboActions(uint8_t combo_index, uint32_t left_hand, uint32_t right_hand) {
+enum { KIND_OF_MAGIC };
-  switch (combo_index) {
+
-  case 0:
+void kindOfMagic(uint8_t combo_index) {
   Macros.type(PSTR("It's a kind of magic!"));
-    break;
-  }
 }
 
-static const kaleidoscope::MagicCombo::combo_t magic_combos[] PROGMEM = {
+USE_MAGIC_COMBOS(
-  {
+[KIND_OF_MAGIC] = {
-    R3C6,  // left palm key
+  .action = kindOfMagic,
-    R3C9   // right palm key
+  .keys = {R3C6, R3C9} // Left Fn + Right Fn
-  },
+});
-  {0, 0}
-};
 
 KALEIDOSCOPE_INIT_PLUGINS(MagicCombo, Macros);
 
 void setup() {
   Kaleidoscope.setup();
-
-  MagicCombo.magic_combos = magic_combos;
 }
 ```
 
-The combo list **must** reside in `PROGMEM`, and is a list of tuples. Each
+It is recommended to use the `RxCy` macros of the core firmware to set the keys
-element in the array has two fields: the left hand state, and the right hand
+that are part of a combination.
-state upon which to trigger the custom action. Both of these are bit fields,
-each bit set tells the extension that the key with that index must be held for
-the action to trigger. It is recommended to use the `RxCy` macros of the core
-`KaleidoscopeFirmware`, and *or* them together to form a bitfield.
-To see how the `RxCy` coordinates correspond to the physical keys of your
-keyboard, you'll have to consult the documentation for the keyboard.
-Below, you can find a diagram showing the layout for the Keyboardio Model 01.
-
-The combo list **must** end with an element containing zero values for both the
-left and the right halves.
 
-## Extension methods
+## Plugin properties
 
 The extension provides a `MagicCombo` singleton object, with the following
-methods and properties:
+property:
-
-### `.magic_combos`
-
-> Setting this property lets the plugin know which combinations of key presses
-> we are interested in. If any of these are found active, the
-> `magicComboActions()` function will be called.
 
 ### `.min_interval`
 
-> Restrict the magic action to fire at most once every `minInterval`
+> Restrict the magic action to fire at most once every `min_interval`
 > milliseconds.
 >
 > Defaults to 500.
 
-## Overrideable methods
+## Plugin callbacks
 
-Whenever an combination is found to be held, the extension will trigger an
+Whenever a combination is found to be held, the plugin will trigger the
-action, in each scan cycle until the keys remain held. This is done by calling
+specified action, which is just a regular method with a single `uint8_t`
-the overrideable `magicComboActions` function:
+argument: the index of the magic combo. This function will be called repeatedly
-
+(every `min_interval` milliseconds) while the combination is held.
-### `magicComboActions(combo_index, left_hand, right_hand)`
-
-> Called whenever a combination is found to be held. The function by default
-> does nothing, and it is recommended to override it from within the Sketch.
->
-> The first argument will be the index in the combo list, the other two are the
-> key states on the left and right halves, respectively.
->
-> Plugins that build upon this extensions *should not* override this function,
-> but provide helpers that can be called from it. An override should only happen
-> in the Sketch.
 
 ## Further reading
 

UPGRADING.md

@@ -0,0 +1,112 @@
+Breaking changes in `MagicCombo`
+================================
+
+To make `MagicCombo` more portable, and easier to use, we had to break the API
+previously provided, there was no way to maintain backwards compatibility. This
+document is an attempt at guiding you through the process of migrating from the
+earlier API to the current one.
+
+Migration should be a straightforward process, but if you get stuck, please feel
+free to [open an issue][gh:issues], or start a thread on the [forums][forums],
+and we'll help you with it.
+
+ [gh:issues]: https://github.com/keyboardio/Kaleidoscope-MagicCombo/issues
+ [forums]: https://community.keyboard.io/
+
+## The old API
+
+```c++
+void magicComboActions(uint8_t combo_index, uint32_t left_hand, uint32_t right_hand) {
+  switch (combo_index) {
+  case 0:
+    Macros.type(PSTR("It's a kind of magic!"));
+    break;
+  }
+}
+
+static const kaleidoscope::MagicCombo::combo_t magic_combos[] PROGMEM = {
+  {
+    R3C6,  // left palm key
+    R3C9   // right palm key
+  },
+  {0, 0}
+};
+
+void setup() {
+  Kaleidoscope.setup();
+
+  MagicCombo.magic_combos = magic_combos;
+}
+```
+
+Previsouly, we used a global, overrideable function (`magicComboActions`) to run
+the actions of all magic combos, similar to how macros are set up to work.
+Unlike macros, magic combos can't be defined in the keymap, due to technical
+reasons, so we had to use a separate list - `magic_combos` in our example. We
+also needed to tell `MagicCombo` to use this list, which is what we've done in
+`setup()`.
+
+## The new API
+
+```c++
+void kindOfMagic(uint8_t combo_index) {
+  Macros.type(PSTR("It's a kind of magic!"));
+}
+
+USE_MAGIC_COMBOS({
+  .action = kindOfMagic,
+  .keys = {R3C6, R3C9} // Left Fn + Right Fn
+});
+```
+
+The new API is much shorter, and is inspired by the way the [Leader][leader]
+plugin works: instead of having a list, and a dispatching function like
+`magicComboActions`, we include the action method in the list too!
+
+ [leader]: https://github.com/keyboardio/Kaleidoscope-Leader
+
+We also don't make a difference between left- and right-hand anymore, you can
+just list keys for either in the same list. This will be very handy for
+non-split keyboards.
+
+## Migration
+
+First of all, we'll need to split up `magicComboActions` into separate
+functions. Each function should have a unique name, but their shape is always
+the same:
+
+```c++
+void someFunction(uint8_t combo_index) {
+ // Do some action here
+}
+```
+
+Copy the body of each `case` statement of `magicComboActions`, and copy them one
+by one into appropriately named functions of the above shape. You can name your
+functions anything you want, the only constraint is that they need to be valid
+C++ function names. The plugin itself does nothing with the name, we'll
+reference them later in the `USE_MAGIC_COMBOS` helper macro.
+
+Once `magicComboActions` is split up, we need to migrate the `magic_combos` list
+to the new format. That list had to be terminated by a `{0, 0}` entry, the new
+method does not require such a sentinel at the end.
+
+For each entry in `magic_combos`, add an entry to `USE_MAGIC_COMBOS`, with the
+following structure:
+
+```c++
+{.action = theActionFunction,
+ .keys = { /* list of keys */ }}
+```
+
+The list of keys are the same `RxCy` constants you used for `magic_combos`, with
+the left- and right hands combined. The action, `theActionFunction`, is the
+function you extracted the magic combo action to. It's the function that has the
+same body as the `case` statement in `magicComboActions` had.
+
+And this is all there is to it.
+
+If your actions made use of the `left_hand` or `right_hand` arguments of
+`magicComboActions`, the same information is still available. But that's a bit
+more involved to get to, out of scope for this simple migration guide. Please
+open an issue, or ask for help on the forums, and we'll help you.

examples/MagicCombo/MagicCombo.ino

@@ -20,6 +20,16 @@
 #include <Kaleidoscope-Macros.h>
 #include <Kaleidoscope-MagicCombo.h>
 
+enum {
+  KIND_OF_MAGIC
+};
+
+void kindOfMagic(uint8_t combo_index) {
+  Macros.type(PSTR("It's a kind of magic!"));
+}
+
+USE_MAGIC_COMBOS([KIND_OF_MAGIC] = {.action = kindOfMagic, .keys = {R3C6, R3C9}});
+
 void magicComboActions(uint8_t combo_index, uint32_t left_hand, uint32_t right_hand) {
   switch (combo_index) {
     case 0:

src/Kaleidoscope/MagicCombo.cpp

@@ -18,43 +18,36 @@
 
 #include <Kaleidoscope-MagicCombo.h>
 
-#if defined(ARDUINO_AVR_MODEL01)
-#define LEFTHANDSTATE  KeyboardHardware.leftHandState
-#define RIGHTHANDSTATE KeyboardHardware.rightHandState
-#endif
-
-#if defined(ARDUINO_AVR_SHORTCUT)
-#define LEFTHANDSTATE  KeyboardHardware.scanner.leftHandState
-#define RIGHTHANDSTATE KeyboardHardware.scanner.rightHandState
-#endif
-
 namespace kaleidoscope {
 
-const MagicCombo::combo_t *MagicCombo::magic_combos;
 uint16_t MagicCombo::min_interval = 500;
 uint32_t MagicCombo::end_time_;
 
 EventHandlerResult MagicCombo::beforeReportingState() {
-  if (!magic_combos)
+  for (byte i = 0; i < magiccombo::combos_length; i++) {
-    return EventHandlerResult::OK;
+    bool match = true;
+    byte j;
 
-  for (byte i = 0;; i++) {
+    for (j = 0; j < MAX_COMBO_LENGTH; j++) {
-    combo_t combo;
+      int8_t comboKey = pgm_read_byte(&(magiccombo::combos[i].keys[j]));
 
-    combo.left_hand = pgm_read_dword(&(magic_combos[i].left_hand));
+      if (comboKey == 0)
-    combo.right_hand = pgm_read_dword(&(magic_combos[i].right_hand));
+        break;
 
-    if (combo.left_hand == 0 && combo.right_hand == 0)
+      match &= KeyboardHardware.isKeyswitchPressed(comboKey);
+      if (!match)
         break;
+    }
 
-    if (LEFTHANDSTATE.all == combo.left_hand &&
+    if (j != KeyboardHardware.pressedKeyswitchCount())
-        RIGHTHANDSTATE.all == combo.right_hand) {
+      match = false;
-      if (millis() >= end_time_) {
+
-        magicComboActions(i, combo.left_hand, combo.right_hand);
+    if (match && (millis() >= end_time_)) {
+      ComboAction action = (ComboAction) pgm_read_ptr(&(magiccombo::combos[i].action));
+
+      (*action)(i);
       end_time_ = millis() + min_interval;
     }
-      break;
-    }
   }
 
   return EventHandlerResult::OK;
@@ -75,7 +68,4 @@ void MagicCombo::legacyLoopHook(bool is_post_clear) {
 
 };
 
-__attribute__((weak)) void magicComboActions(uint8_t comboIndex, uint32_t left_hand, uint32_t right_hand) {
-}
-
 kaleidoscope::MagicCombo MagicCombo;

src/Kaleidoscope/MagicCombo.h

@@ -20,12 +20,40 @@
 
 #include <Kaleidoscope.h>
 
+#define MAX_COMBO_LENGTH 5
+
+#define USE_MAGIC_COMBOS(...)                                             \
+  namespace kaleidoscope {                                                \
+  namespace magiccombo {                                                  \
+  const kaleidoscope::MagicCombo::Combo combos[] PROGMEM = {__VA_ARGS__}; \
+                                                                          \
+  const uint8_t combos_length = sizeof(combos) / sizeof(*combos);         \
+  }                                                                       \
+  }
+
+#define _MAGICCOMBO_API_CHANGE                                            \
+  "The MagicCombo API changed in an incompatible way, you will need to\n" \
+  "upgrade.\n"                                                            \
+  "\n"                                                                    \
+  "Please see the `UPGRADING.md` document shipped with the source:\n"     \
+  " https://github.com/keyboardio/Kaleidoscope-MagicCombo/blob/master/UPGRADING.md"
+
 namespace kaleidoscope {
 
 class MagicCombo : public kaleidoscope::Plugin {
  public:
+  typedef void (*ComboAction)(uint8_t combo_index);
   typedef struct {
+    ComboAction action;
+    int8_t keys[MAX_COMBO_LENGTH + 1];
+  } Combo;
+  typedef struct combo_t {
     uint32_t left_hand, right_hand;
+
+    combo_t& operator=(combo_t &) {
+      static_assert(false, _DEPRECATE(_MAGICCOMBO_API_CHANGE));
+      return *this;
+    }
   } combo_t;
 
   MagicCombo(void) {}
@@ -45,8 +73,11 @@ class MagicCombo : public kaleidoscope::Plugin {
   static uint32_t end_time_;
 };
 
+namespace magiccombo {
+extern const MagicCombo::Combo combos[];
+extern const uint8_t combos_length;
 }
 
-void magicComboActions(uint8_t combo_index, uint32_t left_hand, uint32_t right_hand);
+}
 
 extern kaleidoscope::MagicCombo MagicCombo;