More comment cleanups and clarifications.

Rename some variables to clarify use

firmware/asdf/src/Arch/asdf_arch_atmega328p.c

@@ -386,7 +386,7 @@ void asdf_arch_out1_open_lo_set(uint8_t value)
 //
 // NOTES: OUT2 is inverted by the 7404 buffer, so clearing the bit sets the output high.  OUT2
 // cannot be high impedance.
-/
+//
 // SCOPE: public
 //
 // COMPLEXITY: 2

firmware/asdf/src/asdf_keymaps.c

@@ -28,8 +28,8 @@
 #include "asdf_keymaps.h"
 #include "asdf_keymap_defs.h"
 
-// ASDF_KEYMAP_DECLARATIONS is defined in asdf_keymap_defs.h, agregated from all
-// the included keymap definition files.
+// ASDF_KEYMAP_DECLARATIONS is defined in asdf_keymap_defs.h, aggregated from
+// all the included keymap definition files.
 //
 // This is a terrible practice, but defining the declarations as a macro permits
 // the keymap definitions to be incorporated in a fairly modular fashion, using
@@ -40,14 +40,26 @@
 // constexpr in C++ may be an alternative option as well.
 ASDF_KEYMAP_DECLARATIONS;
 
+// The keymap arrays organized as follows:
+// * Each keymap matrix is a NUM_ROWS x NUM_COLS mapping of key to code for a given modifier.
+// * Each keymap contains a set of keymap matrices, one for each unique
+//   combination of modifier keys.
+// * All the keymaps (NUM_KEYMAPS) are gathered in the keymap_matrixp[] array.
 static keycode_matrix_t const *keymap_matrix[ASDF_NUM_KEYMAPS][ASDF_MOD_NUM_MODIFIERS] =
   ASDF_KEYMAP_DEFS;
 
 
+// Each keymap (representing a keyboard configuration) has an associated set of
+// keyboard initializers that set up the I/O and LED lines for the keyboard, and
+// other configuration unique to the keyboard defined by the map. This builds
+// the keymap initializer array from the initializer definitions in the keymap
+// definitino files.
 static const asdf_virtual_initializer_t keymap_initializer_list[ASDF_NUM_KEYMAPS]
                                                                [ASDF_KEYMAP_INITIALIZER_LENGTH] =
                                                                  ASDF_KEYMAP_INITIALIZERS;
 
+// Index of the currently active keymap, initialized to zero in the init
+// routine.
 static uint8_t keymap_index;
 
 

firmware/asdf/src/asdf_keymaps.h

@@ -28,55 +28,15 @@
 #define ASDF_KEYMAPS_H
 
 
-
+// Define the bit position of each keymap DIP switch. The DIP switch values at
+// each bit position can be used to select the current keymap. This requires the
+// DIP switches to be mapped to the asdf_keymaps_select_X_set() and
+// asdf_keymaps_select_X_clear() functions in each keymap.
 #define ASDF_KEYMAP_BIT_0 1
 #define ASDF_KEYMAP_BIT_1 2
 #define ASDF_KEYMAP_BIT_2 4
 #define ASDF_KEYMAP_BIT_3 8
 
-typedef enum {
-             CLEAR_OUT1,
-             SET_OUT1,
-             CLEAR_OUT2,
-             SET_OUT2,
-             CLEAR_OUT3,
-             SET_OUT3,
-             CLEAR_OC1,
-             SET_OC1,
-             CLEAR_OC3,
-             SET_OC3,
-             PULSE_OUT1,
-             PULSE_OUT2,
-             PULSE_OUT3,
-             TOGGLE_OUT1,
-             TOGGLE_OUT2,
-             TOGGLE_OUT3,
-             ON_VLED1,
-             OFF_VLED1,
-             ON_VLED2,
-             OFF_VLED2,
-             ON_VLED3,
-             OFF_VLED3,
-             SET_CAPS_LED1,
-             SET_CAPS_LED2,
-             SET_CAPS_LED3,
-             SET_SHIFTLOCK_LED1,
-             SET_SHIFTLOCK_LED2,
-             SET_SHIFTLOCK_LED3,
-             SET_VLED1_LED1,
-             SET_VLED2_LED1,
-             SET_VLED3_LED1,
-             SET_VLED1_LED2,
-             SET_VLED2_LED2,
-             SET_VLED3_LED2,
-             SET_VLED1_LED3,
-             SET_VLED2_LED3,
-             SET_VLED3_LED3,
-
-} keymap_init_t;
-
-
-
 // PROCEDURE: asdf_keymaps_select_keymap
 // INPUTS: (uint8_t) index - index of the keymap number to select
 // OUTPUTS: none

firmware/asdf/src/asdf_modifiers.c

