Merge pull request #1035 from gedankenexperimenter/hook/afterReportingState

Add `afterReportingState()` event handler

docs/UPGRADING.md

@@ -7,6 +7,7 @@ If any of this does not make sense to you, or you have trouble updating your .in
 
 * [Upgrade notes](#upgrade-notes)
   + [New features](#new-features)
+    - [New event handler](#new-event-handler)
     - [Event-driven main loop](#event-driven-main-loop)
     - [Keyboard state array](#keyboard-state-array)
     - [New build system](#new-build-system)
@@ -37,6 +38,10 @@ any API we've included in a release. Typically, this means that any code that us
 
 ## New features
 
+### New event handler
+
+One more `KeyEvent` handler has been added: `afterReportingState(const KeyEvent &event)`.  This handler gets called after HID reports are sent for an event, providing a point for plugins to act after an event has been fully processed by `Runtime.handleKeyEvent()`.
+
 ### Event-driven main loop
 
 Kaleidoscope's main loop has been rewritten. It now responds to key toggle-on and toggle-off events, dealing with one event at a time (and possibly more than one in a given cycle). Instead of sending a keyboard HID report at the end of every scan cycle (and letting the HID module suppress duplicates), it now only sends HID reports in response to input events.

docs/api-reference/event-handler-hooks.md

@@ -183,6 +183,14 @@ abortable. That is, if it returns a result other than `OK` it will stop the
 subsequent handlers from getting called, and if it returns `ABORT`, it will also
 stop the report from being sent.]
 
+### `afterReportingState(const KeyEvent &event)`
+
+This gets called after the HID report is sent.  This handler allows a plugin to
+react to an event, but wait until after that event has been fully processed to
+do so.  For example, the OneShot plugin releases keys that are in the "one-shot"
+state in response to key press events, but it does so after those triggering
+press events take place.
+
 ## Other events
 
 ### `onLayerChange()`

plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShot.cpp

@@ -321,6 +321,11 @@ EventHandlerResult OneShot::onKeyEvent(KeyEvent& event) {
   return EventHandlerResult::OK;
 }
 
+// ----------------------------------------------------------------------------
+EventHandlerResult OneShot::afterReportingState(const KeyEvent& event) {
+  return afterEachCycle();
+}
+
 // ----------------------------------------------------------------------------
 EventHandlerResult OneShot::afterEachCycle() {
 

plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShot.h

@@ -222,6 +222,7 @@ class OneShot : public kaleidoscope::Plugin {
 
   EventHandlerResult onNameQuery();
   EventHandlerResult onKeyEvent(KeyEvent &event);
+  EventHandlerResult afterReportingState(const KeyEvent &event);
   EventHandlerResult afterEachCycle();
 
  private:

src/kaleidoscope/Runtime.cpp

@@ -219,6 +219,11 @@ Runtime_::handleKeyEvent(KeyEvent event) {
 
   // Finally, send the new keyboard report
   sendKeyboardReport(event);
+
+  // Now that the report has been sent, let plugins act on it after the fact.
+  // This is useful for plugins that need to react to an event, but must wait
+  // until after that event is processed to do so.
+  Hooks::afterReportingState(event);
 }
 
 // ----------------------------------------------------------------------------

src/kaleidoscope/event_handlers.h

@@ -264,6 +264,19 @@ class SignatureCheckDummy {};
              (const KeyEvent &event),                                     __NL__ \
              (event), ##__VA_ARGS__)                                      __NL__ \
                                                                           __NL__ \
+   /* Called after reporting our state to the host. This is the last   */ __NL__ \
+   /* point at which a plugin can do something in response to an event */ __NL__ \
+   /* before the next event is processed, if multiple events occur in  */ __NL__ \
+   /* and are processed in a single cycle (usually due to delayed      */ __NL__ \
+   /* events or generated events).                                     */ __NL__ \
+   OPERATION(afterReportingState,                                         __NL__ \
+             1,                                                           __NL__ \
+             _CURRENT_IMPLEMENTATION,                                     __NL__ \
+             _NOT_ABORTABLE,                                              __NL__ \
+             (),(),(), /* non template */                                 __NL__ \
+             (const KeyEvent &event),                                     __NL__ \
+             (event), ##__VA_ARGS__)                                      __NL__ \
+                                                                          __NL__ \
    /* Called at the very end of a cycle, after everything's            */ __NL__ \
    /* said and done.                                                   */ __NL__ \
    OPERATION(afterEachCycle,                                              __NL__ \
@@ -351,6 +364,10 @@ class SignatureCheckDummy {};
       OP(beforeReportingState, 2)                                       __NL__ \
    END(beforeReportingState, 1, 2)                                      __NL__ \
                                                                         __NL__ \
+   START(afterReportingState, 1)                                        __NL__ \
+      OP(afterReportingState, 1)                                        __NL__ \
+   END(afterReportingState, 1)                                          __NL__ \
+                                                                        __NL__ \
    START(afterEachCycle, 1)                                             __NL__ \
       OP(afterEachCycle, 1)                                             __NL__ \
    END(afterEachCycle, 1)                                               __NL__ \

tests/issues/423/423.ino

@@ -0,0 +1,78 @@
+/* -*- mode: c++ -*-
+ * Copyright (C) 2021  Keyboard.io, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, version 3.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <Kaleidoscope.h>
+#include <Kaleidoscope-Macros.h>
+#include <Kaleidoscope-OneShot.h>
+#include <Kaleidoscope-TapDance.h>
+
+// *INDENT-OFF*
+KEYMAPS(
+    [0] = KEYMAP_STACKED
+    (
+        Key_Spacebar, Key_A, ___, ___, ___, ___, ___,
+        TD(0), OSM(LeftShift), ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___,
+
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___
+    ),
+)
+// *INDENT-ON*
+
+void tapDanceAction(uint8_t tap_dance_index,
+                    KeyAddr key_addr,
+                    uint8_t tap_count,
+                    kaleidoscope::plugin::TapDance::ActionType tap_dance_action) {
+  switch (tap_dance_index) {
+  case 0:
+    return tapDanceActionKeys(tap_count, tap_dance_action,
+                              Key_Period, M(0), LSHIFT(Key_1));
+  default:
+    break;
+  }
+}
+
+const macro_t *macroAction(uint8_t macro_id, KeyEvent &event) {
+  if (keyToggledOn(event.state)) {
+    switch (macro_id) {
+    case 0:
+      Macros.type(PSTR("abc"));
+      break;
+    default:
+      break;
+    }
+  }
+  return MACRO_NONE;
+}
+
+KALEIDOSCOPE_INIT_PLUGINS(Macros, OneShot, TapDance);
+
+void setup() {
+  Kaleidoscope.setup();
+  TapDance.time_out = 25;
+}
+
+void loop() {
+  Kaleidoscope.loop();
+}

tests/issues/423/sketch.json

@@ -0,0 +1,6 @@
+{
+  "cpu": {
+    "fqbn": "keyboardio:virtual:model01",
+    "port": ""
+  }
+}

tests/issues/423/test.ktest

@@ -0,0 +1,177 @@
+VERSION 1
+
+KEYSWITCH SPACE      0  0
+KEYSWITCH A          0  1
+KEYSWITCH TD_0       1  0
+KEYSWITCH OS_SHIFT   1  1
+
+# ==============================================================================
+NAME Back and forth
+
+RUN 4 ms
+PRESS TD_0
+RUN 1 cycle
+
+RUN 4 ms
+RELEASE TD_0
+RUN 1 cycle
+
+RUN 4 ms
+PRESS OS_SHIFT
+RUN 1 cycle
+EXPECT keyboard-report Key_Period # report: { 37 }
+EXPECT keyboard-report empty
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+
+RUN 4 ms
+RELEASE OS_SHIFT
+RUN 1 cycle
+
+RUN 4 ms
+PRESS TD_0
+RUN 1 cycle
+
+RUN 4 ms
+RELEASE TD_0
+RUN 1 cycle
+
+# ------------------------------------------------------------------------------
+# Next, we press `space`, triggering both the resolution of the TapDance key and
+# the release of the OneShot key, in that order.
+RUN 4 ms
+PRESS SPACE
+RUN 1 cycle
+# First, the TapDance key is resolved, adding `.` to the report. This event also
+# triggers the release of the OneShot key, which shouldn't happen until after
+# the `.` press is processed.
+EXPECT keyboard-report Key_LeftShift Key_Period # report: { 37 e1 }
+# Now the OneShot key is released, removing `shift` from the report.
+EXPECT keyboard-report Key_Period # report: { 37 }
+# The TapDance `.` key has been released, so its release comes next.
+EXPECT keyboard-report empty # report: { }
+# Finally, we get the report for the press of the `space` key.
+EXPECT keyboard-report Key_Spacebar # report: { 2c }
+
+RUN 4 ms
+RELEASE SPACE
+RUN 1 cycle
+EXPECT keyboard-report empty
+
+RUN 4 ms
+PRESS OS_SHIFT
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+
+RUN 4 ms
+RELEASE OS_SHIFT
+RUN 1 cycle
+
+RUN 4 ms
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift Key_A # report: { 4 e1 }
+EXPECT keyboard-report Key_A # report: { 4 }
+
+RUN 4 ms
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty
+
+RUN 5 ms
+
+# ==============================================================================
+NAME Single rollover
+
+RUN 4 ms
+PRESS TD_0
+RUN 1 cycle
+
+RUN 4 ms
+PRESS SPACE
+RUN 1 cycle
+EXPECT keyboard-report Key_Period # report: { 37 }
+EXPECT keyboard-report Key_Period Key_Spacebar # report: { 37 2c }
+
+RUN 4 ms
+PRESS OS_SHIFT
+RUN 1 cycle
+EXPECT keyboard-report Key_Period Key_Spacebar Key_LeftShift # report: { 2c 37 e1 }
+
+RUN 4 ms
+RELEASE TD_0
+RUN 1 cycle
+EXPECT keyboard-report Key_Spacebar Key_LeftShift # report: { 2c e1 }
+
+RUN 4 ms
+RELEASE SPACE
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+
+RUN 4 ms
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift Key_A # report: { 4 e1 }
+
+RUN 4 ms
+RELEASE OS_SHIFT
+RUN 1 cycle
+EXPECT keyboard-report Key_A # report: { 4 }
+
+RUN 4 ms
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty
+
+RUN 5 ms
+
+# ==============================================================================
+NAME OSM applies to whole Macro
+
+RUN 4 ms
+PRESS OS_SHIFT
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+
+RUN 4 ms
+RELEASE OS_SHIFT
+RUN 1 cycle
+
+RUN 4 ms
+PRESS TD_0
+RUN 1 cycle
+
+RUN 4 ms
+RELEASE TD_0
+RUN 1 cycle
+
+RUN 4 ms
+PRESS TD_0
+RUN 1 cycle
+
+RUN 4 ms
+RELEASE TD_0
+RUN 1 cycle
+
+# ------------------------------------------------------------------------------
+# Next, we press `space`, triggering both the resolution of the TapDance key and
+# the release of the OneShot key, in that order.
+RUN 4 ms
+PRESS SPACE
+RUN 1 cycle
+EXPECT keyboard-report Key_LeftShift Key_A # report: { 4 e1 }
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+EXPECT keyboard-report Key_LeftShift Key_B # report: { 5 e1 }
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+EXPECT keyboard-report Key_LeftShift Key_C # report: { 6 e1 }
+EXPECT keyboard-report Key_LeftShift # report: { e1 }
+EXPECT keyboard-report empty # report: { }
+EXPECT keyboard-report Key_Spacebar # report: { 2c }
+
+RUN 4 ms
+RELEASE SPACE
+RUN 1 cycle
+EXPECT keyboard-report empty
+
+
+
+RUN 5 ms