From 111040fe32120ea3f24f679c8683ac2b6134b5eb Mon Sep 17 00:00:00 2001 From: Patrick Surry Date: Tue, 24 Dec 2024 15:37:13 -0500 Subject: [PATCH 1/4] fix: Add delay to DemuxKeyMatrix --- shared-module/keypad_demux/DemuxKeyMatrix.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/shared-module/keypad_demux/DemuxKeyMatrix.c b/shared-module/keypad_demux/DemuxKeyMatrix.c index 42a0a0e68cc1..079f4b3fe372 100644 --- a/shared-module/keypad_demux/DemuxKeyMatrix.c +++ b/shared-module/keypad_demux/DemuxKeyMatrix.c @@ -7,6 +7,7 @@ #include #include "py/gc.h" +#include "py/mphal.h" #include "py/runtime.h" #include "shared-bindings/digitalio/DigitalInOut.h" #include "shared-bindings/keypad/EventQueue.h" @@ -104,7 +105,11 @@ static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { keypad_demux_demuxkeymatrix_obj_t *self = self_in; for (size_t row = 0; row < common_hal_keypad_demux_demuxkeymatrix_get_row_count(self); row++) { - // Set the row address on demultiplexer + // Set the row address on the demultiplexer. This currently assumes an inverting + // demux like the 74hc138 which outputs low on the selected line and high on non-selected ones. + // This class should probably support a columns_to_anodes parameter where this case would + // be True (the row gets pulled low), and a non-inverting demux like 74hc238 would + // set columns_to_anodes=False. size_t mask = 0b00000001; for (size_t row_addr_pin = 0; row_addr_pin < self->row_addr_digitalinouts->len; row_addr_pin++) { digitalio_digitalinout_obj_t *dio = self->row_addr_digitalinouts->items[row_addr_pin]; @@ -112,11 +117,19 @@ static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { mask = mask << 1; } + // Wait a moment to let the columns settle. + // The normal KeyMatrix uses a 1us delay but that still gave echoes on my + // nullbitsco nibble 65% (16 x 5 matrix). For example when key (row, col) is pressed + // both (row, col) and (row+1, col) (and sometimes row+2) are registered, + // especially when row+1 is a power of 2 (all mux bits change) and col is 0. + // The QMK implementation for this keyboard uses a 5us delay which works here too + mp_hal_delay_us(5); + for (size_t column = 0; column < common_hal_keypad_demux_demuxkeymatrix_get_column_count(self); column++) { mp_uint_t key_number = row_column_to_key_number(self, row, column); // Get the current state, by reading whether the column got pulled to the row value or not. - // If low, the key is pressed. + // (with a columns_to_anodes parameter this could mimic the KeyMatrix code) const bool current = !common_hal_digitalio_digitalinout_get_value(self->column_digitalinouts->items[column]); // Record any transitions. From fdb38ccc1103ab7e9217754b78749ae4a7a2bdf2 Mon Sep 17 00:00:00 2001 From: Patrick Surry Date: Wed, 25 Dec 2024 13:29:47 -0500 Subject: [PATCH 2/4] wip: include transpose and columns_to_anodes options --- shared-bindings/keypad_demux/DemuxKeyMatrix.c | 19 ++++++++- shared-bindings/keypad_demux/DemuxKeyMatrix.h | 2 +- shared-bindings/keypad_demux/__init__.c | 10 ++++- shared-module/keypad_demux/DemuxKeyMatrix.c | 39 ++++++++++++------- shared-module/keypad_demux/DemuxKeyMatrix.h | 2 + 5 files changed, 54 insertions(+), 18 deletions(-) diff --git a/shared-bindings/keypad_demux/DemuxKeyMatrix.c b/shared-bindings/keypad_demux/DemuxKeyMatrix.c index 8c867aecee08..9572a64790b7 100644 --- a/shared-bindings/keypad_demux/DemuxKeyMatrix.c +++ b/shared-bindings/keypad_demux/DemuxKeyMatrix.c @@ -36,6 +36,8 @@ //| self, //| row_addr_pins: Sequence[microcontroller.Pin], //| column_pins: Sequence[microcontroller.Pin], +//| columns_to_anodes: bool = True, +//| transpose: bool = False, //| interval: float = 0.020, //| max_events: int = 64, //| debounce_threshold: int = 1, @@ -51,7 +53,18 @@ //| An `keypad.EventQueue` is created when this object is created and is available in the `events` attribute. //| //| :param Sequence[microcontroller.Pin] row_addr_pins: The pins attached to the rows demultiplexer. +//| If your columns are multiplexed, set ``transpose`` to ``True``. //| :param Sequence[microcontroller.Pin] column_pins: The pins attached to the columns. +//| :param bool columns_to_anodes: Default ``True``. +//| If the matrix uses diodes, the diode anodes are typically connected to the column pins +//| with the cathodes connected to the row pins. This implies an inverting multiplexer that drives +//| the selected row pin low. If your diodes are reversed, with a non-inverting multiplexer +//| that drives the selected row high, set ``columns_to_anodes`` to ``False``. +//| If ``transpose`` is ``True`` the sense of columns and rows are reversed here. +//| :param bool transpose: Default ``False``. +//| If your matrix is multiplexed on columns rather than rows, set ``transpose`` to ``True``. +//| This swaps the meaning of ``row_addr_pins`` to ``column_addr_pins``; +//| ``column_pins`` to ``row_pins``; and ``columns_to_anodes`` to ``rows_to_anodes``. //| :param float interval: Scan keys no more often than ``interval`` to allow for debouncing. //| ``interval`` is in float seconds. The default is 0.020 (20 msecs). //| :param int max_events: maximum size of `events` `keypad.EventQueue`: @@ -67,10 +80,12 @@ static mp_obj_t keypad_demux_demuxkeymatrix_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { keypad_demux_demuxkeymatrix_obj_t *self = mp_obj_malloc(keypad_demux_demuxkeymatrix_obj_t, &keypad_demux_demuxkeymatrix_type); - enum { ARG_row_addr_pins, ARG_column_pins, ARG_interval, ARG_max_events, ARG_debounce_threshold }; + enum { ARG_row_addr_pins, ARG_column_pins, ARG_columns_to_anodes, ARG_transpose, ARG_interval, ARG_max_events, ARG_debounce_threshold }; static const mp_arg_t allowed_args[] = { { MP_QSTR_row_addr_pins, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_column_pins, MP_ARG_REQUIRED | MP_ARG_OBJ }, + { MP_QSTR_columns_to_anodes, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} }, + { MP_QSTR_transpose, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, { MP_QSTR_interval, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, { MP_QSTR_max_events, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 64} }, { MP_QSTR_debounce_threshold, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 1} }, @@ -107,7 +122,7 @@ static mp_obj_t keypad_demux_demuxkeymatrix_make_new(const mp_obj_type_t *type, column_pins_array[column] = pin; } - common_hal_keypad_demux_demuxkeymatrix_construct(self, num_row_addr_pins, row_addr_pins_array, num_column_pins, column_pins_array, interval, max_events, debounce_threshold); + common_hal_keypad_demux_demuxkeymatrix_construct(self, num_row_addr_pins, row_addr_pins_array, num_column_pins, column_pins_array, args[ARG_columns_to_anodes].u_bool, args[ARG_transpose].u_bool, interval, max_events, debounce_threshold); return MP_OBJ_FROM_PTR(self); } diff --git a/shared-bindings/keypad_demux/DemuxKeyMatrix.h b/shared-bindings/keypad_demux/DemuxKeyMatrix.h index fc7a11a0d37f..af6f2e346f24 100644 --- a/shared-bindings/keypad_demux/DemuxKeyMatrix.h +++ b/shared-bindings/keypad_demux/DemuxKeyMatrix.h @@ -11,7 +11,7 @@ extern const mp_obj_type_t keypad_demux_demuxkeymatrix_type; -void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], mp_float_t interval, size_t max_events, uint8_t debounce_threshold); +void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], bool columns_to_anodes, bool transpose, mp_float_t interval, size_t max_events, uint8_t debounce_threshold); void common_hal_keypad_demux_demuxkeymatrix_deinit(keypad_demux_demuxkeymatrix_obj_t *self); diff --git a/shared-bindings/keypad_demux/__init__.c b/shared-bindings/keypad_demux/__init__.c index c1359b251592..30ef1234823e 100644 --- a/shared-bindings/keypad_demux/__init__.c +++ b/shared-bindings/keypad_demux/__init__.c @@ -11,8 +11,14 @@ //| """Support for scanning key matrices that use a demultiplexer //| -//| The `keypad_demux` module provides native support to scan sets of keys or buttons, -//| connected in a row-and-column matrix. +//| The `keypad_demux` module provides native support to scan a matrix of keys or buttons +//| where either the row or column axis is controlled by a demultiplexer or decoder IC +//| such as the 74LS138 or 74LS238. In this arrangement a binary input value +//| determines which column (or row) to select, thereby reducing the number of input pins. +//| For example the input 101 would select line 5 in the matrix. +//| Set ``columns_to_anodes`` to ``False`` with a non-inverting demultiplexer +//| which drives the selected line high. +//| Set ``transpose`` to ``True`` if columns are multiplexed rather than rows. //| //| .. jinja //| """ diff --git a/shared-module/keypad_demux/DemuxKeyMatrix.c b/shared-module/keypad_demux/DemuxKeyMatrix.c index 079f4b3fe372..7ace33120bf3 100644 --- a/shared-module/keypad_demux/DemuxKeyMatrix.c +++ b/shared-module/keypad_demux/DemuxKeyMatrix.c @@ -27,11 +27,15 @@ static keypad_scanner_funcs_t keymatrix_funcs = { }; static mp_uint_t row_column_to_key_number(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t row, mp_uint_t column) { - return row * self->column_digitalinouts->len + column; + // return the key index in the user's coordinate system + return row * common_hal_keypad_demux_demuxkeymatrix_get_column_count(self) + column; } -void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], mp_float_t interval, size_t max_events, uint8_t debounce_threshold) { +void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], bool columns_to_anodes, bool transpose, mp_float_t interval, size_t max_events, uint8_t debounce_threshold) { + // the multiplexed pins are outputs so we can set the address for the target row + // the sense of the address pins themselves doesn't change with columns_to_anodes + // but the value output on the selected row line will be !columns_to_anodes mp_obj_t row_addr_dios[num_row_addr_pins]; for (size_t row = 0; row < num_row_addr_pins; row++) { digitalio_digitalinout_obj_t *dio = @@ -42,17 +46,20 @@ void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatri } self->row_addr_digitalinouts = mp_obj_new_tuple(num_row_addr_pins, row_addr_dios); + // the column pins are always inputs, with default state based on columns_to_anodes mp_obj_t column_dios[num_column_pins]; for (size_t column = 0; column < num_column_pins; column++) { digitalio_digitalinout_obj_t *dio = mp_obj_malloc(digitalio_digitalinout_obj_t, &digitalio_digitalinout_type); dio->base.type = &digitalio_digitalinout_type; common_hal_digitalio_digitalinout_construct(dio, column_pins[column]); - common_hal_digitalio_digitalinout_switch_to_input(dio, PULL_UP); + common_hal_digitalio_digitalinout_switch_to_input(dio, columns_to_anodes ? PULL_UP : PULL_DOWN); column_dios[column] = dio; } self->column_digitalinouts = mp_obj_new_tuple(num_column_pins, column_dios); + self->columns_to_anodes = columns_to_anodes; + self->transpose = transpose; self->funcs = &keymatrix_funcs; keypad_construct_common((keypad_scanner_obj_t *)self, interval, max_events, debounce_threshold); @@ -79,11 +86,15 @@ void common_hal_keypad_demux_demuxkeymatrix_deinit(keypad_demux_demuxkeymatrix_o } size_t common_hal_keypad_demux_demuxkeymatrix_get_row_count(keypad_demux_demuxkeymatrix_obj_t *self) { - return 1 << self->row_addr_digitalinouts->len; + return self->transpose + ? self->column_digitalinouts->len + : (size_t)(1 << self->row_addr_digitalinouts->len); } size_t common_hal_keypad_demux_demuxkeymatrix_get_column_count(keypad_demux_demuxkeymatrix_obj_t *self) { - return self->column_digitalinouts->len; + return self->transpose + ? (size_t)(1 << self->row_addr_digitalinouts->len) + : self->column_digitalinouts->len; } mp_uint_t common_hal_keypad_demux_demuxkeymatrix_row_column_to_key_number(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t row, mp_uint_t column) { @@ -105,11 +116,11 @@ static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { keypad_demux_demuxkeymatrix_obj_t *self = self_in; for (size_t row = 0; row < common_hal_keypad_demux_demuxkeymatrix_get_row_count(self); row++) { - // Set the row address on the demultiplexer. This currently assumes an inverting - // demux like the 74hc138 which outputs low on the selected line and high on non-selected ones. - // This class should probably support a columns_to_anodes parameter where this case would - // be True (the row gets pulled low), and a non-inverting demux like 74hc238 would - // set columns_to_anodes=False. + // Set the row address on the demultiplexer. + // When columns_to_anodes is True (default) we've got an inverting demux + // so the selected line is pulled low, and we look for rows that get pulled low. + // When columns_to_anodes is False we do the reverse. + // We can safely ignore transposition since it's handled by row_column_to_key_number size_t mask = 0b00000001; for (size_t row_addr_pin = 0; row_addr_pin < self->row_addr_digitalinouts->len; row_addr_pin++) { digitalio_digitalinout_obj_t *dio = self->row_addr_digitalinouts->items[row_addr_pin]; @@ -128,9 +139,11 @@ static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { for (size_t column = 0; column < common_hal_keypad_demux_demuxkeymatrix_get_column_count(self); column++) { mp_uint_t key_number = row_column_to_key_number(self, row, column); - // Get the current state, by reading whether the column got pulled to the row value or not. - // (with a columns_to_anodes parameter this could mimic the KeyMatrix code) - const bool current = !common_hal_digitalio_digitalinout_get_value(self->column_digitalinouts->items[column]); + // Get the current state, by reading whether the column got pulled to the row value or not, + // which is the opposite of columns_to_anodes. + const bool current = + common_hal_digitalio_digitalinout_get_value(self->column_digitalinouts->items[column]) != + self->columns_to_anodes; // Record any transitions. if (keypad_debounce((keypad_scanner_obj_t *)self, key_number, current)) { diff --git a/shared-module/keypad_demux/DemuxKeyMatrix.h b/shared-module/keypad_demux/DemuxKeyMatrix.h index 45afa93a8442..fe315775dd21 100644 --- a/shared-module/keypad_demux/DemuxKeyMatrix.h +++ b/shared-module/keypad_demux/DemuxKeyMatrix.h @@ -17,6 +17,8 @@ typedef struct { KEYPAD_SCANNER_COMMON_FIELDS; mp_obj_tuple_t *row_addr_digitalinouts; mp_obj_tuple_t *column_digitalinouts; + bool columns_to_anodes; + bool transpose; } keypad_demux_demuxkeymatrix_obj_t; void keypad_demux_demuxkeymatrix_scan(keypad_demux_demuxkeymatrix_obj_t *self); From 029a29a77087e5ecedb99c48e40aff9f2f6b6329 Mon Sep 17 00:00:00 2001 From: Patrick Surry Date: Wed, 25 Dec 2024 17:37:04 -0500 Subject: [PATCH 3/4] fix failing test --- ports/espressif/boards/m5stack_cardputer/cardputer_keyboard.c | 2 ++ shared-bindings/keypad_demux/DemuxKeyMatrix.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ports/espressif/boards/m5stack_cardputer/cardputer_keyboard.c b/ports/espressif/boards/m5stack_cardputer/cardputer_keyboard.c index 4bb7dd7c42ba..73880f66e19d 100644 --- a/ports/espressif/boards/m5stack_cardputer/cardputer_keyboard.c +++ b/ports/espressif/boards/m5stack_cardputer/cardputer_keyboard.c @@ -110,6 +110,8 @@ void cardputer_keyboard_init(void) { row_addr_pins, // row_addr_pins 7, // num_column_pins column_pins, // column_pins + true, // columns_to_anodes + false, // transpose 0.01f, // interval 20, // max_events 2 // debounce_threshold diff --git a/shared-bindings/keypad_demux/DemuxKeyMatrix.h b/shared-bindings/keypad_demux/DemuxKeyMatrix.h index af6f2e346f24..8bdaa597dc03 100644 --- a/shared-bindings/keypad_demux/DemuxKeyMatrix.h +++ b/shared-bindings/keypad_demux/DemuxKeyMatrix.h @@ -11,7 +11,7 @@ extern const mp_obj_type_t keypad_demux_demuxkeymatrix_type; -void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], bool columns_to_anodes, bool transpose, mp_float_t interval, size_t max_events, uint8_t debounce_threshold); +void common_hal_keypad_demux_demuxkeymatrix_construct(keypad_demux_demuxkeymatrix_obj_t *self, mp_uint_t num_row_addr_pins, const mcu_pin_obj_t *row_addr_pins[], mp_uint_t num_column_pins, const mcu_pin_obj_t *column_pins[], bool columns_to_anodes, bool transpose, mp_float_t interval, size_t max_events, uint8_t debounce_threshold); void common_hal_keypad_demux_demuxkeymatrix_deinit(keypad_demux_demuxkeymatrix_obj_t *self); From e185c28c590072babdce0c46beb2eaefec5ba20d Mon Sep 17 00:00:00 2001 From: Patrick Surry Date: Thu, 26 Dec 2024 10:04:21 -0500 Subject: [PATCH 4/4] Fix transpose logic --- shared-module/keypad_demux/DemuxKeyMatrix.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/shared-module/keypad_demux/DemuxKeyMatrix.c b/shared-module/keypad_demux/DemuxKeyMatrix.c index 7ace33120bf3..b90669d772fc 100644 --- a/shared-module/keypad_demux/DemuxKeyMatrix.c +++ b/shared-module/keypad_demux/DemuxKeyMatrix.c @@ -114,13 +114,13 @@ static size_t demuxkeymatrix_get_key_count(void *self_in) { static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { keypad_demux_demuxkeymatrix_obj_t *self = self_in; - - for (size_t row = 0; row < common_hal_keypad_demux_demuxkeymatrix_get_row_count(self); row++) { + // We always scan through the multiplexed lines using the array sizes directly + // and apply transposition only when translating to key number. + for (int row = 0; row < (1 << self->row_addr_digitalinouts->len); row++) { // Set the row address on the demultiplexer. // When columns_to_anodes is True (default) we've got an inverting demux // so the selected line is pulled low, and we look for rows that get pulled low. // When columns_to_anodes is False we do the reverse. - // We can safely ignore transposition since it's handled by row_column_to_key_number size_t mask = 0b00000001; for (size_t row_addr_pin = 0; row_addr_pin < self->row_addr_digitalinouts->len; row_addr_pin++) { digitalio_digitalinout_obj_t *dio = self->row_addr_digitalinouts->items[row_addr_pin]; @@ -136,8 +136,10 @@ static void demuxkeymatrix_scan_now(void *self_in, mp_obj_t timestamp) { // The QMK implementation for this keyboard uses a 5us delay which works here too mp_hal_delay_us(5); - for (size_t column = 0; column < common_hal_keypad_demux_demuxkeymatrix_get_column_count(self); column++) { - mp_uint_t key_number = row_column_to_key_number(self, row, column); + for (size_t column = 0; column < self->column_digitalinouts->len; column++) { + mp_uint_t key_number = self->transpose + ? row_column_to_key_number(self, column, row) + : row_column_to_key_number(self, row, column); // Get the current state, by reading whether the column got pulled to the row value or not, // which is the opposite of columns_to_anodes.