Use a single list of keypad scanners

Author: dhalbert

py/circuitpy_mpconfig.h

@@ -533,9 +533,7 @@ extern const struct _mp_obj_module_t ipaddress_module;
 #if CIRCUITPY_KEYPAD
 extern const struct _mp_obj_module_t keypad_module;
 #define KEYPAD_MODULE        { MP_OBJ_NEW_QSTR(MP_QSTR_keypad), (mp_obj_t)&keypad_module },
-#define KEYPAD_ROOT_POINTERS \
-    mp_obj_t keypad_keys_linked_list; \
-    mp_obj_t keypad_keymatrix_linked_list;
+#define KEYPAD_ROOT_POINTERS mp_obj_t keypad_scanners_linked_list;
 #else
 #define KEYPAD_MODULE
 #define KEYPAD_ROOT_POINTERS

shared-module/keypad/KeyMatrix.c

@@ -26,16 +26,14 @@
 
 #include "py/gc.h"
 #include "py/runtime.h"
+#include "shared-bindings/keypad/__init__.h"
 #include "shared-bindings/keypad/Event.h"
 #include "shared-bindings/keypad/KeyMatrix.h"
 #include "shared-bindings/digitalio/DigitalInOut.h"
 #include "shared-bindings/util.h"
 #include "supervisor/port.h"
-#include "supervisor/shared/lock.h"
 #include "supervisor/shared/tick.h"
 
-static supervisor_lock_t keypad_keymatrix_linked_list_lock;
-
 #define DEBOUNCE_TICKS (20)
 
 // Top bit of 16-bit event indicates pressed or released. Rest is key_num.
@@ -75,12 +73,8 @@ void common_hal_keypad_keymatrix_construct(keypad_keymatrix_obj_t *self, mp_uint
     // Event queue is 16-bit values.
     ringbuf_alloc(&self->encoded_events, max_events * 2, false);
 
-    // Add self to the list of active Keys objects.
-
-    supervisor_acquire_lock(&keypad_keymatrix_linked_list_lock);
-    self->next = MP_STATE_VM(keypad_keymatrix_linked_list);
-    MP_STATE_VM(keypad_keymatrix_linked_list) = self;
-    supervisor_release_lock(&keypad_keymatrix_linked_list_lock);
+    // Add self to the list of active keypad scanners.
+    keypad_register_scanner((keypad_scanner_obj_t *)self);
 
     supervisor_enable_tick();
 }
@@ -90,6 +84,9 @@ void common_hal_keypad_keymatrix_deinit(keypad_keymatrix_obj_t *self) {
         return;
     }
 
+    // Remove self from the list of active keypad scanners first.
+    keypad_deregister_scanner((keypad_scanner_obj_t *)self);
+
     for (size_t row = 0; row < common_hal_keypad_keymatrix_num_rows(self); row++) {
         common_hal_digitalio_digitalinout_deinit(self->row_digitalinouts->items[row]);
     }
@@ -99,25 +96,6 @@ void common_hal_keypad_keymatrix_deinit(keypad_keymatrix_obj_t *self) {
         common_hal_digitalio_digitalinout_deinit(self->col_digitalinouts->items[col]);
     }
     self->col_digitalinouts = MP_ROM_NONE;
