-
Notifications
You must be signed in to change notification settings - Fork 669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: immediate resume after shutdown #3875
Comments
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. |
This issue has been mentioned on Meshtastic. There might be relevant details there: https://meshtastic.discourse.group/t/dodeepsleep-with-msectowake/12338/13 |
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! |
@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. I picked up project environment "env:tlora-v2-1-1_6" and Line 235 to 238 of sleep.cpp are grayed out. Try comment out line 267 and 268 solve the issue for TLORA_2_1_1P6
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
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 #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. |
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. |
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 |
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. |
There may be models whose push button is connected to GND. |
Thank you very much for confirming that!
I had a slight suspicion that maybe there was something odd happening when calling
Specifically the ones without external pull-ups, too. Just to absolutely confirm: replacing like 267 of sleep.cpp with |
replacing like 267 of sleep.cpp with pinMode(BUTTON_PIN, INPUT_PULLUP); resolves the issue for me; no extra changes required
|
Category
Hardware Compatibility
Hardware
T-Lora v2 1.6
Firmware Version
2.3.9.f06c56a
Description
Device: T-LoRa V2.1-1.6
Firmware: 2.3.9.f06c56a Alpha
Error: immediate resume after shutdown
Expected: shutdown forever (until reset button pressed)
The device does not have a push button, only a reset button and power switch for the battery. The shutdown worked up to version 2.3.4. This issue also discussed at https://meshtastic.discourse.group/t/dodeepsleep-with-msectowake/12338/12
This error also affects SDS (super deep sleep) and the protection of the battery against deep discharge.
Relevant log output
The text was updated successfully, but these errors were encountered: