From 461beed04f405317c52a5982b90513118c36b5e3 Mon Sep 17 00:00:00 2001 From: Thomas Faour Date: Sun, 19 Jul 2026 09:22:40 -0400 Subject: [PATCH] Fix: release GPIO hold on every button pin before reconfiguring it ESP-IDF locks ("holds") every pin armed as a GPIO deep-sleep wakeup source across the sleep transition, and never releases it automatically on wake -- confirmed against sleep_modes.c's esp_sleep_gpio_wakeup_prepare_on_hp_periph_powerdown(), which calls gpio_hold_en() with no corresponding gpio_hold_dis() anywhere in ESP-IDF's own wake path. Left held, live gpio_get_level() reads stay frozen at whatever level the pin had when sleep began (almost always "not pressed"), which is indistinguishable from a real "not pressed" reading and silently broke any live poll for a *new* press later in the same awake session. This never surfaced before the manage menu's escalation feature, since every other button check either used the latched wakeup-status register (unaffected by hold) or only polled once, early in boot, before any sleep/wake cycle in that session. wait_for_button_press() is the first code in this project to repeatedly poll a button live *after* having just woken via that same pin -- exactly the case hold breaks. Fixed by calling gpio_hold_dis() before gpio_config() in all three buttons' init functions, not just manage's -- reset and next-photo have the same latent issue in their own live-read fallback paths, just not yet exercised the same way. --- firmware/main/manage_button.c | 12 ++++++++++++ firmware/main/next_button.c | 6 ++++++ firmware/main/reset_button.c | 6 ++++++ 3 files changed, 24 insertions(+) diff --git a/firmware/main/manage_button.c b/firmware/main/manage_button.c index e02e578..689fd3a 100644 --- a/firmware/main/manage_button.c +++ b/firmware/main/manage_button.c @@ -17,6 +17,18 @@ static const char *TAG = "manage_button"; void manage_button_init(void) { + /* Every deep sleep with this pin armed as a wakeup source leaves it + * "held" (ESP-IDF locks the pin's pull/config across the sleep + * transition -- see esp_sleep_gpio_wakeup_prepare_on_hp_periph_powerdown() + * in sleep_modes.c) and, confirmed on hardware, never un-holds it on + * wake -- the application has to. Left held, live gpio_get_level() + * reads stay frozen at whatever level the pin had the moment sleep + * began (idle/unpressed, almost always), which looks identical to + * "not pressed" and silently breaks any live poll for a NEW press + * later in the same session -- exactly what the manage menu's + * escalation needs. Must run before gpio_config() below. */ + gpio_hold_dis(MANAGE_BUTTON_GPIO); + gpio_config_t io_conf = { .pin_bit_mask = 1ULL << MANAGE_BUTTON_GPIO, .mode = GPIO_MODE_INPUT, diff --git a/firmware/main/next_button.c b/firmware/main/next_button.c index 8850e53..54746de 100644 --- a/firmware/main/next_button.c +++ b/firmware/main/next_button.c @@ -17,6 +17,12 @@ static const char *TAG = "next_button"; void next_button_init(void) { + /* See manage_button.c's manage_button_init() for why this has to run + * before gpio_config() -- a deep sleep with this pin armed as a + * wakeup source leaves it "held," and nothing un-holds it on wake + * except explicitly asking. */ + gpio_hold_dis(NEXT_BUTTON_GPIO); + gpio_config_t io_conf = { .pin_bit_mask = 1ULL << NEXT_BUTTON_GPIO, .mode = GPIO_MODE_INPUT, diff --git a/firmware/main/reset_button.c b/firmware/main/reset_button.c index a4738f7..b14cb31 100644 --- a/firmware/main/reset_button.c +++ b/firmware/main/reset_button.c @@ -18,6 +18,12 @@ static const char *TAG = "reset_button"; void reset_button_init(void) { + /* See manage_button.c's manage_button_init() for why this has to run + * before gpio_config() -- a deep sleep with this pin armed as a + * wakeup source leaves it "held," and nothing un-holds it on wake + * except explicitly asking. */ + gpio_hold_dis(RESET_BUTTON_GPIO); + gpio_config_t io_conf = { .pin_bit_mask = 1ULL << RESET_BUTTON_GPIO, .mode = GPIO_MODE_INPUT,