-
Notifications
You must be signed in to change notification settings - Fork 13.3k
GPIOs get re-init during software restarts #7041
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
Comments
I think it's correct behaviour. ESP.restart() tells SDK to reboot, all register has been cleared. thus, GPIO pin should be reset to make everything stable. |
IMO that would be what they call a breaking change. The current core gives the GPIO ports a consistent starting point. I think it is good to have a known starting state after any reset. That aside, you can accomplish what you want, by supplying your own pinMode() function. The one in the core is defined as a weak link to __pinMode(). From your own pinMode function you can monitor calls to change pins, and protect the pins that need protecting. I did this as a proof of concept to one of the smart plug open-source projects. However, I never made it through my due diligence in verifying all the pins it worked for and under which reset cause. I never did a PR. For the active pins on my devices it works. Here is a code snippet to illustrate what I am talking about: extern struct rst_info resetInfo;
extern "C" void __pinMode( uint8_t pin, uint8_t mode );
// exempt pins are pins that hold their state/status across soft reboots.
// unverified list:
//uint32_t const exempt_pin_mask = BIT(0) | BIT(3) | BIT(4) | BIT(5) |
// BIT(12) | BIT(13) | BIT(14) | BIT(15);
uint32_t const exempt_pin_mask = BIT(LED_BUILTIN) | BIT(RELAY_PIN);
inline bool is_gpio_pin_exempt(int pin) {
return (exempt_pin_mask & BIT(pin)) ? true : false;
}
inline bool is_gpio_persistent(void) {
return REASON_EXCEPTION_RST <= resetInfo.reason &&
REASON_SOFT_RESTART >= resetInfo.reason;
}
extern "C" void pinMode( uint8_t pin, uint8_t mode ) {
static bool in_initPins = true;
if ( in_initPins && is_gpio_persistent() ) {
if ( 16 == pin )
in_initPins = false;
if ( is_gpio_pin_exempt(pin) )
return;
}
__pinMode( pin, mode );
}
void setup() {
// Filter on reset reasons that require GPIO pins
// to be reprogrammed.
if ( !is_gpio_persistent() ) {
pinMode( LED_BUILTIN, OUTPUT );
pinMode( RELAY_PIN, OUTPUT );
}
...
} |
@everslick can you please test the solution in the previous comment for all usable gpio pins? I suggest putting it in a small standalone example sketch. Please report back here if there is any gpio that does not hold state with @mhightower83 's proposed solution during a soft reboot. |
I propose a simple solution (#7044) that lets the user implement any kind of GPIO initialization he wants. I have Implemented it on top of the current (unmodified) core, by using the linker option My own init function just skips over the GPIO that should not be initialized, I put it here for reference:
I personally do not need the changes to the core, but I could get rid of the code duplication in my wrapper, that would make it cleaner. But I think it could be useful for other people who do not have access to the linker trick. |
I tried to create a PR using the Github webinterface, but obviously i can only ever change one file for one PR. Thus the PR #7044 misses the declaration I think the patch is easy enough to be merged as-is - if approved. |
@everslick As the owner of a PR that would have a merge conflict with yours, let me, too, ask the question, why don't you like expressing your change to the initialization procedure in terms of what @mhightower83 has outlined here? Using 16 for detection that initPins() has completed is not the way I'd do it, rather something more explicit, but otherwise, there is really no need to modify an essential system initialization procedure by weak binding yet another function. |
@dok-net I guess its personal taste then? mhightower83's solution strikes me as rather complicated. whats your issue with weak binding another function? any technical reason? |
@dok-net I tried to find the PR you mentioned to be in potential conflict, but couldn't. Can you point me into the right direction, pls? |
@dok-net What is your idea for determining that |
Sorry for putting this quite bluntly, but
IMHO, all is needed is a way to disable the core to mess with the GPIOs. In my case even a |
Um... what's the point of the exercise exactly? What I see is this: a soft reboot does not affect the state of GPIOs set to OUTPUT, i.e. a relay does not switch. Having to set any GPIO back to |
@mhightower83 Here's the code anyway. Cost for turning on PERSIST_GPIOS: 4 bytes DATA, 160 bytes IROM. #include <user_interface.h>
#include <c_types.h>
#define PERSIST_GPIOS
// for this example:
constexpr auto RELAY_PIN = 5;
#ifdef PERSIST_GPIOS
extern "C" void __pinMode(uint8_t pin, uint8_t mode);
// reset exempt gpios hold their state/status across soft reboots.
// unverified list:
uint32_t constexpr reset_exempt_gpio_mask = BIT(0) | BIT(3) | BIT(4) | BIT(5) |
BIT(12) | BIT(13) | BIT(14) | BIT(15);
bool is_reset_gpio_persistent() {
return REASON_EXCEPTION_RST <= ESP.getResetInfoPtr()->reason &&
REASON_SOFT_RESTART >= ESP.getResetInfoPtr()->reason;
}
uint32_t gpio_persist_map = reset_exempt_gpio_mask & (BIT(RELAY_PIN));
extern "C" void pinMode(uint8_t pin, uint8_t mode) {
if (gpio_persist_map) {
if (gpio_persist_map & BIT(pin)) {
if (!is_reset_gpio_persistent()) {
gpio_persist_map = 0UL;
// fall through
}
else {
gpio_persist_map ^= BIT(pin);
return;
}
}
}
__pinMode(pin, mode);
}
#endif
void setup() {
// Filter on reset reasons that require GPIO pins to be reprogrammed.
#ifdef PERSIST_GPIOS
if (!is_reset_gpio_persistent())
#endif
{
pinMode(RELAY_PIN, OUTPUT);
digitalWrite(RELAY_PIN, HIGH);
}
pinMode(LED_BUILTIN, OUTPUT);
digitalWrite(LED_BUILTIN, false);
// ...
}
const auto start = millis();
void loop() {
if (millis() - start > 2000)
ESP.reset();
} |
@devyte Do you think it's by design or by mistake that the initialization after soft-reboot does NOT put all GPIOs to OUTPUT AND a defined state (either low or high)? |
@dok-net The point is a connected relay does indeed switch off, when the 'offending' code is active. I shared your confusion the first time I inspected the code, because I too thought setting the pins to INPUT should not change the pin level, but it does. |
@everslick Ah, it's a newer D1 mini pro that doesn't switch on soft reboot, the older D1 mini (with the pre-assembly ESP8266 module that has the metal cap) switches off on soft reboot and needs the persistence enhancement. |
@everslick Anyway, does the code I shared above work for you? |
Except for the pins that are involved in boot, a Deep Sleep reset doesn't reset the pins. That surprised me when I first noticed it, as every other micro I'd used DID reset the GPIOs. I can understand why Espressif did it, and it's sort of nice if you need it. Yes, the SDK does know the difference between Deep Sleep reset and a normal external reset, so the result of the two is slightly different. Deep Sleep can't distinguish between a reset that it caused with the RTC versus external reset when it's asleep, and both show "Deep-Sleep Wake" for the reset cause. BTW, I use ESP.restart() precisely because I want everything reset as closely as possible to a default power-up state. If I didn't, I'd use ESP.reset() which leaves a number of things untouched. You have my vote for not changing the behavior of ESP.restart(). |
That will not help a bit to stop the Arduino Core from resetting your GPIOs. The irony is that I have never seen code that would assume the Core to set the GPIOs to certain states. Everybody does it in the sketch setup anyway. Whatever, I'm not asking to change the default behavior, I'm asking for a simple way to be able to disable |
@Tech-TX My observation was different, depending on the various ESP 8266 versions I own. Some reset, some don't. The only safe assumption for portable software is to initialize on startup by default. As to the reported reset reason, I don't see any value, a reset via RST line has the same effect, regardless the reported reason, right? The difference depends only on the MCU's revision, then.
|
@everslick The sample code I gave could end up in a shipping example sketch, it's really easy in comparison to the addressed functionality and does everything without yet another core mod. Is there anything in detail that you want chanced? Let us know.
|
@mhightower83 you never approved my suggested code or otherwise. About the ESP8285, using isFlashInterfacePin comes to mind, would anyone like to improve the checks?
|
@dok-net My apologies I paused to research something, got distracted, and completely forgot. I really like the approach of XOR for clearing the one-time processing of a pin. That handles my concerns for the ESP8285 nicely. For uint32_t gpio_persist_map = reset_exempt_gpio_mask & (BIT(RELAY_PIN)); I pause for a moment on this one. Here my hesitation is from my lake of fully understanding all the places that the "C" startup initialization happens at. A simple character string constant appears to get initialized by simply loading the code. Other more complex initializations like a structure init occur later. I have had trouble with using structures too soon. I think this one is OK; however, I would feel more comfortable after verifying. I don't want to say much more about this because I don't trust my recollections. |
* Pull GPIO initialization into its own 'weak' function. By pulling GPIO init into its own weak function, it can be overridden by the user. This is important in cases when GPIOs should not toggle during reboot, exceptions or other crashes. Fixes #7041. * Add prototype for resetPins()
@d-a-v you're merged before the ESP8285 specifics got addressed. Sure, it's the minimum change in the patch now, but it may promote copy&paste use of the weak function and that is not quite correct for the ESP8285 module, or is it? Still thinking |
@everslick Would you like to open another PR, in which you replace
by something like
This is for those out there using the ESP8285 module. I suspect, that usually someone exploiting the new weak binding will copy and paste the existing pinMode initialization loop, so it's better to have a portable template, do you agree? |
@dok-net right. Please proceed with the PR unless you prefer someone else doing it |
I had this working before, but I've now realized that my relay pin once again toggles across a ESP.restart(). I redefine the weak function like this:
Is there any reason this should have stopped working? Do I need to update something? |
Try with Also try to set a flag in your function and check for it, in order to check if no additional delays were added before this function is called. |
I've found the problem. I'm using PlatformIO and the linker isn't linking that function. When I make the change directly in the Arduino source file it works fine. Adding |
I suggest to remove the lines:
in:
Arduino/cores/esp8266/core_esp8266_wiring_digital.cpp
Line 238 in 9e9515b
or maybe check the reset reason and only init the pins if
resetInfo.reason == REASON_DEFAULT_RST
?opinions?
The text was updated successfully, but these errors were encountered: