Holy refactor, Batman: full layer support (MO/DF)

Wow, what a trip this was. Layer support is now fully implemented. Other
changes here mostly revolve around the event dispatching model: more
floating state (hidden in clases wherever) has been purged, with the
reducer (now mutable, comments inline) serving, as it should, as the
sole source of truth. Thunk support has been added to our fake Redux
clone, allowing Action Creators to handle sequences of events (which is
arguably a cleaner way of handling matrix changes when not all matrix
changes should result in a new HID report - in the case of internal
keys). A whole class has been deprecated (Keymap) which only served as
another arbitor of state: instead, the MatrixScanner has been made
smarter and handles diffing internally, dispatching an Action when
needed (and allowing the reducer to parse the keymap and figure out what
key is pressed - this is the infinitely cleaner solution when layers
come into play).

boards/klardotsh/threethree_matrix_pyboard.py

@@ -17,9 +17,21 @@ def main():
     diode_orientation = DiodeOrientation.COLUMNS
 
     keymap = [
-        [KC.DF(1), KC.H, KC.RESET],
+        [
-        [KC.MO(2), KC.I, KC.ENTER],
+            [KC.MO(1), KC.H, KC.RESET],
-        [KC.CTRL, KC.SPACE, KC.SHIFT],
+            [KC.MO(2), KC.I, KC.ENTER],
+            [KC.CTRL, KC.SPACE, KC.SHIFT],
+        ],
+        [
+            [KC.TRNS, KC.B, KC.C],
+            [KC.NO, KC.D, KC.E],
+            [KC.F, KC.G, KC.H],
+        ],
+        [
+            [KC.X, KC.Y, KC.Z],
+            [KC.TRNS, KC.N, KC.O],
+            [KC.R, KC.P, KC.Q],
+        ],
     ]
 
     firmware = Firmware(

kmk/common/event_defs.py

@@ -1,8 +1,16 @@
+import logging
+
 from micropython import const
 
+from kmk.common.keycodes import Keycodes
+
 KEY_UP_EVENT = const(1)
 KEY_DOWN_EVENT = const(2)
 INIT_FIRMWARE_EVENT = const(3)
+NEW_MATRIX_EVENT = const(4)
+HID_REPORT_EVENT = const(5)
+
+logger = logging.getLogger(__name__)
 
 
 def init_firmware(keymap, row_pins, col_pins, diode_orientation):
@@ -15,19 +23,75 @@ def init_firmware(keymap, row_pins, col_pins, diode_orientation):
     }
 
 
-def key_up_event(keycode, row, col):
+def key_up_event(row, col):
     return {
         'type': KEY_UP_EVENT,
-        'keycode': keycode,
         'row': row,
         'col': col,
     }
 
 
-def key_down_event(keycode, row, col):
+def key_down_event(row, col):
     return {
         'type': KEY_DOWN_EVENT,
-        'keycode': keycode,
         'row': row,
         'col': col,
     }
+
+
+def new_matrix_event(matrix):
+    return {
+        'type': NEW_MATRIX_EVENT,
+        'matrix': matrix,
+    }
+
+
+def hid_report_event():
+    return {
+        'type': HID_REPORT_EVENT,
+    }
+
+
+def matrix_changed(new_matrix):
+    def _key_pressed(dispatch, get_state):
+        state = get_state()
+        # Temporarily preserve a reference to the old event
+        # We do fake Redux around here because microcontrollers
+        # aren't exactly RAM or CPU powerhouses - the state does
+        # mutate in place. Unfortunately this makes reasoning
+        # about code a bit messier and really hurts one of the
+        # selling points of Redux. Former development versions
+        # of KMK created new InternalState copies every single
+        # time the state changed, but it was sometimes slow.
+        old_matrix = state.matrix
+        old_keys_pressed = state.keys_pressed
+
+        dispatch(new_matrix_event(new_matrix))
+
+        with get_state() as new_state:
+            for ridx, row in enumerate(new_state.matrix):
+                for cidx, col in enumerate(row):
+                    if col != old_matrix[ridx][cidx]:
+                        if col:
+                            dispatch(key_down_event(
+                                row=ridx,
+                                col=cidx,
+                            ))
+                        else:
+                            dispatch(key_up_event(
+                                row=ridx,
+                                col=cidx,
+                            ))
+
+        with get_state() as new_state:
+            if old_keys_pressed != new_state.keys_pressed:
+                dispatch(hid_report_event())
+
+            if Keycodes.KMK.KC_RESET in new_state.keys_pressed:
+                try:
+                    import machine
+                    machine.bootloader()
+                except ImportError:
+                    logger.warning('Tried to reset to bootloader, but not supported on this chip?')
+
+    return _key_pressed

