OneShot: Switch from 16-bit bitfields to using a struct

On AVR8, 16-bit bit-operations are expensive. Switch from using 4(!) 16-bit
bitfields to using a 16-element array of a carefully constructed struct.

This saves us about 242 PROGMEM at the cost of 8 bytes of memory, and a tiny
performance hit.

Signed-off-by: Gergely Nagy <algernon@keyboard.io>

src/kaleidoscope/plugin/OneShot.cpp

@@ -26,14 +26,10 @@ uint32_t OneShot::start_time_ = 0;
 uint16_t OneShot::time_out = 2500;
 uint16_t OneShot::hold_time_out = 250;
 int16_t OneShot::double_tap_time_out = -1;
-OneShot::state_t OneShot::state_ = {0, 0};
+OneShot::key_state_t OneShot::state_[16];
-OneShot::state_t OneShot::sticky_state_ = {0, 0};
-OneShot::state_t OneShot::stickable_state_ = {0xFF, 0xFF};
-OneShot::state_t OneShot::pressed_state_ = {0, 0};
 Key OneShot::prev_key_;
 bool OneShot::should_cancel_ = false;
 bool OneShot::should_cancel_stickies_ = false;
-uint8_t OneShot::positions_[16];
 
 // --- helper macros ------
 
@@ -41,25 +37,41 @@ uint8_t OneShot::positions_[16];
 #define isModifier(key) (key.raw >= Key_LeftControl.raw && key.raw <= Key_RightGui.raw)
 #define isLayerKey(key) (key.flags == (KEY_FLAGS | SYNTHETIC | SWITCH_TO_KEYMAP))
 
-#define isOneShot(idx) (bitRead (state_.all, (idx)))
+#define isOneShot(idx) (state_[idx].active)
-#define setOneShot(idx) (bitWrite (state_.all, idx, 1))
+#define setOneShot(idx) (state_[idx].active = true)
-#define clearOneShot(idx) (bitWrite (state_.all, idx, 0))
+#define clearOneShot(idx) (state_[idx].active = false)
 
-#define isSticky_(idx) (bitRead (sticky_state_.all, idx))
+#define isSticky_(idx) (state_[idx].sticky)
-#define setSticky(idx) (bitWrite (sticky_state_.all, idx, 1))
+#define setSticky(idx) (state_[idx].sticky = true)
-#define clearSticky(idx) bitWrite (sticky_state_.all, idx, 0)
+#define clearSticky(idx) (state_[idx].sticky = false)
 
-#define setPressed(idx) bitWrite(pressed_state_.all, idx, 1)
+#define setPressed(idx) (state_[idx].pressed = true)
-#define clearPressed(idx) bitWrite(pressed_state_.all, idx, 0)
+#define clearPressed(idx) (state_[idx].pressed = false)
-#define isPressed(idx) bitRead (pressed_state_.all, idx)
+#define isPressed_(idx) (state_[idx].pressed)
 
 #define isSameAsPrevious(key) (key.raw == prev_key_.raw)
 #define saveAsPrevious(key) prev_key_.raw = key.raw
 
 #define hasTimedOut() (millis () - start_time_ >= time_out)
 
+bool OneShot::isPressed() {
+  for (uint8_t i = 0; i < sizeof(state_); i++) {
+    if (state_[i].pressed)
+      return true;
+  }
+  return false;
+}
+
+bool OneShot::isSticky() {
+  for (uint8_t i = 0; i < sizeof(state_); i++) {
+    if (state_[i].sticky)
+      return true;
+  }
+  return false;
+}
+
 bool OneShot::isStickable(Key key) {
-  return bitRead(stickable_state_.all, key.raw - ranges::OS_FIRST);
+  return state_[key.raw - ranges::OS_FIRST].stickable;
 }
 
 void OneShot::positionToCoords(uint8_t pos, byte *row, byte *col) {
@@ -80,7 +92,7 @@ void OneShot::injectNormalKey(uint8_t idx, uint8_t key_state) {
     key.keyCode = LAYER_SHIFT_OFFSET + idx - 8;
   }
 
-  positionToCoords(positions_[idx], &row, &col);
+  positionToCoords(state_[idx].position, &row, &col);
   handleKeyswitchEvent(key, row, col, key_state | INJECTED);
 }
 
