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.