Comment and nomenclature cleanups

- fix and clarify various function header comments
- use consistent nomenclature for virtual layer.

firmware/asdf/src/asdf.c

@@ -132,7 +132,17 @@ asdf_keycode_t asdf_lookup_keycode(uint8_t row, uint8_t col)
 //
 // SCOPE: private
 //
-// COMPLEXITY: 1
+// NOTES: The switch() could be implemented as an array of function pointers,
+// essentially a jump table. However, the switch statement will also be
+// implemented as a jump table, and may be more efficiently implemented as a
+// code jump table than an array of pointers stored in flash that would require
+// an additional flash-read operation. Also, the switch jumptable will make more
+// efficient use of space than a sparse array of function pointers. The
+// reduction in cyclometric complexity by using an array is a technicality,
+// since the elements must still be entered as lines of code in some part of the
+// program.  Here, they are arranged in the place they are used.
+//
+// COMPLEXITY: 1 (+18 for the switch)
 //
 static void asdf_activate_action(action_t keycode)
 {
@@ -238,7 +248,11 @@ static void asdf_activate_action(action_t keycode)
 //
 // SCOPE: private
 //
-// COMPLEXITY: 1
+// NOTES: See NOTE for asdf_activate_action(). The Virtual Outputs have
+// activate() functions, but no deactivate() functions, and are handled by the
+// default case.
+//
+// COMPLEXITY: 1 (+9 for the switch, see note)
 //
 static void asdf_deactivate_action(action_t keycode)
 {
@@ -281,17 +295,6 @@ static void asdf_deactivate_action(action_t keycode)
       break;
     }
     case ACTION_NOTHING:
-    case ACTION_HERE_IS:
-    case ACTION_FN_1:
-    case ACTION_FN_2:
-    case ACTION_FN_3:
-    case ACTION_FN_4:
-    case ACTION_FN_5:
-    case ACTION_FN_6:
-    case ACTION_FN_7:
-    case ACTION_FN_8:
-    case ACTION_FN_9:
-    case ACTION_FN_10:
     default: break;
   }
 }
@@ -310,7 +313,7 @@ static void asdf_deactivate_action(action_t keycode)
 //
 // SCOPE: private
 //
