From 8c8d7d2c6c8e27763348741d92bd327fb97eaaad Mon Sep 17 00:00:00 2001 From: Chris Leishman Date: Tue, 10 Mar 2026 16:37:26 -0700 Subject: [PATCH] Fix enable_internal_pullup to always use pull-ups Previously, the pull direction was based on btn_pressed_level, which caused pull-downs to be applied to encoder pins when btn_pressed_level was 1. Encoder pins always need pull-ups since they are wired with common to ground. Now always uses pull-ups and documents that the button pin may need a separate pull-down when btn_pressed_level is 1. Fixes #12 --- .eil.yml | 2 +- encoder.c | 46 ++++++++++++++++++---------------------------- encoder.h | 14 ++++++++++++-- idf_component.yml | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/.eil.yml b/.eil.yml index 3a572ed..ba74707 100644 --- a/.eil.yml +++ b/.eil.yml @@ -1,7 +1,7 @@ --- name: encoder description: HW timer-based driver for incremental rotary encoders -version: 3.0.0 +version: 3.0.1 groups: - input code_owners: diff --git a/encoder.c b/encoder.c index 14003b5..fd3bf4b 100644 --- a/encoder.c +++ b/encoder.c @@ -224,40 +224,30 @@ esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_en } #endif - // setup GPIO - gpio_config_t io_conf; - memset(&io_conf, 0, sizeof(gpio_config_t)); - io_conf.mode = GPIO_MODE_INPUT; - if (config->enable_internal_pullup) + // setup GPIO pins as inputs + gpio_num_t pins[] = { re->pin_a, re->pin_b, re->pin_btn }; + int num_pins = PIN_VALID(re->pin_btn) ? 3 : 2; + esp_err_t err; + for (int i = 0; i < num_pins; i++) { - if (re->btn_pressed_level == 0) + err = gpio_set_direction(pins[i], GPIO_MODE_INPUT); + if (err != ESP_OK) { - io_conf.pull_up_en = GPIO_PULLUP_ENABLE; - io_conf.pull_down_en = GPIO_PULLDOWN_DISABLE; + vSemaphoreDelete(re->lock); + free(re); + return err; } - else + if (config->enable_internal_pullup) { - io_conf.pull_up_en = GPIO_PULLUP_DISABLE; - io_conf.pull_down_en = GPIO_PULLDOWN_ENABLE; + err = gpio_set_pull_mode(pins[i], GPIO_PULLUP_ONLY); + if (err != ESP_OK) + { + vSemaphoreDelete(re->lock); + free(re); + return err; + } } } - else - { - io_conf.pull_up_en = GPIO_PULLUP_DISABLE; - io_conf.pull_down_en = GPIO_PULLDOWN_DISABLE; - } - io_conf.intr_type = GPIO_INTR_DISABLE; - io_conf.pin_bit_mask = GPIO_BIT(re->pin_a) | GPIO_BIT(re->pin_b); - if (PIN_VALID(re->pin_btn)) - io_conf.pin_bit_mask |= GPIO_BIT(re->pin_btn); - - esp_err_t err = gpio_config(&io_conf); - if (err != ESP_OK) - { - vSemaphoreDelete(re->lock); - free(re); - return err; - } // Create and start per-encoder timer const esp_timer_create_args_t timer_args = diff --git a/encoder.h b/encoder.h index f03b8b2..abb44fe 100644 --- a/encoder.h +++ b/encoder.h @@ -96,8 +96,18 @@ typedef struct gpio_num_t pin_a; //!< Encoder pin A gpio_num_t pin_b; //!< Encoder pin B gpio_num_t pin_btn; //!< Button pin, or GPIO_NUM_NC if unused - uint8_t btn_pressed_level; //!< GPIO level when button is pressed (0 or 1) - bool enable_internal_pullup; //!< Enable internal pull-up/pull-down resistors on GPIO pins + uint8_t btn_pressed_level; //!< GPIO level when button is pressed (0 or 1). + //!< When set to 1, the button pin may need a pull-down + //!< resistor, either external or set via \c gpio_set_pull_mode() + //!< before creating the encoder, as \c enable_internal_pullup + //!< only enables pull-ups. + bool enable_internal_pullup; //!< Enable internal pull-up resistors on all encoder and + //!< button GPIO pins. Pull-ups are always used, which is + //!< correct for encoders wired with common to ground. When + //!< \c btn_pressed_level is 1, the button pin may need a + //!< pull-down instead — use an external resistor or call + //!< \c gpio_set_pull_mode() before creating the encoder. + //!< When false, pull resistors are left unchanged. uint32_t btn_dead_time_us; //!< Button dead time in microseconds uint32_t btn_long_press_time_us; //!< Long press threshold in microseconds uint32_t acceleration_threshold_ms; //!< Acceleration threshold in milliseconds (acceleration starts below this interval) diff --git a/idf_component.yml b/idf_component.yml index 5f39d48..854aa25 100644 --- a/idf_component.yml +++ b/idf_component.yml @@ -1,5 +1,5 @@ --- -version: 3.0.0 +version: 3.0.1 description: HW timer-based driver for incremental rotary encoders license: BSD-3 targets: