From 5a049fdb8820ddc92b60f0e212cbaf2c76d02dc4 Mon Sep 17 00:00:00 2001
From: matt venn <matt@mattvenn.net>
Date: Fri, 16 Jun 2017 09:50:37 +0200
Subject: [PATCH 1/2] comments

---
 firmware/keyscanner.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/firmware/keyscanner.c b/firmware/keyscanner.c
index 5871d3d..2a5eff5 100644
--- a/firmware/keyscanner.c
+++ b/firmware/keyscanner.c
@@ -49,32 +49,25 @@ void keyscanner_main(void) {
         return;
     }
 
+    do_scan = 0;
+
     // For each enabled row...
     for (uint8_t row = 0; row < ROW_COUNT; ++row) {
         // Reset all of our row pins, then
         // set the one we want to read as low
         LOW(PORT_ROWS, row);
-        /* We need a no-op for synchronization. So says the datasheet
-         * in Section 10.2.5 */
+        /* We need a no-op for to allow time for the register state to change.
+         * So says the ATTiny88 datasheet in Section 10.2.5 */
         asm("nop");
         pin_data = PIN_COLS;
         HIGH(PORT_ROWS,row);
-        // Debounce key state
+        // Debounce key state - check debounce.h to see how it works
         debounced_changes += debounce((pin_data) , db + row);
     }
 
     // Most of the time there will be no new key events
-    if (__builtin_expect(debounced_changes == 0, 1)) {
-        // Only run the debouncing delay when we haven't successfully found
-        // a debounced event
-
-        // XXX TODO make sure this isn't crazy. could this 
-        // cause us to do reads too fast and mis-debounce
-        // some secondary key while we successfully debounce a
-        // first key.
-        do_scan = 0;
+    if (__builtin_expect(debounced_changes == 0, 1)) 
         return;
-    }
 
     // Snapshot the keystate to add to the ring buffer
     // Run this with interrupts off to make sure that

From b01f1e8e0888eb7445eb60f19e79f1e77729a199 Mon Sep 17 00:00:00 2001
From: matt venn <matt@mattvenn.net>
Date: Fri, 16 Jun 2017 09:51:30 +0200
Subject: [PATCH 2/2] explanatory comments and imroved readability of code

---
 firmware/debounce.h | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/firmware/debounce.h b/firmware/debounce.h
index 8231fa8..13d11dc 100644
--- a/firmware/debounce.h
+++ b/firmware/debounce.h
@@ -2,6 +2,12 @@
 
 #include <stdint.h>
 
+/*
+each of these 8 bit variables are storing the state for 8 keys
+
+so for key 0, the counter is represented by db0[0] and db1[0] 
+and the state in state[0].
+*/
 typedef struct {
     uint8_t db0;    // counter bit 0
     uint8_t db1;    // counter bit 1
@@ -15,24 +21,45 @@ typedef struct {
  *    will increment that bit's counter. When it overflows, the change is
  *    comitted to the final debounced state and the changed bit returned.
  *
+ *    Because each key's counter and state are stored in this stacked way,
+ *    8 keys are processed in parallel at each operation.
+ *
  * args:
  *    sample - the current state
  *    debouncer - the state variables of the debouncer
  *
  * returns: bits that have changed in the final debounced state
+ *
+ * handy XOR truth table:   A  B  O
+ *                          0  0  0
+ *                          0  1  1
+ *                          1  0  1
+ *                          1  1  0
+ * 
+ * This is used below as a difference detector:
+ *   if A ^ B is true, A and B are different.
+ *
+ * And a way to flip selected bits in a variable or register:
+ *   Set B to 1, then A ^ B = !A
  */
 static inline uint8_t debounce(uint8_t sample, debounce_t *debouncer) {
     uint8_t delta, changes;
 
-    // Set delta to changes from last stable state
+    // Use xor to detect changes from last stable state:
+    // if a key has changed, it's bit will be 1, otherwise 0
     delta = sample ^ debouncer->state;
 
-    // Increment counters and reset any unchanged bits
+    // Increment counters and reset any unchanged bits:
+    // increment bit 1 for all changed keys
     debouncer->db1 = ((debouncer->db1) ^ (debouncer->db0)) & delta;
-    debouncer->db0  = ~(debouncer->db0) & delta;
+    // increment bit 0 for all changed keys
+    debouncer->db0 = ~(debouncer->db0) & delta;
+
+    // Calculate returned change set: if delta is still true
+    // and the counter has wrapped back to 0, the key is changed.
+    changes = delta & ~debouncer->db0 & ~debouncer->db1;
 
-    // update state & calculate returned change set
-    changes = ~(~delta | (debouncer->db0) | (debouncer->db1));
+    // Update state: in this case use xor to flip any bit that is true in changes.
     debouncer->state ^= changes;
 
     return changes;