@@ -30,12 +30,18 @@
 #include "asdf_virtual.h"
 #include "asdf_arch.h"
 
+// Stores the state of the SHIFT and SHIFTLOCK keys
 static shift_state_t shift_state;
+
+// Stores the state of the CAPS and CAPSLOCK keys
 static caps_state_t caps_state;
+
+// Stores the state of the CTRL keys.
 static ctrl_state_t ctrl_state;
 
-static const modifier_index_t modifier_indices[] = {
-
+// The active modifier map depends on the current state of the modifier
+// variables. The map encodes the order or precedence of the various modifiers.
+static const modifier_index_t modifier_mapping[] = {
   MOD_PLAIN_MAP, // 0x00: no modifiers
   MOD_SHIFT_MAP, // 0x01: only SHIFT active
   MOD_CAPS_MAP,  // 0x02: only CAPS active
@@ -208,28 +214,29 @@ void asdf_modifiers_init(void)
 
 // PROCEDURE: asdf_modifier_index
 // INPUTS: none
-// OUTPUTS: returns uint8_t index into key map, based on modifier key status
+// OUTPUTS: returns uint8_t index into key map, based on which modifiers are active.
 //
 // DESCRIPTION: See OUTPUTS
 //
 // SIDE EFFECTS: none
 //
-// COMPLEXITY: 1
+// COMPLEXITY: 4
 //
 modifier_index_t asdf_modifier_index(void)
 {
-  uint8_t modifier = 0;
+  uint8_t active_modifiers = 0;
+
   if (shift_state) {
-    modifier |= ASDF_MODIFIERS_SHIFT_MASK;
+    active_modifiers |= ASDF_MODIFIERS_SHIFT_MASK;
   }
   if (caps_state) {
-    modifier |= ASDF_MODIFIERS_CAPS_MASK;
+    active_modifiers |= ASDF_MODIFIERS_CAPS_MASK;
   }
   if (ctrl_state) {
-    modifier |= ASDF_MODIFIERS_CTRL_MASK;
+    active_modifiers |= ASDF_MODIFIERS_CTRL_MASK;
   }
 
-  return modifier_indices[modifier];
+  return modifier_mapping[active_modifiers];
 }
 
 

firmware/asdf/src/asdf_modifiers.h

@@ -23,6 +23,18 @@
 #if !defined(ASDF_MODIFIERS_H)
 #define ASDF_MODIFIERS_H
 
+// The active modifiers are used to build an index into a map that determinds
+// which modifier map is selected.  The following define the bit position for each modifier.
+// For example, if SHIFT and CTRL are active, the modifier index would be
+//
+// (1 << ASDF_MODIFIERS_SHIFT_POS) | (1 << ASDF_MODIFIERS_CAPS_POS)
+// = (1 << 0) | (1 << 1)
+// = (1 | 2)
+// = 3
+//
+// So, if CTRL+SHIFT is a special modifier, then modifier_mapping[3] will point
+// to the CTRL+SHIFT map. Alternatively, if CTRL+SHIFT is the same as just CTRL,
+// then modifier_mapping[3] will point to the CTRL map.
 #define ASDF_MODIFIERS_SHIFT_POS 0
 #define ASDF_MODIFIERS_CAPS_POS 1
 #define ASDF_MODIFIERS_CTRL_POS 2
@@ -31,6 +43,7 @@
 #define ASDF_MODIFIERS_CAPS_MASK (1 << ASDF_MODIFIERS_CAPS_POS)
 #define ASDF_MODIFIERS_CTRL_MASK (1 << ASDF_MODIFIERS_CTRL_POS)
 
+// Define the legal SHIFT and SHIFTLOCK states
 typedef enum {
   SHIFT_OFF_ST = 0,
   SHIFT_ON_ST = 1,
@@ -39,6 +52,7 @@ typedef enum {
 
 } shift_state_t;
 
+// Define the legal CAPS and CAPSLOCK states
 typedef enum {
   CAPS_OFF_ST = 0,
   CAPS_ON_ST = 1,
@@ -46,8 +60,13 @@ typedef enum {
   CAPS_BOTH_ST = 3 // Never explicitly set. CAPS and CAPSLOCK together.
 } caps_state_t;
 
+// Define the legal CTRL states
 typedef enum { CTRL_OFF_ST = 0, CTRL_ON_ST = 1 } ctrl_state_t;
 
+// Define the legal modifier mappings. In this case, we define 4 maps, for PLAIN
+// (no modifier), SHIFT, CAPS, and CTRL. When combinations of these modifiers
+// are active, then the precedence is determined by the modifier_mapping[]
+// array.
 typedef enum {
   MOD_PLAIN_MAP = 0,
   MOD_SHIFT_MAP,