From 380a2e8a28944810196ffb9059294dde93c61835 Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 00:24:59 -0700
Subject: [PATCH 1/8] rename some variables to make them clearer

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 11 +++++------
 src/Kaleidoscope-NumPad.h   |  3 ++-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 2755efc8..16e480e9 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -19,7 +19,7 @@
 #include "Kaleidoscope.h"
 #include "layers.h"
 
-byte NumPad_::row = 255, NumPad_::col = 255;
+byte NumPad_::numpad_lock_key_row = 255, NumPad_::numpad_lock_key_col = 255;
 uint8_t NumPad_::numPadLayer;
 bool NumPad_::cleanupDone = true;
 bool NumPad_::originalNumLockState = false;
@@ -70,8 +70,8 @@ kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
       Key layer_key = Layer.getKey(numPadLayer, r, c);
 
       if (k == LockLayer(numPadLayer)) {
-        row  = r;
-        col = c;
+        numpad_lock_key_row = r;
+        numpad_lock_key_col = c;
       }
 
       if ((k != layer_key) || (k == Key_NoKey) || (k.flags != KEY_FLAGS)) {
@@ -82,11 +82,10 @@ kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
     }
   }
 
-  if (row > ROWS || col > COLS)
+  if ((numpad_lock_key_row > ROWS) || (numpad_lock_key_col > COLS)) {
     return kaleidoscope::EventHandlerResult::OK;
 
-  cRGB lock_color = breath_compute(lock_hue);
-  LEDControl.setCrgbAt(row, col, lock_color);
+  LEDControl.setCrgbAt(numpad_lock_key_row, numpad_lock_key_col, lock_color);
 
   return kaleidoscope::EventHandlerResult::OK;
 }
diff --git a/src/Kaleidoscope-NumPad.h b/src/Kaleidoscope-NumPad.h
index 2487b64a..843a14da 100644
--- a/src/Kaleidoscope-NumPad.h
+++ b/src/Kaleidoscope-NumPad.h
@@ -32,7 +32,8 @@ class NumPad_ : public kaleidoscope::Plugin {
   kaleidoscope::EventHandlerResult afterEachCycle();
 
  private:
-  static byte row, col;
+  static uint8_t numpad_lock_key_row;
+  static uint8_t numpad_lock_key_col;
   static bool cleanupDone;
   static bool originalNumLockState;
 };

