[Bug]: immediate resume after shutdown about firmware HOT 11 OPEN

I believe it is already acceptable to simply omit the PIN_BUTTON definition when defining a variant without a button. I'm not actually sure what the original reason for adding a button definition for this model was, and I'd hate to break someone else's device by removing it. My concern is that some T-Lora users may have DIY builds which do use a button connected to GPIO12.

The best practise to tell the firmware the board does not habe a user button is to not define that macro. This is one of the oldest parts of the code and comes from the idea, to have fixed pins assigned for optional peripherals. we have since removed the fixed GPS pins from those devices, and in theory the user button can be removed and replaced by the protobuf setting. We just need to put this into release nodes. Fixed definitions shold only exist if a button is physically present on the shipped devices.

from firmware.

Yes, I tested commenting out line 267 and 268, and it resolved the issue.

I repeated the test with pinMode INPUT_PULLUP, and it also resolved the issue.
I don't change the main-esp32.cpp and think gpio_pullup_en alone has no impact,
because pinMode is a Arduino wrapper around gpio_set_direction and gpio_set_pull_mode.
https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/gpio.html

from firmware.

replacing like 267 of sleep.cpp with pinMode(BUTTON_PIN, INPUT_PULLUP); resolves the issue for me; no extra changes required

#ifdef BUTTON_PIN
    // Avoid leakage through button pin
    if (GPIO_IS_VALID_OUTPUT_GPIO(BUTTON_PIN)) {
        pinMode(BUTTON_PIN, INPUT_PULLUP); // bugfix #3875 pinMode(BUTTON_PIN, INPUT);
        gpio_hold_en((gpio_num_t)BUTTON_PIN);
    }
#endif

from firmware.

I remember speaking about this on the discourse server, was hoping to hear back from you! I don't have the hardware myself, but I've reached out to hopefully do some testing with someone who does. Still suspecting it's either due to the defined PIN_BUTTON (GPIO12) for this variant requesting an internal pull-up, or maybe due to its role as a bootstrapping pin (fails if pulled HIGH at boot). If all else fails, there may be a case to disable the PIN_BUTTON wake interrupt for this variant, and rely on the reset instead.

from firmware.

This issue has been mentioned on Meshtastic. There might be relevant details there:

https://meshtastic.discourse.group/t/dodeepsleep-with-msectowake/12338/13

from firmware.

The shutdown worked up to version 2.3.4

If you're wondering what the change was, the EXT0 interrupt was reassigned to listen for interrupts from the radio during sleep, which meant the button interrupt was shifted to EXT1. I think we've caught most of the edge cases at this point, but seems there's still one or two out there!

from firmware.

@todd-herbert sorry for the late response. I didn't have access to a computer with VS Code this past week to try out your hint. Today I tested the shutdown on different hardware variants on firmware: 2.3.9.f06c56a.

The T-Lora 2.1-1.6 device don't have a push button, only one reset button and power switch.
https://github.com/LilyGO/TTGO-LORA32/blob/master/schematic1in6.pdf

I picked up project environment "env:tlora-v2-1-1_6" and Line 235 to 238 of sleep.cpp are grayed out.
Line 267 is enabled, meaning BUTTON_PIN is defined in variant.h (BUTTON_PIN 35).

Try comment out line 267 and 268 solve the issue for TLORA_2_1_1P6

#ifdef BUTTON_PIN
    // Avoid leakage through button pin
    if (GPIO_IS_VALID_OUTPUT_GPIO(BUTTON_PIN)) {
        // pinMode(BUTTON_PIN, INPUT);
        // gpio_hold_en((gpio_num_t)BUTTON_PIN);
    }
#endif

Perhaps a precompiler variable HAS_NO_BUTTON in variant.h would help to exclude all devices without a push button from such operations in future. There may also be (repeater) devices without PUSH_BUTTON in the future.

from firmware.

The T-Lora 2.1-1.6 device don't have a push button, only one reset button and power switch.

Perhaps a precompiler variable HAS_NO_BUTTON in variant.h would help to exclude all devices without a push button from such operations in future. There may also be (repeater) devices without PUSH_BUTTON in the future.

I believe it is already acceptable to simply omit the PIN_BUTTON definition when defining a variant without a button. I'm not actually sure what the original reason for adding a button definition for this model was, and I'd hate to break someone else's device by removing it. My concern is that some T-Lora users may have DIY builds which do use a button connected to GPIO12.

Try comment out line 267 and 268 solve the issue for TLORA_2_1_1P6

Just to make sure I understand correctly: you tested commenting out line 267 and 268, and it resolved the issue? If so, that's very helpful information.

If commenting out 267 and 268 worked for you, could I possibly bother you for a tiny bit more testing? I would be interested to know if it is specifically line 267 or 268 which is the issue. My hope is that changing line 267 to INPUT_PULLUP will resolve the issue both on your device (without a button) and on any other user's builds (which may have a button connected to GPIO12?)

#ifdef BUTTON_PIN
    // Avoid leakage through button pin
    if (GPIO_IS_VALID_OUTPUT_GPIO(BUTTON_PIN)) {
        pinMode(BUTTON_PIN, INPUT_PULLUP);
        gpio_hold_en((gpio_num_t)BUTTON_PIN);
    }
#endif

I'm also suspicious that lines 212-214 of src/platform/esp32/main-esp32.cpp may need to be removed..

I'm sorry to ask this of you. Ideally I wouldn't bother you, except that I'm not able to test on my own.

from firmware.

Makes sense to me. I would still quite like to figure out what changes would resolve this particular situation though if possible, because I notice there are a few other variants which also have BUTTON_NEED_PULLUP, and I'm suspicious that they might be affected by the same bug right now.

from firmware.

there are a few other variants which also have BUTTON_NEED_PULLUP

There may be models whose push button is connected to GND.

from firmware.

Thank you very much for confirming that!

I don't change the main-esp32.cpp and think gpio_pullup_en alone has no impact,
because pinMode is a Arduino wrapper around gpio_set_direction and gpio_set_pull_mode.

I had a slight suspicion that maybe there was something odd happening when calling gpio_pullup_en after gpio_hold_en, but from the results of your testing, it seems that it's no problem!

There may be models whose push button is connected to GND.

Specifically the ones without external pull-ups, too.

Just to absolutely confirm: replacing like 267 of sleep.cpp with pinMode(BUTTON_PIN, INPUT_PULLUP); resolves the issue for you; no extra changes required? I ask because I'll make that change for any devices with BUTTON_NEED_PULLUP (hopefully saving them from this same bug), unless of course @caveman99 is happy to make the changes? I can also go through and purge any PIN_BUTTON definitions for devices without an onboard user button, if that helps tidy things up.

from firmware.