kmk/common/internal_keycodes.py

@@ -4,53 +4,41 @@ from kmk.common.event_defs import KEY_DOWN_EVENT, KEY_UP_EVENT
 from kmk.common.keycodes import Keycodes
 
 
-def process(state, action, logger=None):
+def process_internal_key_event(state, action, changed_key, logger=None):
-    if action['keycode'].code < 1000:
-        return state
-
     if logger is None:
         logger = logging.getLogger(__name__)
 
-    logger.warning(action['keycode'])
+    if changed_key.code == Keycodes.Layers._KC_DF:
-    if action['keycode'] == Keycodes.KMK.KC_RESET:
+        return df(state, action, changed_key, logger=logger)
-        return reset(state, action, logger=logger)
+    elif changed_key.code == Keycodes.Layers._KC_MO:
-    elif action['keycode'].code == Keycodes.Layers._KC_DF:
+        return mo(state, action, changed_key, logger=logger)
-        return df(state, action, logger=logger)
+    elif changed_key.code == Keycodes.Layers.KC_TILDE:
-    elif action['keycode'].code == Keycodes.Layers._KC_MO:
-        return tilde(state, action, logger=logger)
-    elif action['keycode'].code == Keycodes.Layers.KC_TILDE:
         pass
     else:
         return state
 
 
-def tilde(state, action, logger):
+def tilde(state, action, changed_key, logger):
     # TODO Actually process keycodes
     return state
 
 
-def reset(state, action, logger):
+def df(state, action, changed_key, logger):
-    logger.debug('Rebooting to bootloader')
-    import machine
-    machine.bootloader()
-
-
-def df(state, action, logger):
     """Switches the default layer"""
-    state.active_layers[0] = action['keycode'].layer
+    state.active_layers[0] = changed_key.layer
 
     return state
 
 
-def mo(state, action, logger):
+def mo(state, action, changed_key, logger):
     """Momentarily activates layer, switches off when you let go"""
     if action['type'] == KEY_UP_EVENT:
         state.active_layers = [
             layer for layer in state.active_layers
-            if layer != action['keycode'].layer
+            if layer != changed_key.layer
         ]
     elif action['type'] == KEY_DOWN_EVENT:
-        state.active_layers.append(action['keycode'].layer)
+        state.active_layers.append(changed_key.layer)
 
     return state
 

kmk/common/internal_state.py

@@ -2,9 +2,11 @@ import logging
 import sys
 
 from kmk.common.consts import DiodeOrientation
-from kmk.common.event_defs import (INIT_FIRMWARE_EVENT, KEY_DOWN_EVENT,
+from kmk.common.event_defs import (HID_REPORT_EVENT, INIT_FIRMWARE_EVENT,
-                                   KEY_UP_EVENT)
+                                   KEY_DOWN_EVENT, KEY_UP_EVENT,
-from kmk.common.internal_keycodes import process as process_internal
+                                   NEW_MATRIX_EVENT)
+from kmk.common.internal_keycodes import process_internal_key_event
+from kmk.common.keycodes import FIRST_KMK_INTERNAL_KEYCODE, Keycodes
 
 
 class ReduxStore:
@@ -16,9 +18,15 @@ class ReduxStore:
         self.callbacks = []
 
     def dispatch(self, action):
-        self.logger.debug('Dispatching action: {}'.format(action))
+        if callable(action):
-        self.state = self.reducer(self.state, action)
+            self.logger.debug('Received thunk')
-        self.logger.debug('Dispatching complete: {}'.format(action))
+            action(self.dispatch, self.get_state)
+            self.logger.debug('Finished thunk')
+            return None
+
+        self.logger.debug('Dispatching action: Type {} >> {}'.format(action['type'], action))
+        self.state = self.reducer(self.state, action, logger=self.logger)
+        self.logger.debug('Dispatching complete: Type {}'.format(action['type']))
 
         self.logger.debug('New state: {}'.format(self.state))
 