-
-    // Remove self from the list of active KeyMatrix objects.
-
-    supervisor_acquire_lock(&keypad_keymatrix_linked_list_lock);
-    if (MP_STATE_VM(keypad_keymatrix_linked_list) == self) {
-        // I'm at the front; splice myself out.
-        MP_STATE_VM(keypad_keymatrix_linked_list) = self->next;
-    } else {
-        keypad_keymatrix_obj_t *current = MP_STATE_VM(keypad_keymatrix_linked_list);
-        while (current) {
-            if (current->next == self) {
-                // Splice myself out.
-                current->next = self->next;
-                break;
-            }
-            current = current->next;
-        }
-    }
-    supervisor_release_lock(&keypad_keymatrix_linked_list_lock);
 }
 
 bool common_hal_keypad_keymatrix_deinited(keypad_keymatrix_obj_t *self) {
@@ -160,7 +138,7 @@ mp_uint_t common_hal_keypad_keymatrix_key_num(keypad_keymatrix_obj_t *self, mp_u
     return row_col_to_key_num(self, row, col);
 }
 
-static void keypad_keymatrix_scan(keypad_keymatrix_obj_t *self) {
+void keypad_keymatrix_scan(keypad_keymatrix_obj_t *self) {
     uint64_t now = port_get_raw_ticks(NULL);
     if (now - self->last_scan_ticks < DEBOUNCE_TICKS) {
         // Too soon. Wait longer to debounce.
@@ -201,28 +179,3 @@ static void keypad_keymatrix_scan(keypad_keymatrix_obj_t *self) {
         common_hal_digitalio_digitalinout_switch_to_input(self->row_digitalinouts->items[row], PULL_UP);
     }
 }
-
-void keypad_keymatrix_tick(void) {
-    // Fast path.
-    if (!MP_STATE_VM(keypad_keymatrix_linked_list)) {
-        return;
-    }
-
-    if (supervisor_try_lock(&keypad_keymatrix_linked_list_lock)) {
-        keypad_keymatrix_obj_t *keypad_keymatrix = MP_STATE_VM(keypad_keymatrix_linked_list);
-        while (keypad_keymatrix) {
-            keypad_keymatrix_scan(keypad_keymatrix);
-            keypad_keymatrix = keypad_keymatrix->next;
-        }
-        supervisor_release_lock(&keypad_keymatrix_linked_list_lock);
-    }
-}
-
-void keypad_keymatrix_reset(void) {
-    if (MP_STATE_VM(keypad_keymatrix_linked_list)) {
-        supervisor_disable_tick();
-    }
-
-    MP_STATE_VM(keypad_keys_linked_list) = NULL;
-    keypad_keymatrix_linked_list_lock = false;
-}

shared-module/keypad/KeyMatrix.h

@@ -27,25 +27,25 @@
 #ifndef MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYMATRIX_H
 #define MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYMATRIX_H
 
-#include "common-hal/digitalio/DigitalInOut.h"
-
 #include "py/obj.h"
 #include "py/objtuple.h"
 #include "py/ringbuf.h"
 
-typedef struct _keypad_keymatrix_obj_t {
+#include "common-hal/digitalio/DigitalInOut.h"
+#include "shared-module/keypad/__init__.h"
+
+typedef struct {
     mp_obj_base_t base;
+    // All scanners have a next field here, to keep a linked list of active scanners.
+    keypad_scanner_obj_t *next;
     mp_obj_tuple_t *row_digitalinouts;
     mp_obj_tuple_t *col_digitalinouts;
     uint64_t last_scan_ticks;
     bool *previously_pressed;
     bool *currently_pressed;
     ringbuf_t encoded_events;
-    // Keep a linked list of active KeyMatrix objects.
-    struct _keypad_keymatrix_obj_t *next;
 } keypad_keymatrix_obj_t;
 
-void keypad_keymatrix_tick(void);
-void keypad_keymatrix_reset(void);
+void keypad_keymatrix_scan(keypad_keymatrix_obj_t *self);
 
 #endif  // MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYMATRIX_H

shared-module/keypad/Keys.c

@@ -26,15 +26,13 @@
 
 #include "py/gc.h"
 #include "py/runtime.h"
+#include "shared-bindings/keypad/__init__.h"
 #include "shared-bindings/keypad/Event.h"
 #include "shared-bindings/keypad/Keys.h"
 #include "shared-bindings/digitalio/DigitalInOut.h"
 #include "supervisor/port.h"
-#include "supervisor/shared/lock.h"
 #include "supervisor/shared/tick.h"
 
-static supervisor_lock_t keypad_keys_linked_list_lock;
-
 #define DEBOUNCE_TICKS (20)
 
 // Top bit of 16-bit event indicates pressed or released. Rest is key_num.
@@ -64,12 +62,8 @@ void common_hal_keypad_keys_construct(keypad_keys_obj_t *self, mp_uint_t num_pin
     // Event queue is 16-bit values.
     ringbuf_alloc(&self->encoded_events, max_events * 2, false);
 
-    // Add self to the list of active Keys objects.
-
-    supervisor_acquire_lock(&keypad_keys_linked_list_lock);
-    self->next = MP_STATE_VM(keypad_keys_linked_list);
-    MP_STATE_VM(keypad_keys_linked_list) = self;
-    supervisor_release_lock(&keypad_keys_linked_list_lock);
+    // Add self to the list of active keypad scanners.
+    keypad_register_scanner((keypad_scanner_obj_t *)self);
 
     supervisor_enable_tick();
 }
@@ -79,29 +73,14 @@ void common_hal_keypad_keys_deinit(keypad_keys_obj_t *self) {
         return;
     }
 
+    // Remove self from the list of active keypad scanners first.
+    keypad_deregister_scanner((keypad_scanner_obj_t *)self);
+
     for (size_t key = 0; key < common_hal_keypad_keys_num_keys(self); key++) {
         common_hal_digitalio_digitalinout_deinit(self->digitalinouts->items[key]);
     }
     self->digitalinouts = MP_ROM_NONE;
 
-    // Remove self from the list of active Keys objects.
-
-    supervisor_acquire_lock(&keypad_keys_linked_list_lock);
-    if (MP_STATE_VM(keypad_keys_linked_list) == self) {
-        // I'm at the front; splice myself out.
-        MP_STATE_VM(keypad_keys_linked_list) = self->next;
-    } else {
-        keypad_keys_obj_t *current = MP_STATE_VM(keypad_keys_linked_list);
-        while (current) {
-            if (current->next == self) {
-                // Splice myself out.
-                current->next = self->next;
-                break;
-            }
-            current = current->next;
-        }
-    }
-    supervisor_release_lock(&keypad_keys_linked_list_lock);
 }
 
 bool common_hal_keypad_keys_deinited(keypad_keys_obj_t *self) {
@@ -131,7 +110,7 @@ void common_hal_keypad_keys_clear_events(keypad_keys_obj_t *self) {
     ringbuf_clear(&self->encoded_events);
 }
 
-static void keypad_keys_scan(keypad_keys_obj_t *self) {
+void keypad_keys_scan(keypad_keys_obj_t *self) {
     uint64_t now = port_get_raw_ticks(NULL);
     if (now - self->last_scan_ticks < DEBOUNCE_TICKS) {
         // Too soon. Wait longer to debounce.
@@ -160,28 +139,3 @@ static void keypad_keys_scan(keypad_keys_obj_t *self) {
         }
     }
 }
-
-void keypad_keys_tick(void) {
-    // Fast path.
-    if (!MP_STATE_VM(keypad_keys_linked_list)) {
-        return;
-    }
-
-    if (supervisor_try_lock(&keypad_keys_linked_list_lock)) {
-        keypad_keys_obj_t *keypad_keys = MP_STATE_VM(keypad_keys_linked_list);
-        while (keypad_keys) {
-            keypad_keys_scan(keypad_keys);
-            keypad_keys = keypad_keys->next;
-        }
-        supervisor_release_lock(&keypad_keys_linked_list_lock);
-    }
-}
-
-void keypad_keys_reset(void) {
-    if (MP_STATE_VM(keypad_keys_linked_list)) {
-        supervisor_disable_tick();
-    }
-
-    MP_STATE_VM(keypad_keys_linked_list) = NULL;
-    keypad_keys_linked_list_lock = false;
-}

shared-module/keypad/Keys.h

@@ -27,25 +27,25 @@
 #ifndef MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYS_H
 #define MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYS_H
 
-#include "common-hal/digitalio/DigitalInOut.h"
-
 #include "py/obj.h"
 #include "py/objtuple.h"
 #include "py/ringbuf.h"
 
-typedef struct _keypad_keys_obj_t {
+#include "common-hal/digitalio/DigitalInOut.h"
+#include "shared-module/keypad/__init__.h"
+
+typedef struct {
     mp_obj_base_t base;
+    // All scanners have a next field here, to keep a linked list of active scanners.
+    keypad_scanner_obj_t *next;
     mp_obj_tuple_t *digitalinouts;
     uint64_t last_scan_ticks;
-    bool value_when_pressed;
     bool *previously_pressed;
     bool *currently_pressed;
     ringbuf_t encoded_events;
-    // Keep a linked list of active Keys objects.
-    struct _keypad_keys_obj_t *next;
+    bool value_when_pressed;
 } keypad_keys_obj_t;
 
-void keypad_keys_tick(void);
-void keypad_keys_reset(void);
+void keypad_keys_scan(keypad_keys_obj_t *self);
 
 #endif  // MICROPY_INCLUDED_SHARED_MODULE_KEYPAD_KEYS_H

shared-module/keypad/__init__.c

@@ -24,15 +24,70 @@
  * THE SOFTWARE.
  */
 
-#include "shared-module/keypad/Keys.h"
-#include "shared-module/keypad/KeyMatrix.h"
+#include "shared-bindings/keypad/Keys.h"
+#include "shared-bindings/keypad/KeyMatrix.h"
+#include "supervisor/shared/lock.h"
+#include "supervisor/shared/tick.h"
+
+static supervisor_lock_t keypad_scanners_linked_list_lock;
 
 void keypad_tick(void) {
-    keypad_keys_tick();
-    keypad_keymatrix_tick();
+    // Fast path. Return immediately if there are no scanners.
+    if (!MP_STATE_VM(keypad_scanners_linked_list)) {
+        return;
+    }
+
+    // Skip scanning if someone else has the lock. Don't wait for the lock.
+    if (supervisor_try_lock(&keypad_scanners_linked_list_lock)) {
+        mp_obj_t scanner = MP_STATE_VM(keypad_scanners_linked_list);
+        while (scanner) {
+            if (mp_obj_is_type(scanner, &keypad_keys_type)) {
+                keypad_keys_scan((keypad_keys_obj_t *)scanner);
+            } else if (mp_obj_is_type(scanner, &keypad_keymatrix_type)) {
+                keypad_keymatrix_scan((keypad_keymatrix_obj_t *)scanner);
+            }
+
+            scanner = ((keypad_scanner_obj_t *)scanner)->next;
+        }
+        supervisor_release_lock(&keypad_scanners_linked_list_lock);
+    }
 }
 
 void keypad_reset(void) {
-    keypad_keys_reset();
-    keypad_keymatrix_reset();
+    if (MP_STATE_VM(keypad_scanners_linked_list)) {
+        supervisor_disable_tick();
+    }
+
+    MP_STATE_VM(keypad_scanners_linked_list) = NULL;
+    keypad_scanners_linked_list_lock = false;
+}
+
+// Register a Keys, KeyMatrix, etc. that will be scanned in the background
+void keypad_register_scanner(keypad_scanner_obj_t *scanner) {
+    supervisor_acquire_lock(&keypad_scanners_linked_list_lock);
+    scanner->next = MP_STATE_VM(keypad_scanners_linked_list);
+    MP_STATE_VM(keypad_scanners_linked_list) = scanner;
+    supervisor_release_lock(&keypad_scanners_linked_list_lock);
+}
+
+// Remove scanner from the list of active scanners.
+void keypad_deregister_scanner(keypad_scanner_obj_t *scanner) {
+    supervisor_acquire_lock(&keypad_scanners_linked_list_lock);
+    if (MP_STATE_VM(keypad_scanners_linked_list) == scanner) {
+        // Scanner is at the front; splice it out.
+        MP_STATE_VM(keypad_scanners_linked_list) = scanner->next;
+        scanner->next = NULL;
+    } else {
+        keypad_scanner_obj_t *current = MP_STATE_VM(keypad_scanners_linked_list);
+        while (current) {
+            if (current->next == scanner) {
+                // Splice myself out.
+                current->next = scanner->next;
+                scanner->next = NULL;
+                break;
+            }
+            current = current->next;
+        }
+    }
+    supervisor_release_lock(&keypad_scanners_linked_list_lock);
 }

shared-module/keypad/__init__.h

@@ -27,7 +27,21 @@
 #ifndef SHARED_MODULE_KEYPAD_H
 #define SHARED_MODULE_KEYPAD_H
 
+#include "py/obj.h"
+
+// All scanners must have a next field immediately following base.
+// This is an ad hoc "superclass" struct for scanners, though they do
+// not actually have a superclass relationship.
+typedef struct _keypad_scanner_obj_t {
+    mp_obj_base_t base;
+    struct _keypad_scanner_obj_t *next;
+} keypad_scanner_obj_t;
+
 void keypad_tick(void);
 void keypad_reset(void);
 
+void keypad_register_scanner(keypad_scanner_obj_t *scanner);
+void keypad_deregister_scanner(keypad_scanner_obj_t *scanner);
+
+
 #endif // SHARED_MODULE_KEYPAD_H