From ba26242f622504959f0357456b9ff5a70f178f4d Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Mon, 17 Jul 2023 01:16:08 +1000 Subject: [PATCH 01/18] Add encoder abstraction. --- builddefs/common_features.mk | 15 + drivers/encoder/encoder_graycode.c | 198 ++++++++++ keyboards/mechwild/sugarglider/matrix.c | 5 +- keyboards/pica40/rev2/post_rules.mk | 8 - keyboards/pica40/rev2/rev2.c | 98 +---- keyboards/ploopyco/trackball_thumb/info.json | 7 +- .../ploopyco/trackball_thumb/post_rules.mk | 4 - keyboards/ploopyco/trackball_thumb/rules.mk | 1 + .../trackball_thumb/trackball_thumb.c | 20 +- .../trackball_thumb/trackball_thumb.h | 3 - quantum/encoder.c | 338 +++++------------- quantum/encoder.h | 87 +++-- quantum/encoder/tests/config_encoder_common.h | 6 + quantum/encoder/tests/config_mock.h | 1 + .../tests/config_mock_split_left_eq_right.h | 1 + .../tests/config_mock_split_left_gt_right.h | 1 + .../tests/config_mock_split_left_lt_right.h | 1 + .../encoder/tests/config_mock_split_no_left.h | 1 + .../tests/config_mock_split_no_right.h | 1 + .../encoder/tests/config_mock_split_role.h | 1 + quantum/encoder/tests/encoder_tests.cpp | 2 +- .../encoder_tests_split_left_eq_right.cpp | 91 ++++- .../encoder_tests_split_left_gt_right.cpp | 89 ++++- .../encoder_tests_split_left_lt_right.cpp | 89 ++++- .../tests/encoder_tests_split_no_left.cpp | 69 ++-- .../tests/encoder_tests_split_no_right.cpp | 66 ++-- .../tests/encoder_tests_split_role.cpp | 8 +- quantum/encoder/tests/mock_split.c | 4 - quantum/encoder/tests/mock_split.h | 3 - quantum/encoder/tests/rules.mk | 7 + quantum/keyboard.c | 2 +- quantum/split_common/transaction_id_define.h | 1 + quantum/split_common/transactions.c | 26 +- quantum/split_common/transport.h | 4 +- 34 files changed, 737 insertions(+), 521 deletions(-) create mode 100644 drivers/encoder/encoder_graycode.c delete mode 100644 keyboards/pica40/rev2/post_rules.mk delete mode 100644 keyboards/ploopyco/trackball_thumb/post_rules.mk create mode 100644 quantum/encoder/tests/config_encoder_common.h diff --git a/builddefs/common_features.mk b/builddefs/common_features.mk index 5687201d61c0..18a524875581 100644 --- a/builddefs/common_features.mk +++ b/builddefs/common_features.mk @@ -924,9 +924,24 @@ ifeq ($(strip $(BLUETOOTH_ENABLE)), yes) endif endif +ENCODER_ENABLE ?= no +ENCODER_DRIVER ?= graycode +VALID_ENCODER_DRIVER_TYPES := graycode custom ifeq ($(strip $(ENCODER_ENABLE)), yes) + ifeq ($(filter $(ENCODER_DRIVER),$(VALID_ENCODER_DRIVER_TYPES)),) + $(call CATASTROPHIC_ERROR,Invalid ENCODER_DRIVER,ENCODER_DRIVER="$(ENCODER_DRIVER)" is not a valid encoder driver) + endif SRC += $(QUANTUM_DIR)/encoder.c OPT_DEFS += -DENCODER_ENABLE + OPT_DEFS += -DENCODER_DRIVER_$(strip $(shell echo $(ENCODER_DRIVER) | tr '[:lower:]' '[:upper:]')) + + COMMON_VPATH += $(PLATFORM_PATH)/$(PLATFORM_KEY)/$(DRIVER_DIR)/encoder + COMMON_VPATH += $(DRIVER_PATH)/encoder + + ifneq ($(strip $(ENCODER_DRIVER)), custom) + SRC += encoder_$(strip $(ENCODER_DRIVER)).c + endif + ifeq ($(strip $(ENCODER_MAP_ENABLE)), yes) OPT_DEFS += -DENCODER_MAP_ENABLE endif diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c new file mode 100644 index 000000000000..8e716ddc7fc0 --- /dev/null +++ b/drivers/encoder/encoder_graycode.c @@ -0,0 +1,198 @@ +/* + * Copyright 2018 Jack Humbert + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +#include "encoder.h" +#include "gpio.h" +#include "keyboard.h" +#include "action.h" +#include "keycodes.h" +#include "wait.h" + +#ifdef SPLIT_KEYBOARD +# include "split_util.h" +#endif + +// for memcpy +#include + +#if !defined(ENCODER_RESOLUTIONS) && !defined(ENCODER_RESOLUTION) +# define ENCODER_RESOLUTION 4 +#endif + +#if !defined(ENCODERS_PAD_A) || !defined(ENCODERS_PAD_B) +# error "No encoder pads defined by ENCODERS_PAD_A and ENCODERS_PAD_B" +#endif + +extern volatile bool isLeftHand; + +static pin_t encoders_pad_a[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_A; +static pin_t encoders_pad_b[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_B; + +#ifdef ENCODER_RESOLUTIONS +static uint8_t encoder_resolutions[NUM_ENCODERS] = ENCODER_RESOLUTIONS; +#endif + +#ifndef ENCODER_DIRECTION_FLIP +# define ENCODER_CLOCKWISE true +# define ENCODER_COUNTER_CLOCKWISE false +#else +# define ENCODER_CLOCKWISE false +# define ENCODER_COUNTER_CLOCKWISE true +#endif +static int8_t encoder_LUT[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0}; + +static uint8_t encoder_state[NUM_ENCODERS] = {0}; +static int8_t encoder_pulses[NUM_ENCODERS] = {0}; + +// encoder counts +static uint8_t thisCount; +#ifdef SPLIT_KEYBOARD +// encoder offsets for each hand +static uint8_t thisHand, thatHand; +// encoder counts for each hand +static uint8_t thatCount; +#endif + +static uint8_t encoder_value[NUM_ENCODERS] = {0}; + +__attribute__((weak)) void encoder_wait_pullup_charge(void) { + wait_us(100); +} + +__attribute__((weak)) void encoder_init_pin(uint8_t index, bool pad_b) { + setPinInputHigh(pad_b ? encoders_pad_b[index] : encoders_pad_a[index]); +} + +__attribute__((weak)) uint8_t encoder_read_pin(uint8_t index, bool pad_b) { + return readPin(pad_b ? encoders_pad_b[index] : encoders_pad_a[index]) ? 1 : 0; +} + +void encoder_driver_init(void) { +#ifdef SPLIT_KEYBOARD + thisHand = isLeftHand ? 0 : NUM_ENCODERS_LEFT; + thatHand = NUM_ENCODERS_LEFT - thisHand; + thisCount = isLeftHand ? NUM_ENCODERS_LEFT : NUM_ENCODERS_RIGHT; + thatCount = isLeftHand ? NUM_ENCODERS_RIGHT : NUM_ENCODERS_LEFT; +#else // SPLIT_KEYBOARD + thisCount = NUM_ENCODERS; +#endif + +#ifdef ENCODER_TESTS + // Annoying that we have to clear out values during initialisation here, but + // because all the arrays are static locals, rerunning tests in the same + // executable doesn't reset any of these. Kinda crappy having test-only code + // here, but it's the simplest solution. + memset(encoder_value, 0, sizeof(encoder_value)); + memset(encoder_state, 0, sizeof(encoder_state)); + memset(encoder_pulses, 0, sizeof(encoder_pulses)); + static const pin_t encoders_pad_a_left[] = ENCODERS_PAD_A; + static const pin_t encoders_pad_b_left[] = ENCODERS_PAD_B; + for (uint8_t i = 0; i < thisCount; i++) { + encoders_pad_a[i] = encoders_pad_a_left[i]; + encoders_pad_b[i] = encoders_pad_b_left[i]; + } +#endif + +#if defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) + // Re-initialise the pads if it's the right-hand side + if (!isLeftHand) { + static const pin_t encoders_pad_a_right[] = ENCODERS_PAD_A_RIGHT; + static const pin_t encoders_pad_b_right[] = ENCODERS_PAD_B_RIGHT; + for (uint8_t i = 0; i < thisCount; i++) { + encoders_pad_a[i] = encoders_pad_a_right[i]; + encoders_pad_b[i] = encoders_pad_b_right[i]; + } + } +#endif // defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) + + // Encoder resolutions is handled purely master-side, so concatenate the two arrays +#if defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) +# if defined(ENCODER_RESOLUTIONS_RIGHT) + static const uint8_t encoder_resolutions_right[NUM_ENCODERS_RIGHT] = ENCODER_RESOLUTIONS_RIGHT; +# else // defined(ENCODER_RESOLUTIONS_RIGHT) + static const uint8_t encoder_resolutions_right[NUM_ENCODERS_RIGHT] = ENCODER_RESOLUTIONS; +# endif // defined(ENCODER_RESOLUTIONS_RIGHT) + for (uint8_t i = 0; i < NUM_ENCODERS_RIGHT; i++) { + encoder_resolutions[NUM_ENCODERS_LEFT + i] = encoder_resolutions_right[i]; + } +#endif // defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) + + for (uint8_t i = 0; i < thisCount; i++) { + encoder_init_pin(i, false); + encoder_init_pin(i, true); + } + encoder_wait_pullup_charge(); + for (uint8_t i = 0; i < thisCount; i++) { + encoder_state[i] = (encoder_read_pin(i, false) << 0) | (encoder_read_pin(i, true) << 1); + } +} + +static bool encoder_handle_state_change(uint8_t index, uint8_t state) { + bool changed = false; + uint8_t i = index; + +#ifdef ENCODER_RESOLUTIONS + const uint8_t resolution = encoder_resolutions[i]; +#else + const uint8_t resolution = ENCODER_RESOLUTION; +#endif + +#ifdef SPLIT_KEYBOARD + index += thisHand; +#endif + encoder_pulses[i] += encoder_LUT[state & 0xF]; + +#ifdef ENCODER_DEFAULT_POS + if ((encoder_pulses[i] >= resolution) || (encoder_pulses[i] <= -resolution) || ((state & 0x3) == ENCODER_DEFAULT_POS)) { + if (encoder_pulses[i] >= 1) { +#else + if (encoder_pulses[i] >= resolution) { +#endif + + encoder_value[index]++; + changed = true; + encoder_queue_event(index, ENCODER_COUNTER_CLOCKWISE); + } + +#ifdef ENCODER_DEFAULT_POS + if (encoder_pulses[i] <= -1) { +#else + if (encoder_pulses[i] <= -resolution) { // direction is arbitrary here, but this clockwise +#endif + encoder_value[index]--; + changed = true; + encoder_queue_event(index, ENCODER_CLOCKWISE); + } + encoder_pulses[i] %= resolution; +#ifdef ENCODER_DEFAULT_POS + encoder_pulses[i] = 0; + } +#endif + return changed; +} + +void encoder_driver_task(void) { + for (uint8_t i = 0; i < thisCount; i++) { + uint8_t new_status = (encoder_read_pin(i, false) << 0) | (encoder_read_pin(i, true) << 1); + if ((encoder_state[i] & 0x3) != new_status) { + encoder_state[i] <<= 2; + encoder_state[i] |= new_status; + encoder_handle_state_change(i, encoder_state[i]); + } + } +} diff --git a/keyboards/mechwild/sugarglider/matrix.c b/keyboards/mechwild/sugarglider/matrix.c index 96a16df542a3..994cb0c54436 100644 --- a/keyboards/mechwild/sugarglider/matrix.c +++ b/keyboards/mechwild/sugarglider/matrix.c @@ -50,7 +50,7 @@ static void select_row(uint8_t row) { //wait_us(100); return; } - + if (row > 1) { mcp23018_errors += !mcp23018_set_config(I2C_ADDR, mcp23018_PORTB, ALL_INPUT); mcp23018_errors += !mcp23018_set_config(I2C_ADDR, mcp23018_PORTA, ~(row_pos[row])); @@ -87,9 +87,6 @@ bool matrix_scan_custom(matrix_row_t current_matrix[]) { bool changed = false; for (uint8_t current_row = 0; current_row < MATRIX_ROWS; current_row++) { changed |= read_cols_on_row(current_matrix, current_row); -#ifdef ENCODER_ENABLE - encoder_read(); -#endif } return changed; } diff --git a/keyboards/pica40/rev2/post_rules.mk b/keyboards/pica40/rev2/post_rules.mk deleted file mode 100644 index e4dda1925baa..000000000000 --- a/keyboards/pica40/rev2/post_rules.mk +++ /dev/null @@ -1,8 +0,0 @@ -# if ENCODER_ENABLE is set, add defines but avoid adding encoder.c as it's replaced by custom code in rev2.c -ifeq ($(strip $(ENCODER_ENABLE)), yes) - ENCODER_ENABLE := no - OPT_DEFS += -DENCODER_ENABLE - ifeq ($(strip $(ENCODER_MAP_ENABLE)), yes) - OPT_DEFS += -DENCODER_MAP_ENABLE - endif -endif diff --git a/keyboards/pica40/rev2/rev2.c b/keyboards/pica40/rev2/rev2.c index c585ec56d665..d05421150453 100644 --- a/keyboards/pica40/rev2/rev2.c +++ b/keyboards/pica40/rev2/rev2.c @@ -2,99 +2,29 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "rev2.h" +#include "gpio.h" #ifdef ENCODER_ENABLE // code based on encoder.c -static const pin_t encoders_pad_a[] = ENCODERS_PAD_A; -static const pin_t encoders_pad_b[] = ENCODERS_PAD_B; - -static int8_t encoder_LUT[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0}; -static uint8_t encoder_state = 3; -static int8_t encoder_pulses = 0; -static uint8_t encoder_value = 0; - -typedef struct encoder_sync_data { - int value; -} encoder_sync_data; +#define ENCODER_PIN_A (((pin_t[])ENCODERS_PAD_A)[0]) +#define ENCODER_PIN_B (((pin_t[])ENCODERS_PAD_B)[0]) // custom handler that returns encoder B pin status from slave side void encoder_sync_slave_handler(uint8_t in_buflen, const void *in_data, uint8_t out_buflen, void *out_data) { - encoder_sync_data *data = (encoder_sync_data *)out_data; - data->value = readPin(encoders_pad_b[0]); -} - -__attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { - return true; + *(uint8_t *)out_data = readPin(ENCODER_PIN_B) ? 1 : 0; } -bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!encoder_update_user(index, clockwise)) return false; - - tap_code(clockwise ? KC_VOLU : KC_VOLD); +void encoder_init_pin(uint8_t index, bool pad_b) {} - return false; -} - -#ifdef ENCODER_MAP_ENABLE -static void encoder_exec_mapping(uint8_t index, bool clockwise) { - action_exec(clockwise ? ENCODER_CW_EVENT(index, true) : ENCODER_CCW_EVENT(index, true)); - wait_ms(ENCODER_MAP_KEY_DELAY); - action_exec(clockwise ? ENCODER_CW_EVENT(index, false) : ENCODER_CCW_EVENT(index, false)); - wait_ms(ENCODER_MAP_KEY_DELAY); -} -#endif // ENCODER_MAP_ENABLE - -void encoder_init(void) { - setPinInputHigh(encoders_pad_a[0]); - setPinInputHigh(encoders_pad_b[0]); - wait_us(100); - transaction_register_rpc(ENCODER_SYNC, encoder_sync_slave_handler); -} - -bool encoder_read(void) { - // ignore if running on slave side - if (!is_keyboard_master()) return false; - - bool changed = false; - encoder_sync_data data = {0}; - // request pin B status from slave side - if (transaction_rpc_recv(ENCODER_SYNC, sizeof(data), &data)) { - uint8_t new_status = (readPin(encoders_pad_a[0]) << 0) | (data.value << 1); - if ((encoder_state & 0x3) != new_status) { - encoder_state <<= 2; - encoder_state |= new_status; - encoder_pulses += encoder_LUT[encoder_state & 0xF]; - - if (encoder_pulses >= ENCODER_RESOLUTION) { - encoder_value++; - changed = true; -#ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(0, false); -#else // ENCODER_MAP_ENABLE - encoder_update_kb(0, false); -#endif // ENCODER_MAP_ENABLE - } - - if (encoder_pulses <= -ENCODER_RESOLUTION) { - encoder_value--; - changed = true; -#ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(0, true); -#else // ENCODER_MAP_ENABLE - encoder_update_kb(0, true); -#endif // ENCODER_MAP_ENABLE - } - - encoder_pulses %= ENCODER_RESOLUTION; - } +uint8_t encoder_read_pin(uint8_t index, bool pad_b) { + if(pad_b) { + uint8_t data = 0; + transaction_rpc_recv(ENCODER_SYNC, sizeof(data), &data); + return data; } - return changed; + return readPin(ENCODER_PIN_A) ? 1 : 0; } -// do not use standard split encoder transactions -void encoder_state_raw(uint8_t *slave_state) {} -void encoder_update_raw(uint8_t *slave_state) {} - #endif // ENCODER_ENABLE #ifdef PICA40_RGBLIGHT_TIMEOUT @@ -125,6 +55,12 @@ bool should_set_rgblight = false; void keyboard_post_init_kb(void) { setPinOutput(PICA40_RGB_POWER_PIN); +#ifdef ENCODER_ENABLE + setPinInputHigh(ENCODER_PIN_A); + setPinInputHigh(ENCODER_PIN_B); + transaction_register_rpc(ENCODER_SYNC, encoder_sync_slave_handler); +#endif // ENCODER_ENABLE + #ifdef PICA40_RGBLIGHT_TIMEOUT idle_timer = timer_read(); check_rgblight_timer = timer_read(); diff --git a/keyboards/ploopyco/trackball_thumb/info.json b/keyboards/ploopyco/trackball_thumb/info.json index 72e236a5bbf6..c9c81d8f7568 100644 --- a/keyboards/ploopyco/trackball_thumb/info.json +++ b/keyboards/ploopyco/trackball_thumb/info.json @@ -18,7 +18,12 @@ "mousekey": true, "nkro": true, "pointing_device": true, - "encoder": false + "encoder": true + }, + "encoder": { + "rotary": [ + {"pin_a": "NO_PIN", "pin_b": "NO_PIN"} + ] }, "layouts": { "LAYOUT": { diff --git a/keyboards/ploopyco/trackball_thumb/post_rules.mk b/keyboards/ploopyco/trackball_thumb/post_rules.mk deleted file mode 100644 index d922afd9cd76..000000000000 --- a/keyboards/ploopyco/trackball_thumb/post_rules.mk +++ /dev/null @@ -1,4 +0,0 @@ -# Force encoder to be disabled -# But enable the defines for it -ENCODER_ENABLE := no -OPT_DEFS += -DENCODER_ENABLE diff --git a/keyboards/ploopyco/trackball_thumb/rules.mk b/keyboards/ploopyco/trackball_thumb/rules.mk index 4a16c0abfbe1..6365658023de 100644 --- a/keyboards/ploopyco/trackball_thumb/rules.mk +++ b/keyboards/ploopyco/trackball_thumb/rules.mk @@ -2,6 +2,7 @@ F_CPU = 8000000 POINTING_DEVICE_DRIVER = pmw3360 +ENCODER_DRIVER = custom QUANTUM_LIB_SRC += analog.c SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball_thumb/trackball_thumb.c b/keyboards/ploopyco/trackball_thumb/trackball_thumb.c index bbc782da457c..326410cf1364 100644 --- a/keyboards/ploopyco/trackball_thumb/trackball_thumb.c +++ b/keyboards/ploopyco/trackball_thumb/trackball_thumb.c @@ -17,6 +17,7 @@ */ #include "trackball_thumb.h" +#include "encoder.h" #ifndef OPT_DEBOUNCE # define OPT_DEBOUNCE 5 // (ms) Time between scroll events @@ -57,9 +58,6 @@ uint16_t last_mid_click = 0; // Stops scrollwheel from being read if it was bool debug_encoder = false; bool is_drag_scroll = false; -// require, since core encoder.c (where is is normally defined isn't present -__attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { return true; } - bool encoder_update_kb(uint8_t index, bool clockwise) { if (!encoder_update_user(index, clockwise)) { return false; @@ -75,25 +73,25 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { return true; } -void encoder_init(void) { opt_encoder_init(); } +void encoder_driver_init(void) { opt_encoder_init(); } -bool encoder_read(void) { +void encoder_driver_task(void) { // Lovingly ripped from the Ploopy Source // If the mouse wheel was just released, do not scroll. if (timer_elapsed(last_mid_click) < SCROLL_BUTT_DEBOUNCE) { - return false; + return; } // Limit the number of scrolls per unit time. if (timer_elapsed(last_scroll) < OPT_DEBOUNCE) { - return false; + return; } // Don't scroll if the middle button is depressed. if (is_scroll_clicked) { #ifndef IGNORE_SCROLL_CLICK - return false; + return; #endif } @@ -104,10 +102,8 @@ bool encoder_read(void) { int dir = opt_encoder_handler(p1, p2); - if (dir == 0) return false; - ; - encoder_update_kb(0, dir == 1); - return true; + if (dir == 0) return; + encoder_queue_event(0, dir == 1); } report_mouse_t pointing_device_task_kb(report_mouse_t mouse_report) { diff --git a/keyboards/ploopyco/trackball_thumb/trackball_thumb.h b/keyboards/ploopyco/trackball_thumb/trackball_thumb.h index 8ee17634f110..50a71601cf20 100644 --- a/keyboards/ploopyco/trackball_thumb/trackball_thumb.h +++ b/keyboards/ploopyco/trackball_thumb/trackball_thumb.h @@ -28,9 +28,6 @@ #define OPT_ENC1_MUX 4 #define OPT_ENC2_MUX 0 -bool encoder_update_kb(uint8_t index, bool clockwise); -bool encoder_update_user(uint8_t index, bool clockwise); - typedef union { uint32_t raw; struct { diff --git a/quantum/encoder.c b/quantum/encoder.c index 7ab194ed5290..55b8952a403c 100644 --- a/quantum/encoder.c +++ b/quantum/encoder.c @@ -1,82 +1,111 @@ -/* - * Copyright 2018 Jack Humbert - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ +// Copyright 2022 Nick Brassel (@tzarc) +// SPDX-License-Identifier: GPL-2.0-or-later -#include "encoder.h" -#include "keyboard.h" +#include #include "action.h" -#include "keycodes.h" +#include "encoder.h" #include "wait.h" -#ifdef SPLIT_KEYBOARD -# include "split_util.h" -#endif - -// for memcpy -#include - #ifndef ENCODER_MAP_KEY_DELAY -# include "action.h" # define ENCODER_MAP_KEY_DELAY TAP_CODE_DELAY #endif -#if !defined(ENCODER_RESOLUTIONS) && !defined(ENCODER_RESOLUTION) -# define ENCODER_RESOLUTION 4 -#endif +__attribute__((weak)) bool should_process_encoder(void) { + return is_keyboard_master(); +} -#if !defined(ENCODERS_PAD_A) || !defined(ENCODERS_PAD_B) -# error "No encoder pads defined by ENCODERS_PAD_A and ENCODERS_PAD_B" -#endif +static encoder_events_t encoder_events; + +void encoder_init(void) { + memset(&encoder_events, 0, sizeof(encoder_events)); + encoder_driver_init(); +} -extern volatile bool isLeftHand; +static bool encoder_handle_queue(void) { + bool changed = false; + while (encoder_events.tail != encoder_events.head) { + encoder_event_t event = encoder_events.queue[encoder_events.tail]; + encoder_events.tail = (encoder_events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; -static pin_t encoders_pad_a[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_A; -static pin_t encoders_pad_b[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_B; +#ifdef ENCODER_MAP_ENABLE -#ifdef ENCODER_RESOLUTIONS -static uint8_t encoder_resolutions[NUM_ENCODERS] = ENCODER_RESOLUTIONS; -#endif + // The delays below cater for Windows and its wonderful requirements. + action_exec(event.clockwise ? MAKE_ENCODER_CW_EVENT(event.index, true) : MAKE_ENCODER_CCW_EVENT(event.index, true)); +# if ENCODER_MAP_KEY_DELAY > 0 + wait_ms(ENCODER_MAP_KEY_DELAY); +# endif // ENCODER_MAP_KEY_DELAY > 0 -#ifndef ENCODER_DIRECTION_FLIP -# define ENCODER_CLOCKWISE true -# define ENCODER_COUNTER_CLOCKWISE false -#else -# define ENCODER_CLOCKWISE false -# define ENCODER_COUNTER_CLOCKWISE true -#endif -static int8_t encoder_LUT[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0}; + action_exec(event.clockwise ? MAKE_ENCODER_CW_EVENT(event.index, false) : MAKE_ENCODER_CCW_EVENT(event.index, false)); +# if ENCODER_MAP_KEY_DELAY > 0 + wait_ms(ENCODER_MAP_KEY_DELAY); +# endif // ENCODER_MAP_KEY_DELAY > 0 -static uint8_t encoder_state[NUM_ENCODERS] = {0}; -static int8_t encoder_pulses[NUM_ENCODERS] = {0}; +#else // ENCODER_MAP_ENABLE -// encoder counts -static uint8_t thisCount; -#ifdef SPLIT_KEYBOARD -// encoder offsets for each hand -static uint8_t thisHand, thatHand; -// encoder counts for each hand -static uint8_t thatCount; -#endif + encoder_update_kb(event.index, event.clockwise ? true : false); + +#endif // ENCODER_MAP_ENABLE + + changed = true; + } + return changed; +} -static uint8_t encoder_value[NUM_ENCODERS] = {0}; +bool encoder_task(void) { + bool changed = false; + +#ifdef ENCODER_ENABLE + // Attempt to process existing encoder events in case split handling has already enqueued events + if (should_process_encoder()) { + changed |= encoder_handle_queue(); + } +#endif // ENCODER_ENABLE + + // Let the encoder driver produce events + encoder_driver_task(); + + // Process any events that were enqueued + if (should_process_encoder()) { + changed |= encoder_handle_queue(); + } + + return changed; +} + +bool encoder_queue_event(uint8_t index, bool clockwise) { + // Drop out if we're full + if ((encoder_events.head + 1) % MAX_QUEUED_ENCODER_EVENTS == encoder_events.tail) { + return false; + } + + // Append the event + encoder_event_t new_event = {.index = index, .clockwise = clockwise ? 1 : 0}; + encoder_events.queue[encoder_events.head] = new_event; + + // Increment the head index + encoder_events.head = (encoder_events.head + 1) % MAX_QUEUED_ENCODER_EVENTS; + + return true; +} -__attribute__((weak)) void encoder_wait_pullup_charge(void) { - wait_us(100); +void encoder_retrieve_events(encoder_events_t *events) { + memcpy(events, &encoder_events, sizeof(encoder_events)); } +#ifdef SPLIT_KEYBOARD +void encoder_set_tail_index(uint8_t tail_index) { + encoder_events.tail = tail_index; +} + +void encoder_handle_slave_events(encoder_events_t *events) { + while (events->tail != events->head) { + encoder_event_t event = events->queue[events->tail]; + events->tail = (events->tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + encoder_queue_event(event.index, event.clockwise ? true : false); + } +} +#endif // SPLIT_KEYBOARD + __attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { return true; } @@ -106,192 +135,3 @@ __attribute__((weak)) bool encoder_update_kb(uint8_t index, bool clockwise) { #endif // ENCODER_TESTS return res; } - -__attribute__((weak)) bool should_process_encoder(void) { - return is_keyboard_master(); -} - -void encoder_init(void) { -#ifdef SPLIT_KEYBOARD - thisHand = isLeftHand ? 0 : NUM_ENCODERS_LEFT; - thatHand = NUM_ENCODERS_LEFT - thisHand; - thisCount = isLeftHand ? NUM_ENCODERS_LEFT : NUM_ENCODERS_RIGHT; - thatCount = isLeftHand ? NUM_ENCODERS_RIGHT : NUM_ENCODERS_LEFT; -#else // SPLIT_KEYBOARD - thisCount = NUM_ENCODERS; -#endif - -#ifdef ENCODER_TESTS - // Annoying that we have to clear out values during initialisation here, but - // because all the arrays are static locals, rerunning tests in the same - // executable doesn't reset any of these. Kinda crappy having test-only code - // here, but it's the simplest solution. - memset(encoder_value, 0, sizeof(encoder_value)); - memset(encoder_state, 0, sizeof(encoder_state)); - memset(encoder_pulses, 0, sizeof(encoder_pulses)); - static const pin_t encoders_pad_a_left[] = ENCODERS_PAD_A; - static const pin_t encoders_pad_b_left[] = ENCODERS_PAD_B; - for (uint8_t i = 0; i < thisCount; i++) { - encoders_pad_a[i] = encoders_pad_a_left[i]; - encoders_pad_b[i] = encoders_pad_b_left[i]; - } -#endif - -#if defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) - // Re-initialise the pads if it's the right-hand side - if (!isLeftHand) { - static const pin_t encoders_pad_a_right[] = ENCODERS_PAD_A_RIGHT; - static const pin_t encoders_pad_b_right[] = ENCODERS_PAD_B_RIGHT; - for (uint8_t i = 0; i < thisCount; i++) { - encoders_pad_a[i] = encoders_pad_a_right[i]; - encoders_pad_b[i] = encoders_pad_b_right[i]; - } - } -#endif // defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) - - // Encoder resolutions is handled purely master-side, so concatenate the two arrays -#if defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) -# if defined(ENCODER_RESOLUTIONS_RIGHT) - static const uint8_t encoder_resolutions_right[NUM_ENCODERS_RIGHT] = ENCODER_RESOLUTIONS_RIGHT; -# else // defined(ENCODER_RESOLUTIONS_RIGHT) - static const uint8_t encoder_resolutions_right[NUM_ENCODERS_RIGHT] = ENCODER_RESOLUTIONS; -# endif // defined(ENCODER_RESOLUTIONS_RIGHT) - for (uint8_t i = 0; i < NUM_ENCODERS_RIGHT; i++) { - encoder_resolutions[NUM_ENCODERS_LEFT + i] = encoder_resolutions_right[i]; - } -#endif // defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) - - for (uint8_t i = 0; i < thisCount; i++) { - setPinInputHigh(encoders_pad_a[i]); - setPinInputHigh(encoders_pad_b[i]); - } - encoder_wait_pullup_charge(); - for (uint8_t i = 0; i < thisCount; i++) { - encoder_state[i] = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1); - } -} - -#ifdef ENCODER_MAP_ENABLE -static void encoder_exec_mapping(uint8_t index, bool clockwise) { - // The delays below cater for Windows and its wonderful requirements. - action_exec(clockwise ? MAKE_ENCODER_CW_EVENT(index, true) : MAKE_ENCODER_CCW_EVENT(index, true)); -# if ENCODER_MAP_KEY_DELAY > 0 - wait_ms(ENCODER_MAP_KEY_DELAY); -# endif // ENCODER_MAP_KEY_DELAY > 0 - - action_exec(clockwise ? MAKE_ENCODER_CW_EVENT(index, false) : MAKE_ENCODER_CCW_EVENT(index, false)); -# if ENCODER_MAP_KEY_DELAY > 0 - wait_ms(ENCODER_MAP_KEY_DELAY); -# endif // ENCODER_MAP_KEY_DELAY > 0 -} -#endif // ENCODER_MAP_ENABLE - -static bool encoder_update(uint8_t index, uint8_t state) { - bool changed = false; - uint8_t i = index; - -#ifdef ENCODER_RESOLUTIONS - const uint8_t resolution = encoder_resolutions[i]; -#else - const uint8_t resolution = ENCODER_RESOLUTION; -#endif - -#ifdef SPLIT_KEYBOARD - index += thisHand; -#endif - encoder_pulses[i] += encoder_LUT[state & 0xF]; - -#ifdef ENCODER_DEFAULT_POS - if ((encoder_pulses[i] >= resolution) || (encoder_pulses[i] <= -resolution) || ((state & 0x3) == ENCODER_DEFAULT_POS)) { - if (encoder_pulses[i] >= 1) { -#else - if (encoder_pulses[i] >= resolution) { -#endif - - encoder_value[index]++; - changed = true; -#ifdef SPLIT_KEYBOARD - if (should_process_encoder()) -#endif // SPLIT_KEYBOARD -#ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_COUNTER_CLOCKWISE); -#else // ENCODER_MAP_ENABLE - encoder_update_kb(index, ENCODER_COUNTER_CLOCKWISE); -#endif // ENCODER_MAP_ENABLE - } - -#ifdef ENCODER_DEFAULT_POS - if (encoder_pulses[i] <= -1) { -#else - if (encoder_pulses[i] <= -resolution) { // direction is arbitrary here, but this clockwise -#endif - encoder_value[index]--; - changed = true; -#ifdef SPLIT_KEYBOARD - if (should_process_encoder()) -#endif // SPLIT_KEYBOARD -#ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_CLOCKWISE); -#else // ENCODER_MAP_ENABLE - encoder_update_kb(index, ENCODER_CLOCKWISE); -#endif // ENCODER_MAP_ENABLE - } - encoder_pulses[i] %= resolution; -#ifdef ENCODER_DEFAULT_POS - encoder_pulses[i] = 0; - } -#endif - return changed; -} - -bool encoder_read(void) { - bool changed = false; - for (uint8_t i = 0; i < thisCount; i++) { - uint8_t new_status = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1); - if ((encoder_state[i] & 0x3) != new_status) { - encoder_state[i] <<= 2; - encoder_state[i] |= new_status; - changed |= encoder_update(i, encoder_state[i]); - } - } - return changed; -} - -#ifdef SPLIT_KEYBOARD -void last_encoder_activity_trigger(void); - -void encoder_state_raw(uint8_t *slave_state) { - memcpy(slave_state, &encoder_value[thisHand], sizeof(uint8_t) * thisCount); -} - -void encoder_update_raw(uint8_t *slave_state) { - bool changed = false; - for (uint8_t i = 0; i < thatCount; i++) { // Note inverted logic -- we want the opposite side - const uint8_t index = i + thatHand; - int8_t delta = slave_state[i] - encoder_value[index]; - while (delta > 0) { - delta--; - encoder_value[index]++; - changed = true; -# ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_COUNTER_CLOCKWISE); -# else // ENCODER_MAP_ENABLE - encoder_update_kb(index, ENCODER_COUNTER_CLOCKWISE); -# endif // ENCODER_MAP_ENABLE - } - while (delta < 0) { - delta++; - encoder_value[index]--; - changed = true; -# ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_CLOCKWISE); -# else // ENCODER_MAP_ENABLE - encoder_update_kb(index, ENCODER_CLOCKWISE); -# endif // ENCODER_MAP_ENABLE - } - } - - // Update the last encoder input time -- handled external to encoder_read() when we're running a split - if (changed) last_encoder_activity_trigger(); -} -#endif diff --git a/quantum/encoder.h b/quantum/encoder.h index 1cbac98cb57a..b002918e104d 100644 --- a/quantum/encoder.h +++ b/quantum/encoder.h @@ -22,45 +22,76 @@ #include "gpio.h" #include "util.h" +#ifdef ENCODER_ENABLE + +__attribute__((weak)) bool should_process_encoder(void); + void encoder_init(void); -bool encoder_read(void); +bool encoder_task(void); +bool encoder_queue_event(uint8_t index, bool clockwise); bool encoder_update_kb(uint8_t index, bool clockwise); bool encoder_update_user(uint8_t index, bool clockwise); -#ifdef SPLIT_KEYBOARD +# ifdef SPLIT_KEYBOARD + +# if defined(ENCODERS_PAD_A_RIGHT) +# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# define NUM_ENCODERS_RIGHT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A_RIGHT)) +# else +# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# define NUM_ENCODERS_RIGHT NUM_ENCODERS_LEFT +# endif +# define NUM_ENCODERS (NUM_ENCODERS_LEFT + NUM_ENCODERS_RIGHT) + +# else // SPLIT_KEYBOARD -void encoder_state_raw(uint8_t* slave_state); -void encoder_update_raw(uint8_t* slave_state); +# define NUM_ENCODERS ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# define NUM_ENCODERS_LEFT NUM_ENCODERS +# define NUM_ENCODERS_RIGHT 0 -# if defined(ENCODERS_PAD_A_RIGHT) -# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) -# define NUM_ENCODERS_RIGHT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A_RIGHT)) -# else -# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) -# define NUM_ENCODERS_RIGHT NUM_ENCODERS_LEFT -# endif -# define NUM_ENCODERS (NUM_ENCODERS_LEFT + NUM_ENCODERS_RIGHT) +# endif // SPLIT_KEYBOARD -#else // SPLIT_KEYBOARD +# ifndef NUM_ENCODERS +# define NUM_ENCODERS 0 +# define NUM_ENCODERS_LEFT 0 +# define NUM_ENCODERS_RIGHT 0 +# endif // NUM_ENCODERS -# define NUM_ENCODERS ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) -# define NUM_ENCODERS_LEFT NUM_ENCODERS -# define NUM_ENCODERS_RIGHT 0 +# define NUM_ENCODERS_MAX_PER_SIDE MAX(NUM_ENCODERS_LEFT, NUM_ENCODERS_RIGHT) -#endif // SPLIT_KEYBOARD +# ifndef MAX_QUEUED_ENCODER_EVENTS +# define MAX_QUEUED_ENCODER_EVENTS ((NUM_ENCODERS_MAX_PER_SIDE) + 1) +# endif // MAX_QUEUED_ENCODER_EVENTS -#ifndef NUM_ENCODERS -# define NUM_ENCODERS 0 -# define NUM_ENCODERS_LEFT 0 -# define NUM_ENCODERS_RIGHT 0 -#endif // NUM_ENCODERS +typedef struct encoder_event_t { + uint8_t index : 7; + uint8_t clockwise : 1; +} encoder_event_t; -#define NUM_ENCODERS_MAX_PER_SIDE MAX(NUM_ENCODERS_LEFT, NUM_ENCODERS_RIGHT) +typedef struct encoder_events_t { + uint8_t head; + uint8_t tail; + encoder_event_t queue[MAX_QUEUED_ENCODER_EVENTS]; +} encoder_events_t; -#ifdef ENCODER_MAP_ENABLE -# define NUM_DIRECTIONS 2 -# define ENCODER_CCW_CW(ccw, cw) \ - { (cw), (ccw) } +// Get the current queued events +void encoder_retrieve_events(encoder_events_t *events); + +# ifdef SPLIT_KEYBOARD +void encoder_set_tail_index(uint8_t tail_index); +void encoder_handle_slave_events(encoder_events_t *events); +# endif // SPLIT_KEYBOARD + +# ifdef ENCODER_MAP_ENABLE +# define NUM_DIRECTIONS 2 +# define ENCODER_CCW_CW(ccw, cw) \ + { (cw), (ccw) } extern const uint16_t encoder_map[][NUM_ENCODERS][NUM_DIRECTIONS]; -#endif // ENCODER_MAP_ENABLE +# endif // ENCODER_MAP_ENABLE + +// "Custom encoder lite" support +void encoder_driver_init(void); +void encoder_driver_task(void); + +#endif // ENCODER_ENABLE diff --git a/quantum/encoder/tests/config_encoder_common.h b/quantum/encoder/tests/config_encoder_common.h new file mode 100644 index 000000000000..6b3b20182b53 --- /dev/null +++ b/quantum/encoder/tests/config_encoder_common.h @@ -0,0 +1,6 @@ +// Copyright 2023 Nick Brassel (@tzarc) +// SPDX-License-Identifier: GPL-2.0-or-later +#pragma once + +// Override the one in quantum/util because it doesn't like working on x64 builds. +#define ARRAY_SIZE(array) (sizeof((array)) / sizeof((array)[0])) diff --git a/quantum/encoder/tests/config_mock.h b/quantum/encoder/tests/config_mock.h index 703dcaf10361..59c423972a8f 100644 --- a/quantum/encoder/tests/config_mock.h +++ b/quantum/encoder/tests/config_mock.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_left_eq_right.h b/quantum/encoder/tests/config_mock_split_left_eq_right.h index c80ac4d51988..4ebe4c452d72 100644 --- a/quantum/encoder/tests/config_mock_split_left_eq_right.h +++ b/quantum/encoder/tests/config_mock_split_left_eq_right.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_left_gt_right.h b/quantum/encoder/tests/config_mock_split_left_gt_right.h index 91d5f3d6058c..4c230ea460c4 100644 --- a/quantum/encoder/tests/config_mock_split_left_gt_right.h +++ b/quantum/encoder/tests/config_mock_split_left_gt_right.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_left_lt_right.h b/quantum/encoder/tests/config_mock_split_left_lt_right.h index 4108a184a68c..fa935cedcc6e 100644 --- a/quantum/encoder/tests/config_mock_split_left_lt_right.h +++ b/quantum/encoder/tests/config_mock_split_left_lt_right.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_no_left.h b/quantum/encoder/tests/config_mock_split_no_left.h index 9db7fa7e41b1..a4464978509d 100644 --- a/quantum/encoder/tests/config_mock_split_no_left.h +++ b/quantum/encoder/tests/config_mock_split_no_left.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_no_right.h b/quantum/encoder/tests/config_mock_split_no_right.h index 14f18015e661..8b10e49bfba8 100644 --- a/quantum/encoder/tests/config_mock_split_no_right.h +++ b/quantum/encoder/tests/config_mock_split_no_right.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/config_mock_split_role.h b/quantum/encoder/tests/config_mock_split_role.h index c80ac4d51988..4ebe4c452d72 100644 --- a/quantum/encoder/tests/config_mock_split_role.h +++ b/quantum/encoder/tests/config_mock_split_role.h @@ -1,6 +1,7 @@ // Copyright 2022 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include "config_encoder_common.h" #define MATRIX_ROWS 1 #define MATRIX_COLS 1 diff --git a/quantum/encoder/tests/encoder_tests.cpp b/quantum/encoder/tests/encoder_tests.cpp index b7c18aeec008..499e413aed4a 100644 --- a/quantum/encoder/tests/encoder_tests.cpp +++ b/quantum/encoder/tests/encoder_tests.cpp @@ -41,7 +41,7 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderTest : public ::testing::Test {}; diff --git a/quantum/encoder/tests/encoder_tests_split_left_eq_right.cpp b/quantum/encoder/tests/encoder_tests_split_left_eq_right.cpp index 916e47b18517..7d6b3e30e6bf 100644 --- a/quantum/encoder/tests/encoder_tests_split_left_eq_right.cpp +++ b/quantum/encoder/tests/encoder_tests_split_left_eq_right.cpp @@ -33,22 +33,29 @@ struct update { uint8_t updates_array_idx = 0; update updates[32]; +bool isMaster; bool isLeftHand; +extern "C" { +bool is_keyboard_master(void) { + return isMaster; +} + bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!isLeftHand) { + if (!is_keyboard_master()) { // this method has no effect on slave half - printf("ignoring update on right hand (%d,%s)\n", index, clockwise ? "CW" : "CC"); + printf("ignoring update on slave (%d,%s)\n", index, clockwise ? "CW" : "CC"); return true; } updates[updates_array_idx % 32] = {index, clockwise}; updates_array_idx++; return true; } +}; bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestLeftEqRight : public ::testing::Test { @@ -63,6 +70,7 @@ class EncoderSplitTestLeftEqRight : public ::testing::Test { }; TEST_F(EncoderSplitTestLeftEqRight, TestInitLeft) { + isMaster = true; isLeftHand = true; encoder_init(); EXPECT_EQ(pinIsInputHigh[0], true); @@ -77,6 +85,7 @@ TEST_F(EncoderSplitTestLeftEqRight, TestInitLeft) { } TEST_F(EncoderSplitTestLeftEqRight, TestInitRight) { + isMaster = true; isLeftHand = false; encoder_init(); EXPECT_EQ(pinIsInputHigh[0], false); @@ -90,7 +99,8 @@ TEST_F(EncoderSplitTestLeftEqRight, TestInitRight) { EXPECT_EQ(updates_array_idx, 0); // no updates received } -TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseLeft) { +TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseLeftMaster) { + isMaster = true; isLeftHand = true; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -102,9 +112,19 @@ TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseLeft) { EXPECT_EQ(updates_array_idx, 1); // one update received EXPECT_EQ(updates[0].index, 0); EXPECT_EQ(updates[0].clockwise, true); + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseRightSent) { +TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseRightMaster) { + isMaster = true; isLeftHand = false; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -113,23 +133,60 @@ TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseRightSent) { setAndRead(6, true); setAndRead(7, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); + EXPECT_EQ(updates_array_idx, 1); // one update received + EXPECT_EQ(updates[0].index, 3); + EXPECT_EQ(updates[0].clockwise, true); - EXPECT_EQ(slave_state[0], 0); - EXPECT_EQ(slave_state[1], 0xFF); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftEqRight, TestMultipleEncodersRightReceived) { +TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseLeftSlave) { + isMaster = false; isLeftHand = true; encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(0, false); + setAndRead(1, false); + setAndRead(0, true); + setAndRead(1, true); - uint8_t slave_state[32] = {1, 0xFF}; // First right encoder is CCW, Second right encoder CW - encoder_update_raw(slave_state); + EXPECT_EQ(updates_array_idx, 0); // no updates received - EXPECT_EQ(updates_array_idx, 2); // two updates received, one for each changed item on the right side - EXPECT_EQ(updates[0].index, 2); - EXPECT_EQ(updates[0].clockwise, false); - EXPECT_EQ(updates[1].index, 3); - EXPECT_EQ(updates[1].clockwise, true); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave +} + +TEST_F(EncoderSplitTestLeftEqRight, TestOneClockwiseRightSlave) { + isMaster = false; + isLeftHand = false; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(6, false); + setAndRead(7, false); + setAndRead(6, true); + setAndRead(7, true); + + EXPECT_EQ(updates_array_idx, 0); // no updates received + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave } diff --git a/quantum/encoder/tests/encoder_tests_split_left_gt_right.cpp b/quantum/encoder/tests/encoder_tests_split_left_gt_right.cpp index 7b64bb298136..2beb4e39720b 100644 --- a/quantum/encoder/tests/encoder_tests_split_left_gt_right.cpp +++ b/quantum/encoder/tests/encoder_tests_split_left_gt_right.cpp @@ -33,22 +33,29 @@ struct update { uint8_t updates_array_idx = 0; update updates[32]; +bool isMaster; bool isLeftHand; +extern "C" { +bool is_keyboard_master(void) { + return isMaster; +} + bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!isLeftHand) { + if (!is_keyboard_master()) { // this method has no effect on slave half - printf("ignoring update on right hand (%d,%s)\n", index, clockwise ? "CW" : "CC"); + printf("ignoring update on slave (%d,%s)\n", index, clockwise ? "CW" : "CC"); return true; } updates[updates_array_idx % 32] = {index, clockwise}; updates_array_idx++; return true; } +}; bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestLeftGreaterThanRight : public ::testing::Test { @@ -94,7 +101,8 @@ TEST_F(EncoderSplitTestLeftGreaterThanRight, TestInitRight) { EXPECT_EQ(updates_array_idx, 0); // no updates received } -TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseLeft) { +TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseLeftMaster) { + isMaster = true; isLeftHand = true; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -106,9 +114,19 @@ TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseLeft) { EXPECT_EQ(updates_array_idx, 1); // one update received EXPECT_EQ(updates[0].index, 0); EXPECT_EQ(updates[0].clockwise, true); + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseRightSent) { +TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseRightMaster) { + isMaster = true; isLeftHand = false; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -117,23 +135,60 @@ TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseRightSent) { setAndRead(6, true); setAndRead(7, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); + EXPECT_EQ(updates_array_idx, 1); // one update received + EXPECT_EQ(updates[0].index, 3); + EXPECT_EQ(updates[0].clockwise, true); - EXPECT_EQ(slave_state[0], 0xFF); - EXPECT_EQ(slave_state[1], 0); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftGreaterThanRight, TestMultipleEncodersRightReceived) { +TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseLeftSlave) { + isMaster = false; isLeftHand = true; encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(0, false); + setAndRead(1, false); + setAndRead(0, true); + setAndRead(1, true); - uint8_t slave_state[32] = {1, 0xFF}; // First right encoder is CCW, Second right encoder no change, third right encoder CW - encoder_update_raw(slave_state); + EXPECT_EQ(updates_array_idx, 0); // no updates received - EXPECT_EQ(updates_array_idx, 2); // two updates received, one for each changed item on the right side - EXPECT_EQ(updates[0].index, 3); - EXPECT_EQ(updates[0].clockwise, false); - EXPECT_EQ(updates[1].index, 4); - EXPECT_EQ(updates[1].clockwise, true); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave +} + +TEST_F(EncoderSplitTestLeftGreaterThanRight, TestOneClockwiseRightSlave) { + isMaster = false; + isLeftHand = false; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(6, false); + setAndRead(7, false); + setAndRead(6, true); + setAndRead(7, true); + + EXPECT_EQ(updates_array_idx, 0); // no updates received + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave } diff --git a/quantum/encoder/tests/encoder_tests_split_left_lt_right.cpp b/quantum/encoder/tests/encoder_tests_split_left_lt_right.cpp index a6519c576257..5612f8b65895 100644 --- a/quantum/encoder/tests/encoder_tests_split_left_lt_right.cpp +++ b/quantum/encoder/tests/encoder_tests_split_left_lt_right.cpp @@ -33,22 +33,29 @@ struct update { uint8_t updates_array_idx = 0; update updates[32]; +bool isMaster; bool isLeftHand; +extern "C" { +bool is_keyboard_master(void) { + return isMaster; +} + bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!isLeftHand) { + if (!is_keyboard_master()) { // this method has no effect on slave half - printf("ignoring update on right hand (%d,%s)\n", index, clockwise ? "CW" : "CC"); + printf("ignoring update on slave (%d,%s)\n", index, clockwise ? "CW" : "CC"); return true; } updates[updates_array_idx % 32] = {index, clockwise}; updates_array_idx++; return true; } +}; bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestLeftLessThanRight : public ::testing::Test { @@ -94,7 +101,8 @@ TEST_F(EncoderSplitTestLeftLessThanRight, TestInitRight) { EXPECT_EQ(updates_array_idx, 0); // no updates received } -TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseLeft) { +TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseLeftMaster) { + isMaster = true; isLeftHand = true; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -106,9 +114,19 @@ TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseLeft) { EXPECT_EQ(updates_array_idx, 1); // one update received EXPECT_EQ(updates[0].index, 0); EXPECT_EQ(updates[0].clockwise, true); + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseRightSent) { +TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseRightMaster) { + isMaster = true; isLeftHand = false; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -117,23 +135,60 @@ TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseRightSent) { setAndRead(6, true); setAndRead(7, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); + EXPECT_EQ(updates_array_idx, 1); // one update received + EXPECT_EQ(updates[0].index, 3); + EXPECT_EQ(updates[0].clockwise, true); - EXPECT_EQ(slave_state[0], 0); - EXPECT_EQ(slave_state[1], 0xFF); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestLeftLessThanRight, TestMultipleEncodersRightReceived) { +TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseLeftSlave) { + isMaster = false; isLeftHand = true; encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(0, false); + setAndRead(1, false); + setAndRead(0, true); + setAndRead(1, true); - uint8_t slave_state[32] = {1, 0, 0xFF}; // First right encoder is CCW, Second right encoder no change, third right encoder CW - encoder_update_raw(slave_state); + EXPECT_EQ(updates_array_idx, 0); // no updates received - EXPECT_EQ(updates_array_idx, 2); // two updates received, one for each changed item on the right side - EXPECT_EQ(updates[0].index, 2); - EXPECT_EQ(updates[0].clockwise, false); - EXPECT_EQ(updates[1].index, 4); - EXPECT_EQ(updates[1].clockwise, true); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave +} + +TEST_F(EncoderSplitTestLeftLessThanRight, TestOneClockwiseRightSlave) { + isMaster = false; + isLeftHand = false; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(6, false); + setAndRead(7, false); + setAndRead(6, true); + setAndRead(7, true); + + EXPECT_EQ(updates_array_idx, 0); // no updates received + + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave } diff --git a/quantum/encoder/tests/encoder_tests_split_no_left.cpp b/quantum/encoder/tests/encoder_tests_split_no_left.cpp index b6b2d7e2d19a..980e4074ffda 100644 --- a/quantum/encoder/tests/encoder_tests_split_no_left.cpp +++ b/quantum/encoder/tests/encoder_tests_split_no_left.cpp @@ -33,22 +33,29 @@ struct update { uint8_t updates_array_idx = 0; update updates[32]; +bool isMaster; bool isLeftHand; +extern "C" { +bool is_keyboard_master(void) { + return isMaster; +} + bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!isLeftHand) { + if (!is_keyboard_master()) { // this method has no effect on slave half - printf("ignoring update on right hand (%d,%s)\n", index, clockwise ? "CW" : "CC"); + printf("ignoring update on slave (%d,%s)\n", index, clockwise ? "CW" : "CC"); return true; } updates[updates_array_idx % 32] = {index, clockwise}; updates_array_idx++; return true; } +}; bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestNoLeft : public ::testing::Test { @@ -82,19 +89,8 @@ TEST_F(EncoderSplitTestNoLeft, TestInitRight) { EXPECT_EQ(updates_array_idx, 0); // no updates received } -TEST_F(EncoderSplitTestNoLeft, TestOneClockwiseLeft) { - isLeftHand = true; - encoder_init(); - // send 4 pulses. with resolution 4, that's one step and we should get 1 update. - setAndRead(0, false); - setAndRead(1, false); - setAndRead(0, true); - setAndRead(1, true); - - EXPECT_EQ(updates_array_idx, 0); // no updates received -} - -TEST_F(EncoderSplitTestNoLeft, TestOneClockwiseRightSent) { +TEST_F(EncoderSplitTestNoLeft, TestOneClockwiseLeftMaster) { + isMaster = true; isLeftHand = false; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. @@ -103,23 +99,38 @@ TEST_F(EncoderSplitTestNoLeft, TestOneClockwiseRightSent) { setAndRead(2, true); setAndRead(3, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); + EXPECT_EQ(updates_array_idx, 1); // one update received + EXPECT_EQ(updates[0].index, 1); + EXPECT_EQ(updates[0].clockwise, true); - EXPECT_EQ(slave_state[0], 0); - EXPECT_EQ(slave_state[1], 0xFF); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestNoLeft, TestMultipleEncodersRightReceived) { - isLeftHand = true; +TEST_F(EncoderSplitTestNoLeft, TestOneClockwiseRightSlave) { + isMaster = false; + isLeftHand = false; encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(2, false); + setAndRead(3, false); + setAndRead(2, true); + setAndRead(3, true); - uint8_t slave_state[32] = {1, 0xFF}; // First right encoder is CCW, Second right encoder no change, third right encoder CW - encoder_update_raw(slave_state); + EXPECT_EQ(updates_array_idx, 0); // no updates received - EXPECT_EQ(updates_array_idx, 2); // two updates received, one for each changed item on the right side - EXPECT_EQ(updates[0].index, 0); - EXPECT_EQ(updates[0].clockwise, false); - EXPECT_EQ(updates[1].index, 1); - EXPECT_EQ(updates[1].clockwise, true); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave } diff --git a/quantum/encoder/tests/encoder_tests_split_no_right.cpp b/quantum/encoder/tests/encoder_tests_split_no_right.cpp index fa0a7c18a806..d39659853b2a 100644 --- a/quantum/encoder/tests/encoder_tests_split_no_right.cpp +++ b/quantum/encoder/tests/encoder_tests_split_no_right.cpp @@ -33,22 +33,29 @@ struct update { uint8_t updates_array_idx = 0; update updates[32]; +bool isMaster; bool isLeftHand; +extern "C" { +bool is_keyboard_master(void) { + return isMaster; +} + bool encoder_update_kb(uint8_t index, bool clockwise) { - if (!isLeftHand) { + if (!is_keyboard_master()) { // this method has no effect on slave half - printf("ignoring update on right hand (%d,%s)\n", index, clockwise ? "CW" : "CC"); + printf("ignoring update on slave (%d,%s)\n", index, clockwise ? "CW" : "CC"); return true; } updates[updates_array_idx % 32] = {index, clockwise}; updates_array_idx++; return true; } +}; bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestNoRight : public ::testing::Test { @@ -82,37 +89,48 @@ TEST_F(EncoderSplitTestNoRight, TestInitRight) { EXPECT_EQ(updates_array_idx, 0); // no updates received } -TEST_F(EncoderSplitTestNoRight, TestOneClockwiseLeft) { +TEST_F(EncoderSplitTestNoRight, TestOneClockwiseLeftMaster) { + isMaster = true; isLeftHand = true; encoder_init(); // send 4 pulses. with resolution 4, that's one step and we should get 1 update. - setAndRead(0, false); - setAndRead(1, false); - setAndRead(0, true); - setAndRead(1, true); + setAndRead(2, false); + setAndRead(3, false); + setAndRead(2, true); + setAndRead(3, true); - EXPECT_EQ(updates_array_idx, 1); // one updates received - EXPECT_EQ(updates[0].index, 0); + EXPECT_EQ(updates_array_idx, 1); // one update received + EXPECT_EQ(updates[0].index, 1); EXPECT_EQ(updates[0].clockwise, true); -} -TEST_F(EncoderSplitTestNoRight, TestOneClockwiseRightSent) { - isLeftHand = false; - encoder_init(); - - uint8_t slave_state[32] = {0xAA, 0xAA}; - encoder_state_raw(slave_state); - - EXPECT_EQ(slave_state[0], 0xAA); - EXPECT_EQ(slave_state[1], 0xAA); + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 0); // No events should be queued on master } -TEST_F(EncoderSplitTestNoRight, TestMultipleEncodersRightReceived) { +TEST_F(EncoderSplitTestNoRight, TestOneClockwiseRightSlave) { + isMaster = false; isLeftHand = true; encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(2, false); + setAndRead(3, false); + setAndRead(2, true); + setAndRead(3, true); - uint8_t slave_state[32] = {1, 0xFF}; // These values would trigger updates if there were encoders on the other side - encoder_update_raw(slave_state); + EXPECT_EQ(updates_array_idx, 0); // no updates received - EXPECT_EQ(updates_array_idx, 0); // no updates received -- no right-hand encoders + int events_queued = 0; + encoder_events_t events; + encoder_retrieve_events(&events); + while (events.tail != events.head) { + events.tail = (events.tail + 1) % MAX_QUEUED_ENCODER_EVENTS; + ++events_queued; + } + EXPECT_EQ(events_queued, 1); // One event should be queued on slave } diff --git a/quantum/encoder/tests/encoder_tests_split_role.cpp b/quantum/encoder/tests/encoder_tests_split_role.cpp index 0ab7bfc2a783..b588af8c70aa 100644 --- a/quantum/encoder/tests/encoder_tests_split_role.cpp +++ b/quantum/encoder/tests/encoder_tests_split_role.cpp @@ -50,7 +50,7 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { bool setAndRead(pin_t pin, bool val) { setPin(pin, val); - return encoder_read(); + return encoder_task(); } class EncoderSplitTestRole : public ::testing::Test { @@ -87,9 +87,6 @@ TEST_F(EncoderSplitTestRole, TestPrimaryRight) { setAndRead(6, true); setAndRead(7, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); - EXPECT_EQ(num_updates, 1); // one update received } @@ -116,8 +113,5 @@ TEST_F(EncoderSplitTestRole, TestNotPrimaryRight) { setAndRead(6, true); setAndRead(7, true); - uint8_t slave_state[32] = {0}; - encoder_state_raw(slave_state); - EXPECT_EQ(num_updates, 0); // zero updates received } diff --git a/quantum/encoder/tests/mock_split.c b/quantum/encoder/tests/mock_split.c index 5cc6cd19e172..dd3c26d9584b 100644 --- a/quantum/encoder/tests/mock_split.c +++ b/quantum/encoder/tests/mock_split.c @@ -36,7 +36,3 @@ bool setPin(pin_t pin, bool val) { } void last_encoder_activity_trigger(void) {} - -__attribute__((weak)) bool is_keyboard_master(void) { - return true; -} diff --git a/quantum/encoder/tests/mock_split.h b/quantum/encoder/tests/mock_split.h index 2fc12f18306f..f9ce73d4b528 100644 --- a/quantum/encoder/tests/mock_split.h +++ b/quantum/encoder/tests/mock_split.h @@ -22,9 +22,6 @@ #define SPLIT_KEYBOARD typedef uint8_t pin_t; -void encoder_state_raw(uint8_t* slave_state); -void encoder_update_raw(uint8_t* slave_state); - extern bool pins[]; extern bool pinIsInputHigh[]; diff --git a/quantum/encoder/tests/rules.mk b/quantum/encoder/tests/rules.mk index d01c1c66ee52..2fba8a5038ef 100644 --- a/quantum/encoder/tests/rules.mk +++ b/quantum/encoder/tests/rules.mk @@ -3,6 +3,7 @@ encoder_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock.h encoder_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests.cpp \ $(QUANTUM_PATH)/encoder.c @@ -13,6 +14,7 @@ encoder_split_left_eq_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_eq_right_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_eq_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -23,6 +25,7 @@ encoder_split_left_gt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_gt_right_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_gt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -33,6 +36,7 @@ encoder_split_left_lt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_lt_right_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_lt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -43,6 +47,7 @@ encoder_split_no_left_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_ encoder_split_no_left_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_left.cpp \ $(QUANTUM_PATH)/encoder.c @@ -53,6 +58,7 @@ encoder_split_no_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split encoder_split_no_right_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -63,6 +69,7 @@ encoder_split_role_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_rol encoder_split_role_SRC := \ platforms/test/timer.c \ + drivers/encoder/encoder_gpio.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_role.cpp \ $(QUANTUM_PATH)/encoder.c diff --git a/quantum/keyboard.c b/quantum/keyboard.c index c2ca15d52def..83644447020b 100644 --- a/quantum/keyboard.c +++ b/quantum/keyboard.c @@ -670,7 +670,7 @@ void keyboard_task(void) { #endif #ifdef ENCODER_ENABLE - if (encoder_read()) { + if (encoder_task()) { last_encoder_activity_trigger(); activity_has_occurred = true; } diff --git a/quantum/split_common/transaction_id_define.h b/quantum/split_common/transaction_id_define.h index 4d4d2b957084..05b3bf7b625d 100644 --- a/quantum/split_common/transaction_id_define.h +++ b/quantum/split_common/transaction_id_define.h @@ -31,6 +31,7 @@ enum serial_transaction_id { #ifdef ENCODER_ENABLE GET_ENCODERS_CHECKSUM, GET_ENCODERS_DATA, + PUT_ENCODER_TAIL, #endif // ENCODER_ENABLE #ifndef DISABLE_SYNC_TIMER diff --git a/quantum/split_common/transactions.c b/quantum/split_common/transactions.c index 2b9423cd6369..2cfa83e7a3f0 100644 --- a/quantum/split_common/transactions.c +++ b/quantum/split_common/transactions.c @@ -234,21 +234,28 @@ static void master_matrix_handlers_slave(matrix_row_t master_matrix[], matrix_ro #ifdef ENCODER_ENABLE static bool encoder_handlers_master(matrix_row_t master_matrix[], matrix_row_t slave_matrix[]) { - static uint32_t last_update = 0; - uint8_t temp_state[NUM_ENCODERS_MAX_PER_SIDE]; + static uint32_t last_update = 0; + encoder_events_t temp_events; - bool okay = read_if_checksum_mismatch(GET_ENCODERS_CHECKSUM, GET_ENCODERS_DATA, &last_update, temp_state, split_shmem->encoders.state, sizeof(temp_state)); - if (okay) encoder_update_raw(temp_state); + bool okay = read_if_checksum_mismatch(GET_ENCODERS_CHECKSUM, GET_ENCODERS_DATA, &last_update, &temp_events, &split_shmem->encoders.events, sizeof(temp_events)); + if (okay) { + encoder_handle_slave_events(&split_shmem->encoders.events); + transport_write(PUT_ENCODER_TAIL, &split_shmem->encoders.events.tail, sizeof(split_shmem->encoders.events.tail)); + split_shmem->encoders.checksum = crc8(&split_shmem->encoders.events, sizeof(split_shmem->encoders.events)); + } return okay; } static void encoder_handlers_slave(matrix_row_t master_matrix[], matrix_row_t slave_matrix[]) { - uint8_t encoder_state[NUM_ENCODERS_MAX_PER_SIDE]; - encoder_state_raw(encoder_state); // Always prepare the encoder state for read. - memcpy(split_shmem->encoders.state, encoder_state, sizeof(encoder_state)); + encoder_retrieve_events(&split_shmem->encoders.events); // Now update the checksum given that the encoders has been written to - split_shmem->encoders.checksum = crc8(encoder_state, sizeof(encoder_state)); + split_shmem->encoders.checksum = crc8(&split_shmem->encoders.events, sizeof(split_shmem->encoders.events)); +} + +static void encoder_handlers_slave_reset(uint8_t initiator2target_buffer_size, const void *initiator2target_buffer, uint8_t target2initiator_buffer_size, void *target2initiator_buffer) { + uint8_t tail_index = *(uint8_t *)initiator2target_buffer; + encoder_set_tail_index(tail_index); } // clang-format off @@ -256,7 +263,8 @@ static void encoder_handlers_slave(matrix_row_t master_matrix[], matrix_row_t sl # define TRANSACTIONS_ENCODERS_SLAVE() TRANSACTION_HANDLER_SLAVE_AUTOLOCK(encoder) # define TRANSACTIONS_ENCODERS_REGISTRATIONS \ [GET_ENCODERS_CHECKSUM] = trans_target2initiator_initializer(encoders.checksum), \ - [GET_ENCODERS_DATA] = trans_target2initiator_initializer(encoders.state), + [GET_ENCODERS_DATA] = trans_target2initiator_initializer(encoders.events), \ + [PUT_ENCODER_TAIL] = trans_initiator2target_initializer_cb(encoders.events.tail, encoder_handlers_slave_reset), // clang-format on #else // ENCODER_ENABLE diff --git a/quantum/split_common/transport.h b/quantum/split_common/transport.h index 2e2b918d4518..4f6b968fa8a0 100644 --- a/quantum/split_common/transport.h +++ b/quantum/split_common/transport.h @@ -65,8 +65,8 @@ typedef struct _split_master_matrix_sync_t { #ifdef ENCODER_ENABLE typedef struct _split_slave_encoder_sync_t { - uint8_t checksum; - uint8_t state[NUM_ENCODERS_MAX_PER_SIDE]; + uint8_t checksum; + encoder_events_t events; } split_slave_encoder_sync_t; #endif // ENCODER_ENABLE From fd99f3ffe432142ee51a6a9faed97a73689f8faa Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Tue, 18 Jul 2023 06:12:37 +1000 Subject: [PATCH 02/18] Lower bound on number of queued events. --- quantum/encoder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/encoder.h b/quantum/encoder.h index b002918e104d..7da7ffbb1e53 100644 --- a/quantum/encoder.h +++ b/quantum/encoder.h @@ -61,7 +61,7 @@ bool encoder_update_user(uint8_t index, bool clockwise); # define NUM_ENCODERS_MAX_PER_SIDE MAX(NUM_ENCODERS_LEFT, NUM_ENCODERS_RIGHT) # ifndef MAX_QUEUED_ENCODER_EVENTS -# define MAX_QUEUED_ENCODER_EVENTS ((NUM_ENCODERS_MAX_PER_SIDE) + 1) +# define MAX_QUEUED_ENCODER_EVENTS MAX(4, ((NUM_ENCODERS_MAX_PER_SIDE) + 1)) # endif // MAX_QUEUED_ENCODER_EVENTS typedef struct encoder_event_t { From ec9f3d861d230ee4a8fc6f7825ec2e510537e69b Mon Sep 17 00:00:00 2001 From: Drashna Jaelre Date: Mon, 17 Jul 2023 13:30:55 -0700 Subject: [PATCH 03/18] Add changes for other ploopy boards (#22) --- keyboards/ploopyco/mouse/mouse.c | 20 +++++++++---------- keyboards/ploopyco/mouse/rules.mk | 4 ++-- keyboards/ploopyco/trackball/rules.mk | 4 ++-- keyboards/ploopyco/trackball/trackball.c | 20 +++++++++---------- keyboards/ploopyco/trackball_mini/rules.mk | 4 ++-- .../ploopyco/trackball_mini/trackball_mini.c | 20 +++++++++---------- 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/keyboards/ploopyco/mouse/mouse.c b/keyboards/ploopyco/mouse/mouse.c index e2dada4a1555..5f4a82e474cb 100644 --- a/keyboards/ploopyco/mouse/mouse.c +++ b/keyboards/ploopyco/mouse/mouse.c @@ -66,8 +66,6 @@ uint8_t OptLowPin = OPT_ENC1; bool debug_encoder = false; bool is_drag_scroll = false; -__attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { return true; } - bool encoder_update_kb(uint8_t index, bool clockwise) { if (!encoder_update_user(index, clockwise)) { return false; @@ -83,7 +81,14 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { return true; } -void process_wheel(void) { +void encoder_driver_init(void) { + setPinInput(OPT_ENC1); + setPinInput(OPT_ENC2); + + opt_encoder_init(); +} + +void encoder_driver_task(void) { // Lovingly ripped from the Ploopy Source // If the mouse wheel was just released, do not scroll. @@ -111,12 +116,10 @@ void process_wheel(void) { int dir = opt_encoder_handler(p1, p2); if (dir == 0) return; - encoder_update_kb(0, dir > 0); + encoder_queue_event(0, dir == 1); } report_mouse_t pointing_device_task_kb(report_mouse_t mouse_report) { - process_wheel(); - if (is_drag_scroll) { mouse_report.h = mouse_report.x; #ifdef PLOOPY_DRAGSCROLL_INVERT @@ -177,9 +180,6 @@ void keyboard_pre_init_kb(void) { // debug_mouse = true; // debug_encoder = true; - setPinInput(OPT_ENC1); - setPinInput(OPT_ENC2); - /* Ground all output pins connected to ground. This provides additional * pathways to ground. If you're messing with this, know this: driving ANY * of these pins high will cause a short. On the MCU. Ka-blooey. @@ -204,8 +204,6 @@ void keyboard_pre_init_kb(void) { void pointing_device_init_kb(void) { pointing_device_set_cpi(dpi_array[keyboard_config.dpi_config]); - // initialize the scroll wheel's optical encoder - opt_encoder_init(); } void eeconfig_init_kb(void) { diff --git a/keyboards/ploopyco/mouse/rules.mk b/keyboards/ploopyco/mouse/rules.mk index 8d62f78ed711..e7af0c9d74fb 100644 --- a/keyboards/ploopyco/mouse/rules.mk +++ b/keyboards/ploopyco/mouse/rules.mk @@ -16,8 +16,8 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = pmw3360 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE := no -OPTS_DEF += -DENCODER_ENABLE +ENCODER_ENABLE = yes +ENCODER_DRIVER = custom QUANTUM_LIB_SRC += analog.c SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball/rules.mk b/keyboards/ploopyco/trackball/rules.mk index 0f5f3f648f60..c9e6dfaf0588 100644 --- a/keyboards/ploopyco/trackball/rules.mk +++ b/keyboards/ploopyco/trackball/rules.mk @@ -16,8 +16,8 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = pmw3360 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE := no -OPTS_DEF += -DENCODER_ENABLE +ENCODER_ENABLE = yes +ENCODER_DRIVER = custom QUANTUM_LIB_SRC += analog.c SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball/trackball.c b/keyboards/ploopyco/trackball/trackball.c index 8fd5b8650d39..0c606bab0704 100644 --- a/keyboards/ploopyco/trackball/trackball.c +++ b/keyboards/ploopyco/trackball/trackball.c @@ -66,8 +66,6 @@ uint8_t OptLowPin = OPT_ENC1; bool debug_encoder = false; bool is_drag_scroll = false; -__attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { return true; } - bool encoder_update_kb(uint8_t index, bool clockwise) { if (!encoder_update_user(index, clockwise)) { return false; @@ -83,7 +81,15 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { return true; } -void process_wheel(void) { + +void encoder_driver_init(void) { + setPinInput(OPT_ENC1); + setPinInput(OPT_ENC2); + + opt_encoder_init(); +} + +void encoder_driver_task(void) { // TODO: Replace this with interrupt driven code, polling is S L O W // Lovingly ripped from the Ploopy Source @@ -112,11 +118,10 @@ void process_wheel(void) { int dir = opt_encoder_handler(p1, p2); if (dir == 0) return; - encoder_update_kb(0, dir > 0); + encoder_queue_event(0, dir == 1); } report_mouse_t pointing_device_task_kb(report_mouse_t mouse_report) { - process_wheel(); if (is_drag_scroll) { #ifdef PLOOPY_DRAGSCROLL_H_INVERT @@ -189,9 +194,6 @@ void keyboard_pre_init_kb(void) { // debug_mouse = true; // debug_encoder = true; - setPinInput(OPT_ENC1); - setPinInput(OPT_ENC2); - /* Ground all output pins connected to ground. This provides additional * pathways to ground. If you're messing with this, know this: driving ANY * of these pins high will cause a short. On the MCU. Ka-blooey. @@ -216,8 +218,6 @@ void keyboard_pre_init_kb(void) { void pointing_device_init_kb(void) { pointing_device_set_cpi(dpi_array[keyboard_config.dpi_config]); - // initialize the scroll wheel's optical encoder - opt_encoder_init(); } void eeconfig_init_kb(void) { diff --git a/keyboards/ploopyco/trackball_mini/rules.mk b/keyboards/ploopyco/trackball_mini/rules.mk index d7016929c905..458ad5dbacdc 100644 --- a/keyboards/ploopyco/trackball_mini/rules.mk +++ b/keyboards/ploopyco/trackball_mini/rules.mk @@ -13,8 +13,8 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = adns5050 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE := no -OPTS_DEF += -DENCODER_ENABLE +ENCODER_ENABLE = yes +ENCODER_DRIVER = custom QUANTUM_LIB_SRC += analog.c SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball_mini/trackball_mini.c b/keyboards/ploopyco/trackball_mini/trackball_mini.c index 8bc0ab99c234..8517a54e708a 100644 --- a/keyboards/ploopyco/trackball_mini/trackball_mini.c +++ b/keyboards/ploopyco/trackball_mini/trackball_mini.c @@ -74,8 +74,6 @@ uint8_t OptLowPin = OPT_ENC1; bool debug_encoder = false; bool is_drag_scroll = false; -__attribute__((weak)) bool encoder_update_user(uint8_t index, bool clockwise) { return true; } - bool encoder_update_kb(uint8_t index, bool clockwise) { if (!encoder_update_user(index, clockwise)) { return false; @@ -91,7 +89,14 @@ bool encoder_update_kb(uint8_t index, bool clockwise) { return true; } -void process_wheel(void) { +void encoder_driver_init(void) { + setPinInput(OPT_ENC1); + setPinInput(OPT_ENC2); + + opt_encoder_init(); +} + +void encoder_driver_task(void) { uint16_t p1 = adc_read(OPT_ENC1_MUX); uint16_t p2 = adc_read(OPT_ENC2_MUX); @@ -113,21 +118,17 @@ void process_wheel(void) { } if (dir == 0) return; - encoder_update_kb(0, dir > 0); + encoder_queue_event(0, dir == 1); lastScroll = timer_read(); } void pointing_device_init_kb(void) { - opt_encoder_init(); - // set the DPI. pointing_device_set_cpi(dpi_array[keyboard_config.dpi_config]); } report_mouse_t pointing_device_task_kb(report_mouse_t mouse_report) { - process_wheel(); - if (is_drag_scroll) { mouse_report.h = mouse_report.x; #ifdef PLOOPY_DRAGSCROLL_INVERT @@ -180,9 +181,6 @@ void keyboard_pre_init_kb(void) { // debug_mouse = true; // debug_encoder = true; - setPinInput(OPT_ENC1); - setPinInput(OPT_ENC2); - /* Ground all output pins connected to ground. This provides additional * pathways to ground. If you're messing with this, know this: driving ANY * of these pins high will cause a short. On the MCU. Ka-blooey. From aa667abee68c91b0b5b5690eb643b742b97287d7 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Tue, 18 Jul 2023 06:33:33 +1000 Subject: [PATCH 04/18] Fix tests --- quantum/encoder/tests/rules.mk | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/quantum/encoder/tests/rules.mk b/quantum/encoder/tests/rules.mk index 2fba8a5038ef..feaea0b2acf8 100644 --- a/quantum/encoder/tests/rules.mk +++ b/quantum/encoder/tests/rules.mk @@ -3,7 +3,7 @@ encoder_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock.h encoder_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests.cpp \ $(QUANTUM_PATH)/encoder.c @@ -14,7 +14,7 @@ encoder_split_left_eq_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_eq_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_eq_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -25,7 +25,7 @@ encoder_split_left_gt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_gt_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_gt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -36,7 +36,7 @@ encoder_split_left_lt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_lt_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_lt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -47,7 +47,7 @@ encoder_split_no_left_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_ encoder_split_no_left_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_left.cpp \ $(QUANTUM_PATH)/encoder.c @@ -58,7 +58,7 @@ encoder_split_no_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split encoder_split_no_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -69,7 +69,7 @@ encoder_split_role_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_rol encoder_split_role_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_gpio.c \ + drivers/encoder/encoder_graycode.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_role.cpp \ $(QUANTUM_PATH)/encoder.c From 78f3470d4f948351d6a01821cab0bb0b275c10cf Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Thu, 20 Jul 2023 06:34:01 +1000 Subject: [PATCH 05/18] Cleanup. --- drivers/encoder/encoder_graycode.c | 39 ++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index 8e716ddc7fc0..5bd7a5c69571 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -75,11 +75,24 @@ __attribute__((weak)) void encoder_wait_pullup_charge(void) { } __attribute__((weak)) void encoder_init_pin(uint8_t index, bool pad_b) { - setPinInputHigh(pad_b ? encoders_pad_b[index] : encoders_pad_a[index]); + pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; + if (pin != NO_PIN) { + setPinInputHigh(pin); + } } __attribute__((weak)) uint8_t encoder_read_pin(uint8_t index, bool pad_b) { - return readPin(pad_b ? encoders_pad_b[index] : encoders_pad_a[index]) ? 1 : 0; + pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; + if (pin != NO_PIN) { + return readPin(pin) ? 1 : 0; + } + return 0; +} + +__attribute__((weak)) void encoder_init_post_kb(void) { + extern void encoder_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); + // Unused normally, but can be used for things like setting up pin-change interrupts in keyboard code. + // During the interrupt, call `encoder_handle_read()` with the pin states and it'll queue up an encoder event if needed. } void encoder_driver_init(void) { @@ -140,9 +153,11 @@ void encoder_driver_init(void) { for (uint8_t i = 0; i < thisCount; i++) { encoder_state[i] = (encoder_read_pin(i, false) << 0) | (encoder_read_pin(i, true) << 1); } + + encoder_init_post_kb(); } -static bool encoder_handle_state_change(uint8_t index, uint8_t state) { +static void encoder_handle_state_change(uint8_t index, uint8_t state) { bool changed = false; uint8_t i = index; @@ -186,13 +201,17 @@ static bool encoder_handle_state_change(uint8_t index, uint8_t state) { return changed; } -void encoder_driver_task(void) { +void encoder_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state) { + uint8_t state = pin_a_state | (pin_b_state << 1); + if ((encoder_state[index] & 0x3) != state) { + encoder_state[index] <<= 2; + encoder_state[index] |= state; + encoder_handle_state_change(index, encoder_state[index]); + } +} + +__attribute__((weak)) void encoder_driver_task(void) { for (uint8_t i = 0; i < thisCount; i++) { - uint8_t new_status = (encoder_read_pin(i, false) << 0) | (encoder_read_pin(i, true) << 1); - if ((encoder_state[i] & 0x3) != new_status) { - encoder_state[i] <<= 2; - encoder_state[i] |= new_status; - encoder_handle_state_change(i, encoder_state[i]); - } + encoder_handle_read(i, encoder_read_pin(i, false), encoder_read_pin(i, true)); } } From 8f72a979b13605cb0666e186155100a04b4ca9ec Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Thu, 20 Jul 2023 08:39:10 +1000 Subject: [PATCH 06/18] Function naming. --- drivers/encoder/encoder_graycode.c | 42 +++++++++++++++--------------- keyboards/pica40/rev2/rev2.c | 4 +-- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index 5bd7a5c69571..1075cbd61b96 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -74,14 +74,14 @@ __attribute__((weak)) void encoder_wait_pullup_charge(void) { wait_us(100); } -__attribute__((weak)) void encoder_init_pin(uint8_t index, bool pad_b) { +__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { setPinInputHigh(pin); } } -__attribute__((weak)) uint8_t encoder_read_pin(uint8_t index, bool pad_b) { +__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { return readPin(pin) ? 1 : 0; @@ -89,12 +89,25 @@ __attribute__((weak)) uint8_t encoder_read_pin(uint8_t index, bool pad_b) { return 0; } -__attribute__((weak)) void encoder_init_post_kb(void) { - extern void encoder_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); +__attribute__((weak)) void encoder_graycode_post_init_kb(void) { + extern void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); // Unused normally, but can be used for things like setting up pin-change interrupts in keyboard code. // During the interrupt, call `encoder_handle_read()` with the pin states and it'll queue up an encoder event if needed. } +void encoder_graycode_post_init(void) { + for (uint8_t i = 0; i < thisCount; i++) { + encoder_graycode_init_pin(i, false); + encoder_graycode_init_pin(i, true); + } + encoder_wait_pullup_charge(); + for (uint8_t i = 0; i < thisCount; i++) { + encoder_state[i] = (encoder_graycode_read_pin(i, false) << 0) | (encoder_graycode_read_pin(i, true) << 1); + } + + encoder_graycode_post_init_kb(); +} + void encoder_driver_init(void) { #ifdef SPLIT_KEYBOARD thisHand = isLeftHand ? 0 : NUM_ENCODERS_LEFT; @@ -145,21 +158,11 @@ void encoder_driver_init(void) { } #endif // defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) - for (uint8_t i = 0; i < thisCount; i++) { - encoder_init_pin(i, false); - encoder_init_pin(i, true); - } - encoder_wait_pullup_charge(); - for (uint8_t i = 0; i < thisCount; i++) { - encoder_state[i] = (encoder_read_pin(i, false) << 0) | (encoder_read_pin(i, true) << 1); - } - - encoder_init_post_kb(); + encoder_graycode_post_init(); } static void encoder_handle_state_change(uint8_t index, uint8_t state) { - bool changed = false; - uint8_t i = index; + uint8_t i = index; #ifdef ENCODER_RESOLUTIONS const uint8_t resolution = encoder_resolutions[i]; @@ -180,7 +183,6 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { #endif encoder_value[index]++; - changed = true; encoder_queue_event(index, ENCODER_COUNTER_CLOCKWISE); } @@ -190,7 +192,6 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { if (encoder_pulses[i] <= -resolution) { // direction is arbitrary here, but this clockwise #endif encoder_value[index]--; - changed = true; encoder_queue_event(index, ENCODER_CLOCKWISE); } encoder_pulses[i] %= resolution; @@ -198,10 +199,9 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { encoder_pulses[i] = 0; } #endif - return changed; } -void encoder_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state) { +void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state) { uint8_t state = pin_a_state | (pin_b_state << 1); if ((encoder_state[index] & 0x3) != state) { encoder_state[index] <<= 2; @@ -212,6 +212,6 @@ void encoder_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state __attribute__((weak)) void encoder_driver_task(void) { for (uint8_t i = 0; i < thisCount; i++) { - encoder_handle_read(i, encoder_read_pin(i, false), encoder_read_pin(i, true)); + encoder_graycode_handle_read(i, encoder_graycode_read_pin(i, false), encoder_graycode_read_pin(i, true)); } } diff --git a/keyboards/pica40/rev2/rev2.c b/keyboards/pica40/rev2/rev2.c index d05421150453..c7326f719b7d 100644 --- a/keyboards/pica40/rev2/rev2.c +++ b/keyboards/pica40/rev2/rev2.c @@ -14,9 +14,9 @@ void encoder_sync_slave_handler(uint8_t in_buflen, const void *in_data, uint8_t *(uint8_t *)out_data = readPin(ENCODER_PIN_B) ? 1 : 0; } -void encoder_init_pin(uint8_t index, bool pad_b) {} +void encoder_graycode_init_pin(uint8_t index, bool pad_b) {} -uint8_t encoder_read_pin(uint8_t index, bool pad_b) { +uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { if(pad_b) { uint8_t data = 0; transaction_rpc_recv(ENCODER_SYNC, sizeof(data), &data); From 37667154cfe80f9e354e28c970280153ad38f36a Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Thu, 20 Jul 2023 09:33:01 +1000 Subject: [PATCH 07/18] planck/rev7? --- drivers/encoder/encoder_graycode.c | 18 +++---- keyboards/planck/rev7/info.json | 9 +++- keyboards/planck/rev7/matrix.c | 86 ++++-------------------------- 3 files changed, 26 insertions(+), 87 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index 1075cbd61b96..5a2c452fe4db 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -68,8 +68,6 @@ static uint8_t thisHand, thatHand; static uint8_t thatCount; #endif -static uint8_t encoder_value[NUM_ENCODERS] = {0}; - __attribute__((weak)) void encoder_wait_pullup_charge(void) { wait_us(100); } @@ -92,7 +90,7 @@ __attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_ __attribute__((weak)) void encoder_graycode_post_init_kb(void) { extern void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); // Unused normally, but can be used for things like setting up pin-change interrupts in keyboard code. - // During the interrupt, call `encoder_handle_read()` with the pin states and it'll queue up an encoder event if needed. + // During the interrupt, read the pins then call `encoder_handle_read()` with the pin states and it'll queue up an encoder event if needed. } void encoder_graycode_post_init(void) { @@ -123,7 +121,6 @@ void encoder_driver_init(void) { // because all the arrays are static locals, rerunning tests in the same // executable doesn't reset any of these. Kinda crappy having test-only code // here, but it's the simplest solution. - memset(encoder_value, 0, sizeof(encoder_value)); memset(encoder_state, 0, sizeof(encoder_state)); memset(encoder_pulses, 0, sizeof(encoder_pulses)); static const pin_t encoders_pad_a_left[] = ENCODERS_PAD_A; @@ -146,7 +143,7 @@ void encoder_driver_init(void) { } #endif // defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) - // Encoder resolutions is handled purely master-side, so concatenate the two arrays + // Encoder resolutions is defined differently in config.h, so concatenate #if defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) # if defined(ENCODER_RESOLUTIONS_RIGHT) static const uint8_t encoder_resolutions_right[NUM_ENCODERS_RIGHT] = ENCODER_RESOLUTIONS_RIGHT; @@ -164,15 +161,16 @@ void encoder_driver_init(void) { static void encoder_handle_state_change(uint8_t index, uint8_t state) { uint8_t i = index; +#ifdef SPLIT_KEYBOARD + index += thisHand; +#endif + #ifdef ENCODER_RESOLUTIONS - const uint8_t resolution = encoder_resolutions[i]; + const uint8_t resolution = encoder_resolutions[index]; #else const uint8_t resolution = ENCODER_RESOLUTION; #endif -#ifdef SPLIT_KEYBOARD - index += thisHand; -#endif encoder_pulses[i] += encoder_LUT[state & 0xF]; #ifdef ENCODER_DEFAULT_POS @@ -182,7 +180,6 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { if (encoder_pulses[i] >= resolution) { #endif - encoder_value[index]++; encoder_queue_event(index, ENCODER_COUNTER_CLOCKWISE); } @@ -191,7 +188,6 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { #else if (encoder_pulses[i] <= -resolution) { // direction is arbitrary here, but this clockwise #endif - encoder_value[index]--; encoder_queue_event(index, ENCODER_CLOCKWISE); } encoder_pulses[i] %= resolution; diff --git a/keyboards/planck/rev7/info.json b/keyboards/planck/rev7/info.json index e4d743342366..27ec6d8b9c53 100644 --- a/keyboards/planck/rev7/info.json +++ b/keyboards/planck/rev7/info.json @@ -22,7 +22,14 @@ "diode_direction": "COL2ROW", "encoder": { "rotary": [ - {"pin_a": "B12", "pin_b": "B13"} + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, + {"pin_a": "B12", "pin_b": "B13", "resolution": 4} ] }, "features": { diff --git a/keyboards/planck/rev7/matrix.c b/keyboards/planck/rev7/matrix.c index c89c399a40d2..e4665a012ff9 100644 --- a/keyboards/planck/rev7/matrix.c +++ b/keyboards/planck/rev7/matrix.c @@ -32,31 +32,16 @@ #define STM32_IWDG_RL_MS(s) STM32_IWDG_RL_US(s * 1000.0) #define STM32_IWDG_RL_S(s) STM32_IWDG_RL_US(s * 1000000.0) -#if !defined(PLANCK_ENCODER_RESOLUTION) -# define PLANCK_ENCODER_RESOLUTION 4 -#endif - #if !defined(PLANCK_WATCHDOG_TIMEOUT) # define PLANCK_WATCHDOG_TIMEOUT 1.0 #endif -#ifdef ENCODER_MAP_ENABLE -#error "The encoder map feature is not currently supported by the Planck's encoder matrix" -#endif - /* matrix state(1:on, 0:off) */ static pin_t matrix_row_pins[MATRIX_ROWS] = MATRIX_ROW_PINS; static pin_t matrix_col_pins[MATRIX_COLS] = MATRIX_COL_PINS; static matrix_row_t matrix_inverted[MATRIX_COLS]; -#ifdef ENCODER_ENABLE -int8_t encoder_LUT[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0}; -uint8_t encoder_state[8] = {0}; -int8_t encoder_pulses[8] = {0}; -uint8_t encoder_value[8] = {0}; -#endif - void matrix_init_custom(void) { // actual matrix setup - cols for (int i = 0; i < MATRIX_COLS; i++) { @@ -84,31 +69,6 @@ void matrix_init_custom(void) { #endif } -#ifdef ENCODER_ENABLE -bool encoder_update(uint8_t index, uint8_t state) { - bool changed = false; - uint8_t i = index; - - encoder_pulses[i] += encoder_LUT[state & 0xF]; - - if (encoder_pulses[i] >= PLANCK_ENCODER_RESOLUTION) { - encoder_value[index]++; - changed = true; - encoder_update_kb(index, false); - } - if (encoder_pulses[i] <= -PLANCK_ENCODER_RESOLUTION) { - encoder_value[index]--; - changed = true; - encoder_update_kb(index, true); - } - encoder_pulses[i] %= PLANCK_ENCODER_RESOLUTION; -#ifdef ENCODER_DEFAULT_POS - encoder_pulses[i] = 0; -#endif - return changed; -} -#endif - bool matrix_scan_custom(matrix_row_t current_matrix[]) { #ifndef PLANCK_WATCHDOG_DISABLE // reset watchdog @@ -149,40 +109,16 @@ bool matrix_scan_custom(matrix_row_t current_matrix[]) { changed |= old != current_matrix[row]; } -#ifdef ENCODER_ENABLE - // encoder-matrix functionality - - // set up C/rows for encoder read - for (int i = 0; i < MATRIX_ROWS; i++) { - setPinOutput(matrix_row_pins[i]); - writePinHigh(matrix_row_pins[i]); - } - - // set up A & B for reading - setPinInputHigh(B12); - setPinInputHigh(B13); - - for (int i = 0; i < MATRIX_ROWS; i++) { - writePinLow(matrix_row_pins[i]); - wait_us(10); - uint8_t new_status = (palReadPad(GPIOB, 12) << 0) | (palReadPad(GPIOB, 13) << 1); - if ((encoder_state[i] & 0x3) != new_status) { - encoder_state[i] <<= 2; - encoder_state[i] |= new_status; - encoder_update(i, encoder_state[i]); - } - writePinHigh(matrix_row_pins[i]); - } - - // revert A & B to matrix state - setPinInputLow(B12); - setPinInputLow(B13); - - // revert C/rows to matrix state - for (int i = 0; i < MATRIX_ROWS; i++) { - setPinInputLow(matrix_row_pins[i]); - } -#endif - return changed; } + +uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { + pin_t pin = pad_b ? B13: B12; + setPinInputHigh(pin); + writePinLow(matrix_row_pins[index]); + wait_us(10); + uint8_t ret = readPin(pin) ? 1 : 0; + setPinInputLow(matrix_row_pins[index]); + setPinInputLow(pin); + return ret; +} From 9b29b46f2ec0c02ad94443afe1cdc63a573c2678 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sat, 11 Nov 2023 21:10:23 +1100 Subject: [PATCH 08/18] Fixup mechwild/sugarglider --- keyboards/mechwild/sugarglider/matrix.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/keyboards/mechwild/sugarglider/matrix.c b/keyboards/mechwild/sugarglider/matrix.c index 994cb0c54436..c76a8345d3b2 100644 --- a/keyboards/mechwild/sugarglider/matrix.c +++ b/keyboards/mechwild/sugarglider/matrix.c @@ -87,6 +87,11 @@ bool matrix_scan_custom(matrix_row_t current_matrix[]) { bool changed = false; for (uint8_t current_row = 0; current_row < MATRIX_ROWS; current_row++) { changed |= read_cols_on_row(current_matrix, current_row); + +#ifdef ENCODER_ENABLE + // Need to frequently read the encoder pins while scanning because the I/O expander takes a long time in comparison. + encoder_driver_task(); +#endif } return changed; } From d78625f752832ab0bf2d9ee36f1795ee6c1cdb91 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Wed, 13 Dec 2023 22:38:10 +1100 Subject: [PATCH 09/18] Update keyboards/planck/rev7/info.json Co-authored-by: Joel Challis --- keyboards/planck/rev7/info.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/keyboards/planck/rev7/info.json b/keyboards/planck/rev7/info.json index 7cf4e2194ada..6f3496eea415 100644 --- a/keyboards/planck/rev7/info.json +++ b/keyboards/planck/rev7/info.json @@ -25,14 +25,14 @@ }, "encoder": { "rotary": [ - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4}, - {"pin_a": "B12", "pin_b": "B13", "resolution": 4} + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"}, + {"pin_a": "B12", "pin_b": "B13"} ] }, "features": { From f5827bdb46d5ed52bb0662833a802f801e28923e Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Fri, 15 Dec 2023 21:09:12 +1100 Subject: [PATCH 10/18] Don't implement pin operations unless A/B pads are specified. --- drivers/encoder/encoder_graycode.c | 63 ++++++++++++++++++------------ quantum/encoder.h | 24 +++++++++--- 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index 5a2c452fe4db..93d103fd437a 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -34,15 +34,43 @@ # define ENCODER_RESOLUTION 4 #endif -#if !defined(ENCODERS_PAD_A) || !defined(ENCODERS_PAD_B) -# error "No encoder pads defined by ENCODERS_PAD_A and ENCODERS_PAD_B" +#undef ENCODER_DEFAULT_PIN_API_IMPL +#if defined(ENCODERS_PAD_A) && defined(ENCODERS_PAD_B) +// Inform the graycode driver that it needs to implement pin init/read functions +# define ENCODER_DEFAULT_PIN_API_IMPL #endif extern volatile bool isLeftHand; +__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b); +__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b); + +#ifdef ENCODER_DEFAULT_PIN_API_IMPL + static pin_t encoders_pad_a[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_A; static pin_t encoders_pad_b[NUM_ENCODERS_MAX_PER_SIDE] = ENCODERS_PAD_B; +__attribute__((weak)) void encoder_wait_pullup_charge(void) { + wait_us(100); +} + +__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b) { + pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; + if (pin != NO_PIN) { + setPinInputHigh(pin); + } +} + +__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { + pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; + if (pin != NO_PIN) { + return readPin(pin) ? 1 : 0; + } + return 0; +} + +#endif // ENCODER_DEFAULT_PIN_API_IMPL + #ifdef ENCODER_RESOLUTIONS static uint8_t encoder_resolutions[NUM_ENCODERS] = ENCODER_RESOLUTIONS; #endif @@ -68,25 +96,6 @@ static uint8_t thisHand, thatHand; static uint8_t thatCount; #endif -__attribute__((weak)) void encoder_wait_pullup_charge(void) { - wait_us(100); -} - -__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b) { - pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; - if (pin != NO_PIN) { - setPinInputHigh(pin); - } -} - -__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { - pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; - if (pin != NO_PIN) { - return readPin(pin) ? 1 : 0; - } - return 0; -} - __attribute__((weak)) void encoder_graycode_post_init_kb(void) { extern void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); // Unused normally, but can be used for things like setting up pin-change interrupts in keyboard code. @@ -94,6 +103,7 @@ __attribute__((weak)) void encoder_graycode_post_init_kb(void) { } void encoder_graycode_post_init(void) { +#ifdef ENCODER_DEFAULT_PIN_API_IMPL for (uint8_t i = 0; i < thisCount; i++) { encoder_graycode_init_pin(i, false); encoder_graycode_init_pin(i, true); @@ -102,6 +112,9 @@ void encoder_graycode_post_init(void) { for (uint8_t i = 0; i < thisCount; i++) { encoder_state[i] = (encoder_graycode_read_pin(i, false) << 0) | (encoder_graycode_read_pin(i, true) << 1); } +#else + memset(encoder_state, 0, sizeof(encoder_state)); +#endif encoder_graycode_post_init_kb(); } @@ -123,8 +136,8 @@ void encoder_driver_init(void) { // here, but it's the simplest solution. memset(encoder_state, 0, sizeof(encoder_state)); memset(encoder_pulses, 0, sizeof(encoder_pulses)); - static const pin_t encoders_pad_a_left[] = ENCODERS_PAD_A; - static const pin_t encoders_pad_b_left[] = ENCODERS_PAD_B; + const pin_t encoders_pad_a_left[] = ENCODERS_PAD_A; + const pin_t encoders_pad_b_left[] = ENCODERS_PAD_B; for (uint8_t i = 0; i < thisCount; i++) { encoders_pad_a[i] = encoders_pad_a_left[i]; encoders_pad_b[i] = encoders_pad_b_left[i]; @@ -134,8 +147,8 @@ void encoder_driver_init(void) { #if defined(SPLIT_KEYBOARD) && defined(ENCODERS_PAD_A_RIGHT) && defined(ENCODERS_PAD_B_RIGHT) // Re-initialise the pads if it's the right-hand side if (!isLeftHand) { - static const pin_t encoders_pad_a_right[] = ENCODERS_PAD_A_RIGHT; - static const pin_t encoders_pad_b_right[] = ENCODERS_PAD_B_RIGHT; + const pin_t encoders_pad_a_right[] = ENCODERS_PAD_A_RIGHT; + const pin_t encoders_pad_b_right[] = ENCODERS_PAD_B_RIGHT; for (uint8_t i = 0; i < thisCount; i++) { encoders_pad_a[i] = encoders_pad_a_right[i]; encoders_pad_b[i] = encoders_pad_b_right[i]; diff --git a/quantum/encoder.h b/quantum/encoder.h index 7da7ffbb1e53..90414a43a06e 100644 --- a/quantum/encoder.h +++ b/quantum/encoder.h @@ -36,17 +36,29 @@ bool encoder_update_user(uint8_t index, bool clockwise); # ifdef SPLIT_KEYBOARD # if defined(ENCODERS_PAD_A_RIGHT) -# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) -# define NUM_ENCODERS_RIGHT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A_RIGHT)) +# ifndef NUM_ENCODERS_LEFT +# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# endif +# ifndef NUM_ENCODERS_RIGHT +# define NUM_ENCODERS_RIGHT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A_RIGHT)) +# endif # else -# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) -# define NUM_ENCODERS_RIGHT NUM_ENCODERS_LEFT +# ifndef NUM_ENCODERS_LEFT +# define NUM_ENCODERS_LEFT ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# endif +# ifndef NUM_ENCODERS_RIGHT +# define NUM_ENCODERS_RIGHT NUM_ENCODERS_LEFT +# endif +# endif +# ifndef NUM_ENCODERS +# define NUM_ENCODERS (NUM_ENCODERS_LEFT + NUM_ENCODERS_RIGHT) # endif -# define NUM_ENCODERS (NUM_ENCODERS_LEFT + NUM_ENCODERS_RIGHT) # else // SPLIT_KEYBOARD -# define NUM_ENCODERS ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# ifndef NUM_ENCODERS +# define NUM_ENCODERS ARRAY_SIZE(((pin_t[])ENCODERS_PAD_A)) +# endif # define NUM_ENCODERS_LEFT NUM_ENCODERS # define NUM_ENCODERS_RIGHT 0 From 880c39b7dda1ee1616882138382c5f2800b6efea Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sat, 16 Dec 2023 21:00:45 +1100 Subject: [PATCH 11/18] Add support for `encoder.driver` to info.json --- data/mappings/info_rules.hjson | 1 + data/schemas/keyboard.jsonschema | 4 ++++ keyboards/tzarc/djinn/info.json | 1 + keyboards/tzarc/ghoul/info.json | 3 +++ 4 files changed, 9 insertions(+) diff --git a/data/mappings/info_rules.hjson b/data/mappings/info_rules.hjson index 02fc2bee9de6..fc25eb3328b2 100644 --- a/data/mappings/info_rules.hjson +++ b/data/mappings/info_rules.hjson @@ -21,6 +21,7 @@ "DEBOUNCE_TYPE": {"info_key": "build.debounce_type"}, "EEPROM_DRIVER": {"info_key": "eeprom.driver"}, "ENCODER_ENABLE": {"info_key": "encoder.enabled", "value_type": "bool"}, + "ENCODER_DRIVER": {"info_key": "encoder.driver"}, "FIRMWARE_FORMAT": {"info_key": "build.firmware_format"}, "KEYBOARD_SHARED_EP": {"info_key": "usb.shared_endpoint.keyboard", "value_type": "bool"}, "LAYOUTS": {"info_key": "community_layouts", "value_type": "list"}, diff --git a/data/schemas/keyboard.jsonschema b/data/schemas/keyboard.jsonschema index 299695808486..b748867dd372 100644 --- a/data/schemas/keyboard.jsonschema +++ b/data/schemas/keyboard.jsonschema @@ -6,6 +6,10 @@ "encoder_config": { "type": "object", "properties": { + "driver": { + "type": "string", + "enum": ["graycode", "custom"] + }, "rotary": { "type": "array", "items": { diff --git a/keyboards/tzarc/djinn/info.json b/keyboards/tzarc/djinn/info.json index 64ed1da6904c..54163cc2da56 100644 --- a/keyboards/tzarc/djinn/info.json +++ b/keyboards/tzarc/djinn/info.json @@ -64,6 +64,7 @@ } }, "encoder": { + "driver": "graycode", "rotary": [ {"pin_a": "C14", "pin_b": "C15", "resolution": 2} ] diff --git a/keyboards/tzarc/ghoul/info.json b/keyboards/tzarc/ghoul/info.json index 26824ca4da9c..14f15b5d6106 100644 --- a/keyboards/tzarc/ghoul/info.json +++ b/keyboards/tzarc/ghoul/info.json @@ -18,6 +18,9 @@ "quantum_painter": true, "rgb_matrix": true }, + "encoder": { + "driver": "graycode" + }, "rgb_matrix": { "driver": "ws2812" }, From 10e98c73e1f82485aab072fff531b0b418c87db0 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sat, 16 Dec 2023 21:54:34 +1100 Subject: [PATCH 12/18] License year updates. --- drivers/encoder/encoder_graycode.c | 19 +++---------------- quantum/encoder.c | 2 +- quantum/encoder/tests/config_mock.h | 2 +- .../tests/config_mock_split_left_eq_right.h | 2 +- .../tests/config_mock_split_left_gt_right.h | 2 +- .../tests/config_mock_split_left_lt_right.h | 2 +- .../encoder/tests/config_mock_split_no_left.h | 2 +- .../tests/config_mock_split_no_right.h | 2 +- .../encoder/tests/config_mock_split_role.h | 2 +- 9 files changed, 11 insertions(+), 24 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index 93d103fd437a..ae10c3749737 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -1,19 +1,6 @@ -/* - * Copyright 2018 Jack Humbert - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ +// Copyright 2018 Jack Humbert +// Copyright 2018-2023 Nick Brassel (@tzarc) +// SPDX-License-Identifier: GPL-2.0-or-later #include #include "encoder.h" diff --git a/quantum/encoder.c b/quantum/encoder.c index 55b8952a403c..c2763de09629 100644 --- a/quantum/encoder.c +++ b/quantum/encoder.c @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #include diff --git a/quantum/encoder/tests/config_mock.h b/quantum/encoder/tests/config_mock.h index 59c423972a8f..9eb59ddc8813 100644 --- a/quantum/encoder/tests/config_mock.h +++ b/quantum/encoder/tests/config_mock.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_left_eq_right.h b/quantum/encoder/tests/config_mock_split_left_eq_right.h index 4ebe4c452d72..ea795657ef17 100644 --- a/quantum/encoder/tests/config_mock_split_left_eq_right.h +++ b/quantum/encoder/tests/config_mock_split_left_eq_right.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_left_gt_right.h b/quantum/encoder/tests/config_mock_split_left_gt_right.h index 4c230ea460c4..abcfe0391896 100644 --- a/quantum/encoder/tests/config_mock_split_left_gt_right.h +++ b/quantum/encoder/tests/config_mock_split_left_gt_right.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_left_lt_right.h b/quantum/encoder/tests/config_mock_split_left_lt_right.h index fa935cedcc6e..075c774b0d11 100644 --- a/quantum/encoder/tests/config_mock_split_left_lt_right.h +++ b/quantum/encoder/tests/config_mock_split_left_lt_right.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_no_left.h b/quantum/encoder/tests/config_mock_split_no_left.h index a4464978509d..dfd8358929bc 100644 --- a/quantum/encoder/tests/config_mock_split_no_left.h +++ b/quantum/encoder/tests/config_mock_split_no_left.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_no_right.h b/quantum/encoder/tests/config_mock_split_no_right.h index 8b10e49bfba8..5683eade8c97 100644 --- a/quantum/encoder/tests/config_mock_split_no_right.h +++ b/quantum/encoder/tests/config_mock_split_no_right.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" diff --git a/quantum/encoder/tests/config_mock_split_role.h b/quantum/encoder/tests/config_mock_split_role.h index 4ebe4c452d72..ea795657ef17 100644 --- a/quantum/encoder/tests/config_mock_split_role.h +++ b/quantum/encoder/tests/config_mock_split_role.h @@ -1,4 +1,4 @@ -// Copyright 2022 Nick Brassel (@tzarc) +// Copyright 2022-2023 Nick Brassel (@tzarc) // SPDX-License-Identifier: GPL-2.0-or-later #pragma once #include "config_encoder_common.h" From 6fc132ef78ac37190919ad78226e961e0a18c76c Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 14:51:39 +1100 Subject: [PATCH 13/18] Review comments. --- keyboards/ploopyco/mouse/config.h | 3 +++ keyboards/ploopyco/trackball/config.h | 3 +++ keyboards/ploopyco/trackball_mini/config.h | 3 +++ keyboards/ploopyco/trackball_thumb/config.h | 3 +++ keyboards/ploopyco/trackball_thumb/info.json | 5 ----- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/keyboards/ploopyco/mouse/config.h b/keyboards/ploopyco/mouse/config.h index 94bd6ef6af68..86af11fc94bb 100644 --- a/keyboards/ploopyco/mouse/config.h +++ b/keyboards/ploopyco/mouse/config.h @@ -32,3 +32,6 @@ /* PMW33XX Settings */ #define PMW33XX_CS_PIN B0 + +/* Custom encoder needs to specify just how many encoders we have */ +#define NUM_ENCODERS 1 diff --git a/keyboards/ploopyco/trackball/config.h b/keyboards/ploopyco/trackball/config.h index de0ed1ebece5..a1f3695d8158 100644 --- a/keyboards/ploopyco/trackball/config.h +++ b/keyboards/ploopyco/trackball/config.h @@ -31,3 +31,6 @@ /* PMW33XX Settings */ #define PMW33XX_CS_PIN B0 #define POINTING_DEVICE_INVERT_Y + +/* Custom encoder needs to specify just how many encoders we have */ +#define NUM_ENCODERS 1 diff --git a/keyboards/ploopyco/trackball_mini/config.h b/keyboards/ploopyco/trackball_mini/config.h index 789deb5a5f14..6b92563fa9ef 100644 --- a/keyboards/ploopyco/trackball_mini/config.h +++ b/keyboards/ploopyco/trackball_mini/config.h @@ -32,3 +32,6 @@ #define ADNS5050_CS_PIN B4 #define POINTING_DEVICE_ROTATION_270 + +/* Custom encoder needs to specify just how many encoders we have */ +#define NUM_ENCODERS 1 diff --git a/keyboards/ploopyco/trackball_thumb/config.h b/keyboards/ploopyco/trackball_thumb/config.h index 8fc084c38ce6..316755f68669 100644 --- a/keyboards/ploopyco/trackball_thumb/config.h +++ b/keyboards/ploopyco/trackball_thumb/config.h @@ -31,3 +31,6 @@ /* PMW3360 Settings */ #define POINTING_DEVICE_CS_PIN B0 + +/* Custom encoder needs to specify just how many encoders we have */ +#define NUM_ENCODERS 1 diff --git a/keyboards/ploopyco/trackball_thumb/info.json b/keyboards/ploopyco/trackball_thumb/info.json index c9c81d8f7568..d75e066322c2 100644 --- a/keyboards/ploopyco/trackball_thumb/info.json +++ b/keyboards/ploopyco/trackball_thumb/info.json @@ -20,11 +20,6 @@ "pointing_device": true, "encoder": true }, - "encoder": { - "rotary": [ - {"pin_a": "NO_PIN", "pin_b": "NO_PIN"} - ] - }, "layouts": { "LAYOUT": { "layout": [ From b6bd9c86b78a57aa6adaffbc917c855951485b4c Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 15:01:00 +1100 Subject: [PATCH 14/18] Move ploopy encoder configs to `info.json` where possible. --- keyboards/ploopyco/mouse/info.json | 6 ++++++ keyboards/ploopyco/mouse/rules.mk | 3 --- keyboards/ploopyco/trackball/info.json | 6 ++++++ keyboards/ploopyco/trackball/rules.mk | 3 --- keyboards/ploopyco/trackball_mini/info.json | 6 ++++++ keyboards/ploopyco/trackball_mini/rules.mk | 3 --- keyboards/ploopyco/trackball_thumb/info.json | 3 +++ keyboards/ploopyco/trackball_thumb/rules.mk | 1 - 8 files changed, 21 insertions(+), 10 deletions(-) diff --git a/keyboards/ploopyco/mouse/info.json b/keyboards/ploopyco/mouse/info.json index 5dc53392ff24..1f110aad71fe 100644 --- a/keyboards/ploopyco/mouse/info.json +++ b/keyboards/ploopyco/mouse/info.json @@ -31,6 +31,12 @@ ["D4", "D2", "E6", "B6", "D7", "C6", "C7", "B7"] ] }, + "features": { + "encoder": true + }, + "encoder": { + "driver": "custom" + }, "layouts": { "LAYOUT": { "layout": [ diff --git a/keyboards/ploopyco/mouse/rules.mk b/keyboards/ploopyco/mouse/rules.mk index 334de8d9d1f6..635695078058 100644 --- a/keyboards/ploopyco/mouse/rules.mk +++ b/keyboards/ploopyco/mouse/rules.mk @@ -16,9 +16,6 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = pmw3360 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE = yes -ENCODER_DRIVER = custom - ANALOG_DRIVER_REQUIRED = yes SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball/info.json b/keyboards/ploopyco/trackball/info.json index 2c30755aa1e9..110264ef1c63 100644 --- a/keyboards/ploopyco/trackball/info.json +++ b/keyboards/ploopyco/trackball/info.json @@ -12,6 +12,12 @@ "bootmagic": { "matrix": [0, 3] }, + "features": { + "encoder": true + }, + "encoder": { + "driver": "custom" + }, "layouts": { "LAYOUT": { "layout": [ diff --git a/keyboards/ploopyco/trackball/rules.mk b/keyboards/ploopyco/trackball/rules.mk index a8fd84efa1f2..ca1a8fd17a13 100644 --- a/keyboards/ploopyco/trackball/rules.mk +++ b/keyboards/ploopyco/trackball/rules.mk @@ -16,9 +16,6 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = pmw3360 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE = yes -ENCODER_DRIVER = custom - ANALOG_DRIVER_REQUIRED = yes SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball_mini/info.json b/keyboards/ploopyco/trackball_mini/info.json index 301c8d18ec57..0e7b12d20d36 100644 --- a/keyboards/ploopyco/trackball_mini/info.json +++ b/keyboards/ploopyco/trackball_mini/info.json @@ -14,6 +14,12 @@ }, "processor": "atmega32u4", "bootloader": "atmel-dfu", + "features": { + "encoder": true + }, + "encoder": { + "driver": "custom" + }, "layouts": { "LAYOUT": { "layout": [ diff --git a/keyboards/ploopyco/trackball_mini/rules.mk b/keyboards/ploopyco/trackball_mini/rules.mk index c2f264ed01b4..d2bacc397404 100644 --- a/keyboards/ploopyco/trackball_mini/rules.mk +++ b/keyboards/ploopyco/trackball_mini/rules.mk @@ -13,9 +13,6 @@ POINTING_DEVICE_ENABLE = yes POINTING_DEVICE_DRIVER = adns5050 MOUSEKEY_ENABLE = yes # Mouse keys -ENCODER_ENABLE = yes -ENCODER_DRIVER = custom - ANALOG_DRIVER_REQUIRED = yes SRC += opt_encoder.c diff --git a/keyboards/ploopyco/trackball_thumb/info.json b/keyboards/ploopyco/trackball_thumb/info.json index d75e066322c2..e27bf472521f 100644 --- a/keyboards/ploopyco/trackball_thumb/info.json +++ b/keyboards/ploopyco/trackball_thumb/info.json @@ -20,6 +20,9 @@ "pointing_device": true, "encoder": true }, + "encoder": { + "driver": "custom" + }, "layouts": { "LAYOUT": { "layout": [ diff --git a/keyboards/ploopyco/trackball_thumb/rules.mk b/keyboards/ploopyco/trackball_thumb/rules.mk index 376692ea093f..6b82d7734b43 100644 --- a/keyboards/ploopyco/trackball_thumb/rules.mk +++ b/keyboards/ploopyco/trackball_thumb/rules.mk @@ -2,7 +2,6 @@ F_CPU = 8000000 POINTING_DEVICE_DRIVER = pmw3360 -ENCODER_DRIVER = custom ANALOG_DRIVER_REQUIRED = yes From d82d43029b1af9023cea2a3333d8d9ec2d92b797 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 15:03:36 +1100 Subject: [PATCH 15/18] Remove unnecessary testing changes. --- keyboards/tzarc/djinn/info.json | 1 - keyboards/tzarc/ghoul/info.json | 3 --- 2 files changed, 4 deletions(-) diff --git a/keyboards/tzarc/djinn/info.json b/keyboards/tzarc/djinn/info.json index 54163cc2da56..64ed1da6904c 100644 --- a/keyboards/tzarc/djinn/info.json +++ b/keyboards/tzarc/djinn/info.json @@ -64,7 +64,6 @@ } }, "encoder": { - "driver": "graycode", "rotary": [ {"pin_a": "C14", "pin_b": "C15", "resolution": 2} ] diff --git a/keyboards/tzarc/ghoul/info.json b/keyboards/tzarc/ghoul/info.json index 14f15b5d6106..26824ca4da9c 100644 --- a/keyboards/tzarc/ghoul/info.json +++ b/keyboards/tzarc/ghoul/info.json @@ -18,9 +18,6 @@ "quantum_painter": true, "rgb_matrix": true }, - "encoder": { - "driver": "graycode" - }, "rgb_matrix": { "driver": "ws2812" }, From 35d39e3719b5f3dc38e4647d77de490ba5d757d9 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 15:08:38 +1100 Subject: [PATCH 16/18] Review comments. --- quantum/encoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quantum/encoder.c b/quantum/encoder.c index c2763de09629..735eb1cd71f6 100644 --- a/quantum/encoder.c +++ b/quantum/encoder.c @@ -54,12 +54,12 @@ static bool encoder_handle_queue(void) { bool encoder_task(void) { bool changed = false; -#ifdef ENCODER_ENABLE +#ifdef SPLIT_KEYBOARD // Attempt to process existing encoder events in case split handling has already enqueued events if (should_process_encoder()) { changed |= encoder_handle_queue(); } -#endif // ENCODER_ENABLE +#endif // SPLIT_KEYBOARD // Let the encoder driver produce events encoder_driver_task(); From 6bc955f94adb973e2f3c0a236c5d77ac93c7d63a Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 15:42:22 +1100 Subject: [PATCH 17/18] Apply suggestions from code review Co-authored-by: Drashna Jaelre --- drivers/encoder/encoder_graycode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_graycode.c index ae10c3749737..fe93d20d9068 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_graycode.c @@ -44,14 +44,14 @@ __attribute__((weak)) void encoder_wait_pullup_charge(void) { __attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { - setPinInputHigh(pin); + gpio_set_pin_input_high(pin); } } __attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { - return readPin(pin) ? 1 : 0; + return gpio_read_pin(pin) ? 1 : 0; } return 0; } From 244b1a6ecb6c2895a144feea1b4790b0e47c8446 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Sun, 18 Feb 2024 16:09:06 +1100 Subject: [PATCH 18/18] Change `graycode` => `quadrature`. --- builddefs/common_features.mk | 4 +-- data/schemas/keyboard.jsonschema | 2 +- ...ncoder_graycode.c => encoder_quadrature.c} | 30 +++++++++---------- keyboards/pica40/rev2/rev2.c | 4 +-- keyboards/planck/rev7/matrix.c | 2 +- quantum/encoder/tests/rules.mk | 14 ++++----- 6 files changed, 28 insertions(+), 28 deletions(-) rename drivers/encoder/{encoder_graycode.c => encoder_quadrature.c} (84%) diff --git a/builddefs/common_features.mk b/builddefs/common_features.mk index ed189f42522b..58e41f523027 100644 --- a/builddefs/common_features.mk +++ b/builddefs/common_features.mk @@ -887,8 +887,8 @@ ifeq ($(strip $(BLUETOOTH_ENABLE)), yes) endif ENCODER_ENABLE ?= no -ENCODER_DRIVER ?= graycode -VALID_ENCODER_DRIVER_TYPES := graycode custom +ENCODER_DRIVER ?= quadrature +VALID_ENCODER_DRIVER_TYPES := quadrature custom ifeq ($(strip $(ENCODER_ENABLE)), yes) ifeq ($(filter $(ENCODER_DRIVER),$(VALID_ENCODER_DRIVER_TYPES)),) $(call CATASTROPHIC_ERROR,Invalid ENCODER_DRIVER,ENCODER_DRIVER="$(ENCODER_DRIVER)" is not a valid encoder driver) diff --git a/data/schemas/keyboard.jsonschema b/data/schemas/keyboard.jsonschema index 3521a954cdec..79668fe386b4 100644 --- a/data/schemas/keyboard.jsonschema +++ b/data/schemas/keyboard.jsonschema @@ -8,7 +8,7 @@ "properties": { "driver": { "type": "string", - "enum": ["graycode", "custom"] + "enum": ["quadrature", "custom"] }, "rotary": { "type": "array", diff --git a/drivers/encoder/encoder_graycode.c b/drivers/encoder/encoder_quadrature.c similarity index 84% rename from drivers/encoder/encoder_graycode.c rename to drivers/encoder/encoder_quadrature.c index ae10c3749737..0369c2127203 100644 --- a/drivers/encoder/encoder_graycode.c +++ b/drivers/encoder/encoder_quadrature.c @@ -23,14 +23,14 @@ #undef ENCODER_DEFAULT_PIN_API_IMPL #if defined(ENCODERS_PAD_A) && defined(ENCODERS_PAD_B) -// Inform the graycode driver that it needs to implement pin init/read functions +// Inform the quadrature driver that it needs to implement pin init/read functions # define ENCODER_DEFAULT_PIN_API_IMPL #endif extern volatile bool isLeftHand; -__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b); -__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b); +__attribute__((weak)) void encoder_quadrature_init_pin(uint8_t index, bool pad_b); +__attribute__((weak)) uint8_t encoder_quadrature_read_pin(uint8_t index, bool pad_b); #ifdef ENCODER_DEFAULT_PIN_API_IMPL @@ -41,14 +41,14 @@ __attribute__((weak)) void encoder_wait_pullup_charge(void) { wait_us(100); } -__attribute__((weak)) void encoder_graycode_init_pin(uint8_t index, bool pad_b) { +__attribute__((weak)) void encoder_quadrature_init_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { setPinInputHigh(pin); } } -__attribute__((weak)) uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { +__attribute__((weak)) uint8_t encoder_quadrature_read_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? encoders_pad_b[index] : encoders_pad_a[index]; if (pin != NO_PIN) { return readPin(pin) ? 1 : 0; @@ -83,27 +83,27 @@ static uint8_t thisHand, thatHand; static uint8_t thatCount; #endif -__attribute__((weak)) void encoder_graycode_post_init_kb(void) { - extern void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); +__attribute__((weak)) void encoder_quadrature_post_init_kb(void) { + extern void encoder_quadrature_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state); // Unused normally, but can be used for things like setting up pin-change interrupts in keyboard code. // During the interrupt, read the pins then call `encoder_handle_read()` with the pin states and it'll queue up an encoder event if needed. } -void encoder_graycode_post_init(void) { +void encoder_quadrature_post_init(void) { #ifdef ENCODER_DEFAULT_PIN_API_IMPL for (uint8_t i = 0; i < thisCount; i++) { - encoder_graycode_init_pin(i, false); - encoder_graycode_init_pin(i, true); + encoder_quadrature_init_pin(i, false); + encoder_quadrature_init_pin(i, true); } encoder_wait_pullup_charge(); for (uint8_t i = 0; i < thisCount; i++) { - encoder_state[i] = (encoder_graycode_read_pin(i, false) << 0) | (encoder_graycode_read_pin(i, true) << 1); + encoder_state[i] = (encoder_quadrature_read_pin(i, false) << 0) | (encoder_quadrature_read_pin(i, true) << 1); } #else memset(encoder_state, 0, sizeof(encoder_state)); #endif - encoder_graycode_post_init_kb(); + encoder_quadrature_post_init_kb(); } void encoder_driver_init(void) { @@ -155,7 +155,7 @@ void encoder_driver_init(void) { } #endif // defined(SPLIT_KEYBOARD) && defined(ENCODER_RESOLUTIONS) - encoder_graycode_post_init(); + encoder_quadrature_post_init(); } static void encoder_handle_state_change(uint8_t index, uint8_t state) { @@ -197,7 +197,7 @@ static void encoder_handle_state_change(uint8_t index, uint8_t state) { #endif } -void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state) { +void encoder_quadrature_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pin_b_state) { uint8_t state = pin_a_state | (pin_b_state << 1); if ((encoder_state[index] & 0x3) != state) { encoder_state[index] <<= 2; @@ -208,6 +208,6 @@ void encoder_graycode_handle_read(uint8_t index, uint8_t pin_a_state, uint8_t pi __attribute__((weak)) void encoder_driver_task(void) { for (uint8_t i = 0; i < thisCount; i++) { - encoder_graycode_handle_read(i, encoder_graycode_read_pin(i, false), encoder_graycode_read_pin(i, true)); + encoder_quadrature_handle_read(i, encoder_quadrature_read_pin(i, false), encoder_quadrature_read_pin(i, true)); } } diff --git a/keyboards/pica40/rev2/rev2.c b/keyboards/pica40/rev2/rev2.c index c7326f719b7d..2ee73dcc6b8f 100644 --- a/keyboards/pica40/rev2/rev2.c +++ b/keyboards/pica40/rev2/rev2.c @@ -14,9 +14,9 @@ void encoder_sync_slave_handler(uint8_t in_buflen, const void *in_data, uint8_t *(uint8_t *)out_data = readPin(ENCODER_PIN_B) ? 1 : 0; } -void encoder_graycode_init_pin(uint8_t index, bool pad_b) {} +void encoder_quadrature_init_pin(uint8_t index, bool pad_b) {} -uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { +uint8_t encoder_quadrature_read_pin(uint8_t index, bool pad_b) { if(pad_b) { uint8_t data = 0; transaction_rpc_recv(ENCODER_SYNC, sizeof(data), &data); diff --git a/keyboards/planck/rev7/matrix.c b/keyboards/planck/rev7/matrix.c index e4665a012ff9..350ce93ce049 100644 --- a/keyboards/planck/rev7/matrix.c +++ b/keyboards/planck/rev7/matrix.c @@ -112,7 +112,7 @@ bool matrix_scan_custom(matrix_row_t current_matrix[]) { return changed; } -uint8_t encoder_graycode_read_pin(uint8_t index, bool pad_b) { +uint8_t encoder_quadrature_read_pin(uint8_t index, bool pad_b) { pin_t pin = pad_b ? B13: B12; setPinInputHigh(pin); writePinLow(matrix_row_pins[index]); diff --git a/quantum/encoder/tests/rules.mk b/quantum/encoder/tests/rules.mk index feaea0b2acf8..eb6106039ee0 100644 --- a/quantum/encoder/tests/rules.mk +++ b/quantum/encoder/tests/rules.mk @@ -3,7 +3,7 @@ encoder_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock.h encoder_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests.cpp \ $(QUANTUM_PATH)/encoder.c @@ -14,7 +14,7 @@ encoder_split_left_eq_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_eq_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_eq_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -25,7 +25,7 @@ encoder_split_left_gt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_gt_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_gt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -36,7 +36,7 @@ encoder_split_left_lt_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_ encoder_split_left_lt_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_left_lt_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -47,7 +47,7 @@ encoder_split_no_left_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_ encoder_split_no_left_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_left.cpp \ $(QUANTUM_PATH)/encoder.c @@ -58,7 +58,7 @@ encoder_split_no_right_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split encoder_split_no_right_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_right.cpp \ $(QUANTUM_PATH)/encoder.c @@ -69,7 +69,7 @@ encoder_split_role_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_rol encoder_split_role_SRC := \ platforms/test/timer.c \ - drivers/encoder/encoder_graycode.c \ + drivers/encoder/encoder_quadrature.c \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_role.cpp \ $(QUANTUM_PATH)/encoder.c