@@ -55,6 +63,12 @@ class InternalState:
     def __init__(self, preserve_intermediate_states=False):
         self.preserve_intermediate_states = preserve_intermediate_states
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        pass
+
     def to_dict(self, verbose=False):
         ret = {
             'keys_pressed': self.keys_pressed,
@@ -86,6 +100,21 @@ class InternalState:
         return self
 
 
+def find_key_in_map(state, row, col):
+    # Later-added layers have priority. Sift through the layers
+    # in reverse order until we find a valid keycode object
+    for layer in reversed(state.active_layers):
+        layer_key = state.keymap[layer][row][col]
+
+        if not layer_key or layer_key == Keycodes.KMK.KC_TRNS:
+            continue
+
+        if layer_key == Keycodes.KMK.KC_NO:
+            break
+
+        return layer_key
+
+
 def kmk_reducer(state=None, action=None, logger=None):
     if state is None:
         state = InternalState()
@@ -99,41 +128,52 @@ def kmk_reducer(state=None, action=None, logger=None):
 
         return state
 
-    if action['type'] == KEY_UP_EVENT:
+    if action['type'] == NEW_MATRIX_EVENT:
-        newstate = state.update(
+        return state.update(
-            keys_pressed=frozenset(
+            matrix=action['matrix'],
-                key for key in state.keys_pressed if key != action['keycode']
-            ),
-            matrix=[
-                r if ridx != action['row'] else [
-                    c if cidx != action['col'] else False
-                    for cidx, c in enumerate(r)
-                ]
-                for ridx, r in enumerate(state.matrix)
-            ],
         )
 
-        if action['keycode'].code >= 1000:
+    if action['type'] == KEY_UP_EVENT:
-            return process_internal(newstate, action, logger=logger)
+        row = action['row']
+        col = action['col']
+
+        changed_key = find_key_in_map(state, row, col)
+
+        logger.debug('Detected change to key: {}'.format(changed_key))
+
+        if not changed_key:
+            return state
+
+        newstate = state.update(
+            keys_pressed=frozenset(
+                key for key in state.keys_pressed if key != changed_key
+            ),
+        )
+
+        if changed_key.code >= FIRST_KMK_INTERNAL_KEYCODE:
+            return process_internal_key_event(newstate, action, changed_key, logger=logger)
 
         return newstate
 
     if action['type'] == KEY_DOWN_EVENT:
+        row = action['row']
+        col = action['col']
+
+        changed_key = find_key_in_map(state, row, col)
+
+        logger.debug('Detected change to key: {}'.format(changed_key))
+
+        if not changed_key:
+            return state
+
         newstate = state.update(
             keys_pressed=(
-                state.keys_pressed | {action['keycode']}
+                state.keys_pressed | {changed_key}
             ),
-            matrix=[
-                r if ridx != action['row'] else [
-                    c if cidx != action['col'] else True
-                    for cidx, c in enumerate(r)
-                ]
-                for ridx, r in enumerate(state.matrix)
-            ],
         )
 
-        if action['keycode'].code >= 1000:
+        if changed_key.code >= FIRST_KMK_INTERNAL_KEYCODE:
-            return process_internal(newstate, action, logger=logger)
+            return process_internal_key_event(newstate, action, changed_key, logger=logger)
 
         return newstate
 
@@ -148,3 +188,14 @@ def kmk_reducer(state=None, action=None, logger=None):
                 for r in action['row_pins']
             ],
         )
+
+    # HID events are non-mutating, used exclusively for listeners to know
+    # they should be doing things. This could/should arguably be folded back
+    # into KEY_UP_EVENT and KEY_DOWN_EVENT, but for now it's nice to separate
+    # this out for debugging's sake.
+    if action['type'] == HID_REPORT_EVENT:
+        return state
+
+    # On unhandled events, log and do not mutate state
+    logger.warning('Unhandled event! Returning state unmodified.')
+    return state

kmk/common/keycodes.py

@@ -8,6 +8,8 @@ except ImportError:
 from kmk.common.types import AttrDict
 from kmk.common.util import flatten_dict
 
+FIRST_KMK_INTERNAL_KEYCODE = 1000
+
 Keycode = namedtuple('Keycode', ('code', 'is_modifier'))
 LayerKeycode = namedtuple('LayerKeycode', ('code', 'layer'))
 
@@ -321,6 +323,8 @@ class Keycodes(KeycodeCategory):
         KC_RSPC = Keycode(1004, False)
         KC_LEAD = Keycode(1005, False)
         KC_LOCK = Keycode(1006, False)
+        KC_NO = Keycode(1100, False)
+        KC_TRNS = Keycode(1101, False)
 
     class Layers(KeycodeCategory):
         _KC_DF = 1050