@@ -99,7 +111,7 @@ EventHandlerResult OneShot::onKeyswitchEvent(Key &mapped_key, byte row, byte col
   if (keyState & INJECTED)
     return EventHandlerResult::OK;
 
-  if (!state_.all) {
+  if (!isActive()) {
     if (!isOneShotKey(mapped_key)) {
       return EventHandlerResult::OK;
     }
@@ -108,7 +120,7 @@ EventHandlerResult OneShot::onKeyswitchEvent(Key &mapped_key, byte row, byte col
       clearPressed(idx);
     } else if (keyToggledOn(keyState)) {
       start_time_ = millis();
-      positions_[idx] = row * COLS + col;
+      state_[idx].position = row * COLS + col;
       setPressed(idx);
       setOneShot(idx);
       saveAsPrevious(mapped_key);
@@ -148,7 +160,7 @@ EventHandlerResult OneShot::onKeyswitchEvent(Key &mapped_key, byte row, byte col
         } else {
           start_time_ = millis();
 
-          positions_[idx] = row * COLS + col;
+          state_[idx].position = row * COLS + col;
           setOneShot(idx);
           saveAsPrevious(mapped_key);
 
@@ -173,7 +185,7 @@ EventHandlerResult OneShot::onKeyswitchEvent(Key &mapped_key, byte row, byte col
 }
 
 EventHandlerResult OneShot::beforeReportingState() {
-  if (!state_.all)
+  if (!isActive())
     return EventHandlerResult::OK;
 
   for (uint8_t i = 0; i < 8; i++) {
@@ -186,7 +198,7 @@ EventHandlerResult OneShot::beforeReportingState() {
 }
 
 EventHandlerResult OneShot::afterEachCycle() {
-  if (!state_.all)
+  if (!isActive())
     return EventHandlerResult::OK;
 
   if (hasTimedOut())
@@ -203,7 +215,7 @@ EventHandlerResult OneShot::afterEachCycle() {
           cancelOneShot(i);
           clearPressed(i);
         }
-      } else if (isOneShot(i) && !isPressed(i)) {
+      } else if (isOneShot(i) && !isPressed_(i)) {
         is_cancelled = true;
         cancelOneShot(i);
       }
@@ -225,13 +237,21 @@ void OneShot::inject(Key mapped_key, uint8_t key_state) {
 // --- glue code ---
 
 bool OneShot::isActive(void) {
-  return (state_.all && !hasTimedOut()) || (pressed_state_.all) || (sticky_state_.all);
+  for (uint8_t i = 0; i < 16; i++) {
+    if ((state_[i].active && !hasTimedOut()) ||
+        state_[i].pressed ||
+        state_[i].sticky)
+      return true;
+  }
+  return false;
 }
 
 bool OneShot::isActive(Key key) {
   uint8_t idx = key.raw - ranges::OS_FIRST;
 
-  return (bitRead(state_.all, idx) && !hasTimedOut()) || (isPressed(idx)) || (isSticky_(idx));
+  return (state_[idx].active && !hasTimedOut()) ||
+      state_[idx].pressed ||
+      state_[idx].sticky;
 }
 
 bool OneShot::isSticky(Key key) {
@@ -254,28 +274,36 @@ void OneShot::cancel(bool with_stickies) {
 
 void OneShot::enableStickability(Key key) {
   if (key >= ranges::OS_FIRST && key <= ranges::OS_LAST)
-    bitSet(stickable_state_.all, (key.raw - ranges::OS_FIRST));
+    state_[key.raw - ranges::OS_FIRST].stickable = true;
 }
 
 void OneShot::disableStickability(Key key) {
   if (key >= ranges::OS_FIRST && key <= ranges::OS_LAST)
-    bitClear(stickable_state_.all, (key.raw - ranges::OS_FIRST));
+    state_[key.raw - ranges::OS_FIRST].stickable = false;
 }
 
 void OneShot::enableStickabilityForModifiers() {
-  stickable_state_.mods = 0xFF;
+  for (uint8_t i = 0; i < 8; i++) {
+    state_[i].stickable = true;
+  }
 }
 
 void OneShot::enableStickabilityForLayers() {
-  stickable_state_.layers = 0xFF;
+  for (uint8_t i = 8; i < 16; i++) {
+    state_[i].stickable = true;
+  }
 }
 
 void OneShot::disableStickabilityForModifiers() {
-  stickable_state_.mods = 0;
+  for (uint8_t i = 0; i < 8; i++) {
+    state_[i].stickable = false;
+  }
 }
 
 void OneShot::disableStickabilityForLayers() {
-  stickable_state_.layers = 0;
+  for (uint8_t i = 8; i < 16; i++) {
+    state_[i].stickable = false;
+  }
 }
 
 }

src/kaleidoscope/plugin/OneShot.h

@@ -28,21 +28,22 @@ namespace plugin {
 
 class OneShot : public kaleidoscope::Plugin {
  public:
-  OneShot(void) {}
+  OneShot(void) {
+    for (uint8_t i = 0; i < 16; i++) {
+      state_[i].stickable = true;
+    }
+  }
 
   static bool isOneShotKey(Key key) {
     return (key.raw >= kaleidoscope::ranges::OS_FIRST && key.raw <= kaleidoscope::ranges::OS_LAST);
   }
   static bool isActive(void);
-  static bool isPressed() {
-    return !!pressed_state_.all;
-  }
-  static bool isSticky() {
-    return !!sticky_state_.all;
-  }
   static bool isActive(Key key);
+  static bool isPressed();
+  static bool isSticky();
   static bool isSticky(Key key);
   static void cancel(bool with_stickies = false);
+
   static uint16_t time_out;
   static int16_t double_tap_time_out;
   static uint16_t hold_time_out;
@@ -78,22 +79,20 @@ class OneShot : public kaleidoscope::Plugin {
   void inject(Key mapped_key, uint8_t key_state);
 
  private:
-  typedef union {
+  typedef struct {
-    struct {
+    bool active: 1;
-      uint8_t mods;
+    bool pressed: 1;
-      uint8_t layers;
+    bool stickable: 1;
-    };
+    bool sticky: 1;
-    uint16_t all;
+    uint8_t __reserved: 4;
-  } state_t;
+    uint8_t position;
+  } key_state_t;
+  static key_state_t state_[16];
+
   static uint32_t start_time_;
-  static state_t state_;
-  static state_t sticky_state_;
-  static state_t stickable_state_;
-  static state_t pressed_state_;
   static Key prev_key_;
   static bool should_cancel_;
   static bool should_cancel_stickies_;
-  static uint8_t positions_[16];
 
   static void positionToCoords(uint8_t pos, byte *row, byte *col);