From b491146f33184820f7b72a93514eaee85773aaee Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 00:27:06 -0700
Subject: [PATCH 2/8] Extract a couple methods and rename a variable

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 52 +++++++++++++++++++++++--------------
 src/Kaleidoscope-NumPad.h   |  4 +++
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 16e480e9..3bb77deb 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -36,32 +36,30 @@ static bool getNumlockState() {
 }
 
 static void syncNumlock(bool state) {
-  bool numState = getNumlockState();
-  if (numState != state) {
+  bool numLockLEDState = getNumlockState();
+  if (numLockLEDState != state) {
     kaleidoscope::hid::pressKey(Key_KeypadNumLock);
   }
 }
 
-kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
-  if (!Layer.isOn(numPadLayer)) {
-    bool numState = getNumlockState();
-    if (!cleanupDone) {
-      LEDControl.set_mode(LEDControl.get_mode_index());
 
-      if (!originalNumLockState) {
-        syncNumlock(false);
-        numState = false;
-      }
-      cleanupDone = true;
-    }
-    originalNumLockState = numState;
 
-    return kaleidoscope::EventHandlerResult::OK;
+void NumPad_::cleanupNumlockState() {
+  bool numLockLEDState = getNumlockState();
+  if (!cleanupDone) {
+    LEDControl.set_mode(LEDControl.get_mode_index());
+
+    if (!originalNumLockState) {
+      syncNumlock(false);
+      numLockLEDState = false;
+    }
+    cleanupDone = true;
   }
+  originalNumLockState = numLockLEDState;
 
-  cleanupDone = false;
-  syncNumlock(true);
+}
 
+void NumPad_::setKeyboardLEDColors(void) {
   LEDControl.set_mode(LEDControl.get_mode_index());
 
   for (uint8_t r = 0; r < ROWS; r++) {
@@ -82,12 +80,28 @@ kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
     }
   }
 
-  if ((numpad_lock_key_row > ROWS) || (numpad_lock_key_col > COLS)) {
+  if ((numpad_lock_key_row <= ROWS) && (numpad_lock_key_col <= COLS)) {
+
+
+    cRGB lock_color = breath_compute(lock_hue);
+    LEDControl.setCrgbAt(numpad_lock_key_row, numpad_lock_key_col, lock_color);
+  }
+}
+
+kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
+  if (!Layer.isOn(numPadLayer)) {
+    cleanupNumlockState();
     return kaleidoscope::EventHandlerResult::OK;
+  }
+
+  cleanupDone = false;
+  syncNumlock(true);
 
-  LEDControl.setCrgbAt(numpad_lock_key_row, numpad_lock_key_col, lock_color);
 
+  setKeyboardLEDColors();
   return kaleidoscope::EventHandlerResult::OK;
 }
 
+
+
 NumPad_ NumPad;
diff --git a/src/Kaleidoscope-NumPad.h b/src/Kaleidoscope-NumPad.h
index 843a14da..bad11578 100644
--- a/src/Kaleidoscope-NumPad.h
+++ b/src/Kaleidoscope-NumPad.h
@@ -32,6 +32,10 @@ class NumPad_ : public kaleidoscope::Plugin {
   kaleidoscope::EventHandlerResult afterEachCycle();
 
  private:
+
+  void cleanupNumlockState(void);
+  void setKeyboardLEDColors(void);
+
   static uint8_t numpad_lock_key_row;
   static uint8_t numpad_lock_key_col;
   static bool cleanupDone;

From 7c1a5cea95e822c546248e7ba538561e1a6f75cb Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 00:53:33 -0700
Subject: [PATCH 3/8] Refactoring. no functional changes

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 20 +++++++++-----------
 src/Kaleidoscope-NumPad.h   |  2 ++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 3bb77deb..34eb654b 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -27,15 +27,15 @@ cRGB NumPad_::color = CRGB(160, 0, 0);
 uint8_t NumPad_::lock_hue = 170;
 
 kaleidoscope::EventHandlerResult NumPad_::onSetup(void) {
-  originalNumLockState = !!(kaleidoscope::hid::getKeyboardLEDs() & LED_NUM_LOCK);
+  originalNumLockState = getNumlockState();
   return kaleidoscope::EventHandlerResult::OK;
 }
 
-static bool getNumlockState() {
+bool NumPad_::getNumlockState() {
   return !!(kaleidoscope::hid::getKeyboardLEDs() & LED_NUM_LOCK);
 }
 
-static void syncNumlock(bool state) {
+void NumPad_::syncNumlockState(bool state) {
   bool numLockLEDState = getNumlockState();
   if (numLockLEDState != state) {
     kaleidoscope::hid::pressKey(Key_KeypadNumLock);
@@ -50,7 +50,7 @@ void NumPad_::cleanupNumlockState() {
     LEDControl.set_mode(LEDControl.get_mode_index());
 
     if (!originalNumLockState) {
-      syncNumlock(false);
+      syncNumlockState(false);
       numLockLEDState = false;
     }
     cleanupDone = true;
@@ -91,14 +91,12 @@ void NumPad_::setKeyboardLEDColors(void) {
 kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
   if (!Layer.isOn(numPadLayer)) {
     cleanupNumlockState();
-    return kaleidoscope::EventHandlerResult::OK;
-  }
-
-  cleanupDone = false;
-  syncNumlock(true);
+  } else {
+    cleanupDone = false;
+    syncNumlockState(true);
+    setKeyboardLEDColors();
 
-
-  setKeyboardLEDColors();
+  }
   return kaleidoscope::EventHandlerResult::OK;
 }
 
diff --git a/src/Kaleidoscope-NumPad.h b/src/Kaleidoscope-NumPad.h
index bad11578..4c7311f3 100644
--- a/src/Kaleidoscope-NumPad.h
+++ b/src/Kaleidoscope-NumPad.h
@@ -35,6 +35,8 @@ class NumPad_ : public kaleidoscope::Plugin {
 
   void cleanupNumlockState(void);
   void setKeyboardLEDColors(void);
+  bool getNumlockState(void);
+  void syncNumlockState(bool);
 
   static uint8_t numpad_lock_key_row;
   static uint8_t numpad_lock_key_col;

From 79996b7216b9bba62763db9dd6aa63a71f0ebf50 Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 00:53:58 -0700
Subject: [PATCH 4/8] Only try to reset the numlock led if we actually haven't
 done the cleanup

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 34eb654b..be424624 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -45,17 +45,16 @@ void NumPad_::syncNumlockState(bool state) {
 
 
 void NumPad_::cleanupNumlockState() {
-  bool numLockLEDState = getNumlockState();
   if (!cleanupDone) {
+    bool numLockLEDState = getNumlockState();
     LEDControl.set_mode(LEDControl.get_mode_index());
-
     if (!originalNumLockState) {
       syncNumlockState(false);
       numLockLEDState = false;
     }
     cleanupDone = true;
+    originalNumLockState = numLockLEDState;
   }
-  originalNumLockState = numLockLEDState;
 
 }
 

From 8fb50b2415a49903d6f0a493313f02d938b89939 Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 01:02:22 -0700
Subject: [PATCH 5/8] Only run our "toggle numlock mode" once upon toggle of
 the layer

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index be424624..2d32e666 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -21,7 +21,7 @@
 
 byte NumPad_::numpad_lock_key_row = 255, NumPad_::numpad_lock_key_col = 255;
 uint8_t NumPad_::numPadLayer;
-bool NumPad_::cleanupDone = true;
+bool NumPad_::cleanupDone = false;
 bool NumPad_::originalNumLockState = false;
 cRGB NumPad_::color = CRGB(160, 0, 0);
 uint8_t NumPad_::lock_hue = 170;
@@ -52,8 +52,8 @@ void NumPad_::cleanupNumlockState() {
       syncNumlockState(false);
       numLockLEDState = false;
     }
-    cleanupDone = true;
     originalNumLockState = numLockLEDState;
+    cleanupDone = true;
   }
 
 }
@@ -90,10 +90,11 @@ void NumPad_::setKeyboardLEDColors(void) {
 kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
   if (!Layer.isOn(numPadLayer)) {
     cleanupNumlockState();
-  } else {
-    cleanupDone = false;
+  } else if (cleanupDone)  {
+    // If it's the first time we're in this loop after toggling the Numpad mode on
     syncNumlockState(true);
     setKeyboardLEDColors();
+    cleanupDone = false;
 
   }
   return kaleidoscope::EventHandlerResult::OK;

From efbf158b132bce76374c40663dc2c2f3dc82bf3e Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 23:03:55 -0700
Subject: [PATCH 6/8] Bring back the LED breathing effect

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 2d32e666..b621cd50 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -90,12 +90,13 @@ void NumPad_::setKeyboardLEDColors(void) {
 kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
   if (!Layer.isOn(numPadLayer)) {
     cleanupNumlockState();
-  } else if (cleanupDone)  {
-    // If it's the first time we're in this loop after toggling the Numpad mode on
-    syncNumlockState(true);
+  } else {
+    if (cleanupDone)  {
+      // If it's the first time we're in this loop after toggling the Numpad mode on
+      syncNumlockState(true);
+      cleanupDone = false;
+    }
     setKeyboardLEDColors();
-    cleanupDone = false;
-
   }
   return kaleidoscope::EventHandlerResult::OK;
 }

From c6886b09fb15f89e2a69364e84d410dc3edd2460 Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 23:15:07 -0700
Subject: [PATCH 7/8] Change cleanupDone to numlockUnsynced to better represent
 what it does

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 10 +++++-----
 src/Kaleidoscope-NumPad.h   |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index b621cd50..18d86928 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -21,7 +21,7 @@
 
 byte NumPad_::numpad_lock_key_row = 255, NumPad_::numpad_lock_key_col = 255;
 uint8_t NumPad_::numPadLayer;
-bool NumPad_::cleanupDone = false;
+bool NumPad_::numlockUnsynced = false;
 bool NumPad_::originalNumLockState = false;
 cRGB NumPad_::color = CRGB(160, 0, 0);
 uint8_t NumPad_::lock_hue = 170;
@@ -45,7 +45,7 @@ void NumPad_::syncNumlockState(bool state) {
 
 
 void NumPad_::cleanupNumlockState() {
-  if (!cleanupDone) {
+  if (!numlockUnsynced) {
     bool numLockLEDState = getNumlockState();
     LEDControl.set_mode(LEDControl.get_mode_index());
     if (!originalNumLockState) {
@@ -53,7 +53,7 @@ void NumPad_::cleanupNumlockState() {
       numLockLEDState = false;
     }
     originalNumLockState = numLockLEDState;
-    cleanupDone = true;
+    numlockUnsynced = true;
   }
 
 }
@@ -91,10 +91,10 @@ kaleidoscope::EventHandlerResult NumPad_::afterEachCycle() {
   if (!Layer.isOn(numPadLayer)) {
     cleanupNumlockState();
   } else {
-    if (cleanupDone)  {
+    if (numlockUnsynced)  {
       // If it's the first time we're in this loop after toggling the Numpad mode on
       syncNumlockState(true);
-      cleanupDone = false;
+      numlockUnsynced = false;
     }
     setKeyboardLEDColors();
   }
diff --git a/src/Kaleidoscope-NumPad.h b/src/Kaleidoscope-NumPad.h
index 4c7311f3..1d3595e9 100644
--- a/src/Kaleidoscope-NumPad.h
+++ b/src/Kaleidoscope-NumPad.h
@@ -40,7 +40,7 @@ class NumPad_ : public kaleidoscope::Plugin {
 
   static uint8_t numpad_lock_key_row;
   static uint8_t numpad_lock_key_col;
-  static bool cleanupDone;
+  static bool numlockUnsynced;
   static bool originalNumLockState;
 };
 

From e3acde94194bcc0c145ab5cf4e9109107b4e05c8 Mon Sep 17 00:00:00 2001
From: Jesse Vincent <jesse@keyboard.io>
Date: Fri, 24 Aug 2018 23:23:08 -0700
Subject: [PATCH 8/8] rename a variable that didn't match the style guide

Signed-off-by: Jesse Vincent <jesse@keyboard.io>
---
 src/Kaleidoscope-NumPad.cpp | 10 +++++-----
 src/Kaleidoscope-NumPad.h   |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Kaleidoscope-NumPad.cpp b/src/Kaleidoscope-NumPad.cpp
index 18d86928..d90fbcdc 100644
--- a/src/Kaleidoscope-NumPad.cpp
+++ b/src/Kaleidoscope-NumPad.cpp
@@ -19,7 +19,7 @@
 #include "Kaleidoscope.h"
 #include "layers.h"
 
-byte NumPad_::numpad_lock_key_row = 255, NumPad_::numpad_lock_key_col = 255;
+byte NumPad_::numpadLayerToggleKeyRow = 255, NumPad_::numpadLayerToggleKeyCol = 255;
 uint8_t NumPad_::numPadLayer;
 bool NumPad_::numlockUnsynced = false;
 bool NumPad_::originalNumLockState = false;
@@ -67,8 +67,8 @@ void NumPad_::setKeyboardLEDColors(void) {
       Key layer_key = Layer.getKey(numPadLayer, r, c);
 
       if (k == LockLayer(numPadLayer)) {
-        numpad_lock_key_row = r;
-        numpad_lock_key_col = c;
+        numpadLayerToggleKeyRow = r;
+        numpadLayerToggleKeyCol = c;
       }
 
       if ((k != layer_key) || (k == Key_NoKey) || (k.flags != KEY_FLAGS)) {
@@ -79,11 +79,11 @@ void NumPad_::setKeyboardLEDColors(void) {
     }
   }
 
-  if ((numpad_lock_key_row <= ROWS) && (numpad_lock_key_col <= COLS)) {
+  if ((numpadLayerToggleKeyRow <= ROWS) && (numpadLayerToggleKeyCol <= COLS)) {
 
 
     cRGB lock_color = breath_compute(lock_hue);
-    LEDControl.setCrgbAt(numpad_lock_key_row, numpad_lock_key_col, lock_color);
+    LEDControl.setCrgbAt(numpadLayerToggleKeyRow, numpadLayerToggleKeyCol, lock_color);
   }
 }
 
diff --git a/src/Kaleidoscope-NumPad.h b/src/Kaleidoscope-NumPad.h
index 1d3595e9..016ded65 100644
--- a/src/Kaleidoscope-NumPad.h
+++ b/src/Kaleidoscope-NumPad.h
@@ -38,8 +38,8 @@ class NumPad_ : public kaleidoscope::Plugin {
   bool getNumlockState(void);
   void syncNumlockState(bool);
 
-  static uint8_t numpad_lock_key_row;
-  static uint8_t numpad_lock_key_col;
+  static uint8_t numpadLayerToggleKeyRow;
+  static uint8_t numpadLayerToggleKeyCol;
   static bool numlockUnsynced;
   static bool originalNumLockState;
 };