kmk/common/keymap.py

@@ -1,25 +0,0 @@
-from kmk.common.event_defs import key_down_event, key_up_event
-
-
-class Keymap:
-    def __init__(self, map):
-        self.map = map
-
-    def parse(self, matrix, store):
-        state = store.get_state()
-
-        for ridx, row in enumerate(matrix):
-            for cidx, col in enumerate(row):
-                if col != state.matrix[ridx][cidx]:
-                    if col:
-                        store.dispatch(key_down_event(
-                            row=ridx,
-                            col=cidx,
-                            keycode=self.map[ridx][cidx],
-                        ))
-                    else:
-                        store.dispatch(key_up_event(
-                            row=ridx,
-                            col=cidx,
-                            keycode=self.map[ridx][cidx],
-                        ))

kmk/firmware.py

@@ -2,7 +2,6 @@ import logging
 
 from kmk.common.event_defs import init_firmware
 from kmk.common.internal_state import ReduxStore, kmk_reducer
-from kmk.common.keymap import Keymap
 
 try:
     from kmk.circuitpython.matrix import MatrixScanner
@@ -40,9 +39,6 @@ class Firmware:
         ))
 
     def _subscription(self, state, action):
-        if self.cached_state is None or self.cached_state.keymap != state.keymap:
-            self.keymap = Keymap(state.keymap)
-
         if self.cached_state is None or any(
             getattr(self.cached_state, k) != getattr(state, k)
             for k in state.__dict__.keys()
@@ -55,4 +51,8 @@ class Firmware:
 
     def go(self):
         while True:
-            self.keymap.parse(self.matrix.raw_scan(), store=self.store)
+            state = self.store.get_state()
+            update = self.matrix.scan_for_changes(state.matrix)
+
+            if update:
+                self.store.dispatch(update)

kmk/micropython/matrix.py

@@ -2,6 +2,7 @@ import machine
 
 from kmk.common.abstract.matrix_scanner import AbstractMatrixScanner
 from kmk.common.consts import DiodeOrientation
+from kmk.common.event_defs import matrix_changed
 
 
 class MatrixScanner(AbstractMatrixScanner):
@@ -52,3 +53,17 @@ class MatrixScanner(AbstractMatrixScanner):
             opin.value(0)
 
         return self._normalize_matrix(matrix)
+
+    def scan_for_changes(self, old_matrix):
+        matrix = self.raw_scan()
+
+        if any(
+            any(
+                col != old_matrix[ridx][cidx]
+                for cidx, col in enumerate(row)
+            )
+            for ridx, row in enumerate(matrix)
+        ):
+            return matrix_changed(matrix)
+
+        return None  # The default, but for explicitness

kmk/micropython/pyb_hid.py

@@ -3,8 +3,9 @@ import string
 
 from pyb import USB_HID, delay
 
-from kmk.common.event_defs import KEY_DOWN_EVENT, KEY_UP_EVENT
+from kmk.common.event_defs import HID_REPORT_EVENT
-from kmk.common.keycodes import Keycodes, char_lookup
+from kmk.common.keycodes import (FIRST_KMK_INTERNAL_KEYCODE, Keycodes,
+                                 char_lookup)
 
 
 class HIDHelper:
@@ -48,26 +49,19 @@ class HIDHelper:
         self.clear_all()
 
     def _subscription(self, state, action):
-        if action['type'] == KEY_DOWN_EVENT:
+        if action['type'] == HID_REPORT_EVENT:
-            if action['keycode'].code >= 1000:
+            self.clear_all()
-                return
 
-            if action['keycode'].is_modifier:
+            for key in state.keys_pressed:
-                self.add_modifier(action['keycode'])
+                if key.code >= FIRST_KMK_INTERNAL_KEYCODE:
-                self.send()
+                    continue
-            else:
-                self.add_key(action['keycode'])
-                self.send()
-        elif action['type'] == KEY_UP_EVENT:
-            if action['keycode'].code >= 1000:
-                return
 
-            if action['keycode'].is_modifier:
+                if key.is_modifier:
-                self.remove_modifier(action['keycode'])
+                    self.add_modifier(key)
-                self.send()
+                else:
-            else:
+                    self.add_key(key)
-                self.remove_key(action['keycode'])
+
-                self.send()
+            self.send()
 
     def send(self):
         self.logger.debug('Sending HID report: {}'.format(self._evt))