-// COMPLEXITY: 2
+// COMPLEXITY: 3
 //
 static void asdf_activate_key(asdf_keycode_t keycode)
 {
@@ -342,7 +345,7 @@ static void asdf_activate_key(asdf_keycode_t keycode)
 //
 // SCOPE: private
 //
-// COMPLEXITY: 2
+// COMPLEXITY: 3
 //
 static void asdf_deactivate_key(asdf_keycode_t keycode)
 {

firmware/asdf/src/asdf_keymaps.c

@@ -35,17 +35,18 @@
 // the keymap definitions to be incorporated in a fairly modular fashion, using
 // the limited capabilities of the C preprocessor. One alternative is to use a
 // program (in C or at the expense of additional build dependencies, in python
-// or bash) to generate the keymap definitions and declarations. But then the
-// keymap declarations would not be private. constexpr in C++ may be an
-// alternative option as well.
+// or bash) to generate the keymap definitions and declarations from a
+// definition file. But then the keymap declarations would not be private.
+// constexpr in C++ may be an alternative option as well.
 ASDF_KEYMAP_DECLARATIONS;
 
 static keycode_matrix_t const *keymap_matrix[ASDF_NUM_KEYMAPS][ASDF_MOD_NUM_MODIFIERS] =
   ASDF_KEYMAP_DEFS;
 
 
-static const asdf_virtual_initializer_t keymap_initializer_list[ASDF_NUM_KEYMAPS][ASDF_KEYMAP_INITIALIZER_LENGTH] =
-  ASDF_KEYMAP_INITIALIZERS;
+static const asdf_virtual_initializer_t keymap_initializer_list[ASDF_NUM_KEYMAPS]
+                                                               [ASDF_KEYMAP_INITIALIZER_LENGTH] =
+                                                                 ASDF_KEYMAP_INITIALIZERS;
 
 static uint8_t keymap_index;
 
@@ -55,7 +56,8 @@ static uint8_t keymap_index;
 // OUTPUTS: none
 //
 // DESCRIPTION: accepts a index value. If the requested keymap index is valid,
-// then assign the value to the global (to the module) keymap_index variable.
+// then assign the value to the global (to the module) keymap_index variable,
+// and initialize the virtual outputs for the selected keymap.
 //
 // SIDE EFFECTS: May change the module-global keymap_index variable.
 //
@@ -77,8 +79,7 @@ void asdf_keymaps_select_keymap(uint8_t index)
 // INPUTS: none
 // OUTPUTS: none
 //
-// DESCRIPTION: Assigns the keymaps to the indices specified by the modifier
-// index, to avoid hard-coding constant index values.
+// DESCRIPTION: Selects the base keymap.
 //
 // SIDE EFFECTS: builds up the private map table.
 //

firmware/asdf/src/asdf_physical.c

@@ -5,7 +5,7 @@
 //
 // asdf_physical.c
 //
-// This file contains code to manage physical outputs and serves as an API
+// This file contains code to manage physical resources and serves as an API
 // between the virtual layer and the architecture specific code.
 //
 // Copyright 2019 David Fenyes
@@ -65,7 +65,7 @@ static struct {
 // INPUTS: (uint8_t) value
 // OUTPUTS: none
 //
-// DESCRIPTION: If the physical output is valid, set to high if value is true, low
+// DESCRIPTION: If the physical resource is valid, set to high if value is true, low
 // if false.
 //
 // SIDE EFFECTS: see above
@@ -84,11 +84,11 @@ void asdf_physical_set(asdf_physical_dev_t physical_out, uint8_t value)
 
 
 // PROCEDURE: asdf_physical_assert
-// INPUTS: (asdf_physical_dev_t) physical_out: which physical output to set or clear
+// INPUTS: (asdf_physical_dev_t) physical_out: which physical resource to set or clear
 // INPUTS: none
 // OUTPUTS: none
 //
-// DESCRIPTION: Assert the value of the physical output shadow register on the output.
+// DESCRIPTION: Assert the value of the physical resource shadow register on the output.
 //
 // SIDE EFFECTS: see above
 //
@@ -105,11 +105,11 @@ void asdf_physical_assert(asdf_physical_dev_t physical_out)
 }
 
 // PROCEDURE: asdf_physical_toggle
-// INPUTS: (asdf_physical_dev_t) physical_out: which physical output to toggle
+// INPUTS: (asdf_physical_dev_t) physical_out: which physical resource to toggle
 // INPUTS: none
 // OUTPUTS: none
 //
-// DESCRIPTION: Toggle the value of the physical output.
+// DESCRIPTION: Toggle the value of the physical resource.
 //
 // SIDE EFFECTS: see above
 //
@@ -161,7 +161,7 @@ static uint8_t valid_physical_device(asdf_physical_dev_t device)
 //
 // SCOPE: public
 //
-// COMPLEXITY: 2
+// COMPLEXITY: 3
 //
 uint8_t physical_device_is_available(asdf_physical_dev_t device)
 {
@@ -176,11 +176,11 @@ uint8_t physical_device_is_available(asdf_physical_dev_t device)
 }
 
 // PROCEDURE: asdf_physical_next_device
-// INPUTS: (asdf_physical_dev_t) device - the current physical device attached
-// to the virtual device being operated on
+// INPUTS: (asdf_physical_dev_t) device - the current physical resource attached
+// to the virtual output being operated on
 //
-// OUTPUTS: (asdf_physical_dev_t) returns the next physical device assigned to
-// the virtual device.
+// OUTPUTS: (asdf_physical_dev_t) returns the next physical resource assigned to
+// the virtual output.
 //
 // DESCRIPTION: See above.
 //
@@ -200,7 +200,7 @@ asdf_physical_dev_t asdf_physical_next_device(asdf_physical_dev_t device)
 // PROCEDURE: asdf_physical_allocate
 
 // INPUTS: (asdf_physical_out_t) physical_out - the desired physical resource to allocate.
-//         (asdf_physical_out_t) tail - the list of physical devices to tack on
+//         (asdf_physical_out_t) tail - the list of physical resources to tack on
 //         to the requested resource, if available.
 //
 // OUTPUTS: (asdf_physical_out_t) returns TRUE if the allocation is succesful,
@@ -228,11 +228,11 @@ uint8_t asdf_physical_allocate(asdf_physical_dev_t physical_out, asdf_physical_d
     // remove from available list:
     physical_device_table[predecessor].next = physical_device_table[physical_out].next;
 
-    // tack the tail on to the physical device
+    // tack the tail on to the physical resource
 
     physical_device_table[physical_out].next = tail;
 
-    // The physical device shadow value is set here. The shadow values are
+    // The physical resource shadow value is set here. The shadow values are
     // asserted to the outputs only after all the assignments have been
     // performed.
     physical_device_table[physical_out].shadow = initial_value;
@@ -246,7 +246,7 @@ uint8_t asdf_physical_allocate(asdf_physical_dev_t physical_out, asdf_physical_d
 // INPUTS: none
 // OUTPUTS: none
 //
-// DESCRIPTION: Initialize physical device table
+// DESCRIPTION: Initialize physical resource table
 //
 // SIDE EFFECTS: see above
 //

firmware/asdf/src/asdf_physical.h

@@ -49,22 +49,22 @@ typedef enum {
 // INPUTS: (asdf_physical_dev_t) physical_out: which real output to set or clear
 // INPUTS: (uint8_t) value
 // OUTPUTS: none
-// DESCRIPTION: If the physical output is valid, set to high if value is true, low
+// DESCRIPTION: If the physical resource is valid, set to high if value is true, low
 // if false.
 void asdf_physical_set(asdf_physical_dev_t physical_out, uint8_t value);
 
 // PROCEDURE: asdf_physical_assert
-// INPUTS: (asdf_physical_dev_t) physical_out: which physical output to set or clear
+// INPUTS: (asdf_physical_dev_t) physical_out: which physical resource to set or clear
 // INPUTS: none
 // OUTPUTS: none
-// DESCRIPTION: Assert the value of the physical output shadow register on the output.
+// DESCRIPTION: Assert the value of the physical resource shadow register on the output.
 void asdf_physical_assert(asdf_physical_dev_t physical_out);
 
 // PROCEDURE: asdf_physical_toggle
-// INPUTS: (asdf_physical_dev_t) physical_out: which physical output to toggle
+// INPUTS: (asdf_physical_dev_t) physical_out: which physical resource to toggle
 // INPUTS: none
 // OUTPUTS: none
-// DESCRIPTION: Toggle the value of the physical output.
+// DESCRIPTION: Toggle the value of the physical resource.
 void asdf_physical_toggle(asdf_physical_dev_t physical_out);
 
 // PROCEDURE: physical_device_is_available
@@ -78,15 +78,15 @@ void asdf_physical_toggle(asdf_physical_dev_t physical_out);
 uint8_t physical_device_is_available(asdf_physical_dev_t device);
 
 // PROCEDURE: asdf_physical_next_device
-// INPUTS: (asdf_physical_dev_t) device - the current physical device attached
-// to the virtual device being operated on
-// OUTPUTS: (asdf_physical_dev_t) returns the next physical device assigned to
-// the virtual device.
+// INPUTS: (asdf_physical_dev_t) device - the current physical resource attached
+// to the virtual output being operated on
+// OUTPUTS: (asdf_physical_dev_t) returns the next physical resource assigned to
+// the virtual output.
 asdf_physical_dev_t asdf_physical_next_device(asdf_physical_dev_t device);
 
 // PROCEDURE: asdf_physical_allocate
 // INPUTS: (asdf_physical_out_t) physical_out - the desired physical resource to allocate.
-//         (asdf_physical_out_t) tail - the list of physical devices to tack on
+//         (asdf_physical_out_t) tail - the list of physical resources to tack on
 //         to the requested resource, if available.
 // OUTPUTS: (asdf_physical_out_t) returns TRUE if the allocation is succesful,
 //          FALSE (0) otherwise.
@@ -99,7 +99,7 @@ uint8_t asdf_physical_allocate(asdf_physical_dev_t physical_out, asdf_physical_d
 // PROCEDURE: asdf_physical_init
 // INPUTS: none
 // OUTPUTS: none
-// DESCRIPTION: Initialize physical device table
+// DESCRIPTION: Initialize physical resource table
 void asdf_physical_init(void);
 
 

firmware/asdf/src/asdf_virtual.c

@@ -34,13 +34,15 @@
 #include "asdf_config.h"
 #include "asdf_arch.h"
 
-// virtual_device_table[] contains all the virtual outputs. An asdf_virtual_output_t
-// value is used to identify each element. Each element is a virtual device,
-// containing an asdf_virtual_physical_dev_t value indicating the physical device (if
-// any) assigned to the virtual device.
+// virtual_device_table[] contains all the virtual outputs. An
+// asdf_virtual_output_t value is used to identify each element. Each element is
+// a virtual output, containing an asdf_virtual_physical_dev_t value indicating
+// the first in the list of physical resources (if any) assigned to the virtual
+// device. Each element also contains a function that can be applied to the
+// physical resources when the virtual output is activated by a keypress.
 static struct {
-  asdf_physical_dev_t physical_device; // Each virtual device points to a linked
-                                       // list of any number of physical devices.
+  asdf_physical_dev_t physical_device; // Each virtual output points to a linked
+                                       // list of any number of physical resources.
   asdf_virtual_function_t function;
 } virtual_device_table[ASDF_VIRTUAL_NUM_RESOURCES];
 
@@ -49,12 +51,12 @@ static struct {
 // INPUTS: (asdf_virtual_function_t) function: what function to apply to the virtual output
 // OUTPUTS: none
 //
-// DESCRIPTION: for each physical output mapped to the virtual output, apply the
+// DESCRIPTION: for each physical resource mapped to the virtual output, apply the
 // specified function.
 //
 // SIDE EFFECTS: see above
 //
-// NOTES: The virtual output points to a linked list of physical devices.
+// NOTES: The virtual output points to a linked list of physical resources.
 //
 // SCOPE: public
 //
@@ -95,15 +97,15 @@ void asdf_virtual_action(asdf_virtual_dev_t virtual_out, asdf_virtual_function_t
 }
 
 // PROCEDURE: asdf_virtual_activate
-// INPUTS: asdf_virtual_dev_t: The virtual device to be activated
+// INPUTS: asdf_virtual_dev_t: The virtual output to be activated
 // OUTPUTS: none
 //
-// DESCRIPTION: for each physical output mapped to the virtual output, apply the
+// DESCRIPTION: for each physical resource mapped to the virtual output, apply the
 // function assigned to the virtual output at initialization.
 //
 // SIDE EFFECTS: see above
 //
-// NOTES: The virtual output points to a linked list of physical devices.
+// NOTES: The virtual output points to a linked list of physical resources.
 //
 // SCOPE: public
 //
@@ -134,11 +136,11 @@ static uint8_t valid_virtual_device(asdf_virtual_dev_t device)
 }
 
 // PROCEDURE: asdf_virtual_assign
-// INPUTS: (asdf_virtual_dev_t) virtual_out - virtual output to be paired with the physical output
+// INPUTS: (asdf_virtual_dev_t) virtual_out - virtual output to be paired with the physical resource
 //         (asdf_physical_dev_t) physical_out to be assigned to the virtual output.
 //         (asdf_virtual_function_t) - the function to be applied to the virtual
 //              device when activated by a keypress.
-//         (uint8_t) initial_value - the initial state of the physical output.
+//         (uint8_t) initial_value - the initial state of the physical resource.
 //
 // OUTPUTS: none
 //
@@ -147,12 +149,12 @@ static uint8_t valid_virtual_device(asdf_virtual_dev_t device)
 //
 // SIDE EFFECTS: see above.
 //
-// NOTES: if the virtual device is invalid, or the physical device is invalid, or
-// the physical device is already assigned, then nothing happens.
+// NOTES: if the virtual output is invalid, or the physical resource is invalid, or
+// the physical resource is already assigned, then nothing happens.
 //
 // SCOPE: private
 //
-// COMPLEXITY: 2
+// COMPLEXITY: 3
 //
 static void asdf_virtual_assign(asdf_virtual_dev_t virtual_out, asdf_physical_dev_t physical_out,
                                 asdf_virtual_function_t function, uint8_t initial_value)
@@ -177,15 +179,15 @@ static void asdf_virtual_assign(asdf_virtual_dev_t virtual_out, asdf_physical_de
 //
 // SIDE EFFECTS: see above
 //
-// NOTES: 1) //        2) ASDF_VIRTUAL_OUT_DEFAULT_VALUE is defined in asdf_config.h
+// NOTES:  ASDF_VIRTUAL_OUT_DEFAULT_VALUE is defined in asdf_config.h
 //
 // SCOPE: public
 //
-// COMPLEXITY: 2
+// COMPLEXITY: 4
 //
 void asdf_virtual_init(asdf_virtual_initializer_t *const initializer_list)
 {
-  // initial the physical device table every time virtual device table is
+  // initialize the physical resource table every time virtual output table is
   // initialized.
   asdf_physical_init();