GPIOs get re-init during software restarts

[everslick]

I suggest to remove the lines:

  for (int i = 0; i <= 5; ++i) {
    pinMode(i, INPUT);
  }
  // pins 6-11 are used for the SPI flash interface
  for (int i = 12; i <= 16; ++i) {
    pinMode(i, INPUT);
  }

in:

Arduino/cores/esp8266/core_esp8266_wiring_digital.cpp

Line 238 in 9e9515b

extern void initPins() {

or maybe check the reset reason and only init the pins if resetInfo.reason == REASON_DEFAULT_RST ?

opinions?

[tbdltee]

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.
Or do you mean other kinds of restart???

[mhightower83]

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 );
  }
  ...
}

[devyte]

@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.

[everslick]

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 wrap. That works even without a weak function declaration. But it has the drawback of having to (re-)implement the whole initPins() function. And it only works if the user has tight control over the build process, like using make or such. If resetPins() lives in its own function, the code looks just nicer.

My own init function just skips over the GPIO that should not be initialized, I put it here for reference:

void __wrap_initPins() {
  //Disable UART interrupts
  system_set_os_print(0);
  U0IE = 0;
  U1IE = 0;

  for (int i = 0; i <= 5; ++i) {
    pinMode(i, INPUT);
  }
  // pins 6-11 are used for the SPI flash interface
  for (int i = 12; i <= 16; ++i) {
#if defined(HWPIN_RELAIS) && (HWPIN_RELAIS != HWPIN_UNDEFINED)
    if (i == HWPIN_RELAIS) continue;
#endif

    pinMode(i, INPUT);
  }
}

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.

[everslick]

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 void resetPins(); in wiring_private.h, I think that's the only reason the CI build fails.

I think the patch is easy enough to be merged as-is - if approved.

[dok-net]

@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.

[everslick]

@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?

[everslick]

@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?

[mhightower83]

Using 16 for detection that initPins() has completed is not the way I'd do it, rather something more explicit...

@dok-net What is your idea for determining that initPins() has finished? I was never excited about using the pin 16 method. Nothing else jumped out as being better at the time. It also has to be changed to pin 10 if you have an ESP8285.

[everslick]

Sorry for putting this quite bluntly, but pinMode() is just the wrong place for dealing with this.

• We need tricks to know pinMode() was called from initPins().
• We depend on the core to not change the internals of initPins(). (e.g. one day initPins() might not only change the pinMode(), but also set it HIGH or LOW).
• It is non-obvious that a user has to re-implement (or wrap) pinMode() instead of a function that is called resetPins().
• Its more code.

IMHO, all is needed is a way to disable the core to mess with the GPIOs. In my case even a #ifdef NO_GPIO_INIT would do the trick. Or a global variable that guards GPIO initialization, that can be set in preinit(). (I think initPins() is called before preinit(), though. This would need to be changed.) Non of those solutions (weak bound resetPins, ifdef or guard variable) would break the API.

[dok-net]

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 INPUT in each setup() call, doesn't seem to matter, you're losing IRQs, if any, just the same, I guess. Deep sleep on the ESP8266 doesn't maintain GPIO level, and works by pulling RST, same as pressing the reset button, and can't be told apart from other hard resets like power on, right?
@everslick Then, what exactly is the effective change you need, I'm confused.

[dok-net]

@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();
}

[dok-net]

@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)?

[everslick]

@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.

[dok-net]

@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.

[dok-net]

@everslick Anyway, does the code I shared above work for you?

[Tech-TX]

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 INPUT in each setup() call, doesn't seem to matter, you're losing IRQs, if any, just the same, I guess. Deep sleep on the ESP8266 doesn't maintain GPIO level, and works by pulling RST, same as pressing the reset button, and can't be told apart from other hard resets like power on, right?
@everslick Then, what exactly is the effective change you need, I'm confused.

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().

[everslick]

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 initPins().

[dok-net]

@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.

[dok-net]

@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.

[dok-net]

@mhightower83 you never approved my suggested code or otherwise. About the ESP8285, using isFlashInterfacePin comes to mind, would anyone like to improve the checks?

[mhightower83]

@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 ESP.getResetInfoPtr()->reason, I think it may be too early to start using C++ classes. The C++ initialization has not run at the time we need to be running. These class initialization, look like they are in the init_done() call back at the end of user_init(). The pin initializations occur earlier through calls to init() and initVaraiant().

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.

[dok-net]

@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 isFlashInterfacePin for the gap.

[dok-net]

@everslick Would you like to open another PR, in which you replace

  for (int i = 0; i <= 5; ++i) {
    pinMode(i, INPUT);
  }
  // pins 6-11 are used for the SPI flash interface
  for (int i = 12; i <= 16; ++i) {
    pinMode(i, INPUT);
  }

by something like

  for (int i = 0; i <= 16; ++i) {
    if (!isFlashInterfacePin(i))
        pinMode(i, INPUT);
  }

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?

[d-a-v]

@dok-net right. Please proceed with the PR unless you prefer someone else doing it

[bwjohns4]

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:

void resetPins() {
   for (int i = 0; i <= 16; ++i) {
     if (!isFlashInterfacePin(i) && (i != RELAY_Pin))
         pinMode(i, INPUT);
   }
}

Is there any reason this should have stopped working? Do I need to update something?

[d-a-v]

Try with
extern "C" void resetPins() { ...

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.

[bwjohns4]

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 extern "C" solved the problem! Thanks you!