Skip to content

Add ICACHE_RAM_ATTR to __pinMode in core_esp8266_wiring_digital.c #2680

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

Closed
rolpal opened this issue Nov 11, 2016 · 2 comments
Closed

Add ICACHE_RAM_ATTR to __pinMode in core_esp8266_wiring_digital.c #2680

rolpal opened this issue Nov 11, 2016 · 2 comments

Comments

@rolpal
Copy link

rolpal commented Nov 11, 2016

Basic Infos

It is known that random resets are caused by code in Interrupt service routine access flash program memory and most functions now have ICACHE_RAM_ATTR to prevent this but In file core_esp8266_wiring_digital.c the following function do not
extern void __pinMode(uint8_t pin, uint8_t mode)

Hardware

Hardware: ESP-01
Core Version: 2.3.0

Description

I need to alter pin mode while serving Interrupts since the few I/O pins available on the ESP-01 force me to use pins for dual purposes. Doing this require the use of pinMode() function that currently is not ISR safe (no ICACHE_RAM_ATTR).

Could the ICACHE_RAM_ATTR please be added to the pinMode function as well?
My current solution of modifying the library is obviously not preferred...

Settings in IDE

Not really in scope of problem but as below:

Module: Generic ESP8266 Module
Flash Size: 512K
CPU Frequency: 80Mhz
Flash Mode: DIO
Flash Frequency: 40Mhz
Upload Using: SERIAL
Reset Method: ck

Sketch

Only a snip of the ISR

void ICACHE_RAM_ATTR serve_interrupt() {
  noInterrupts();                              // disable further interrupts
  digitalWrite(_ok_out_pin, HIGH);    // indicate interrupt is served
  mill = millis();                         // get current value for millis()
  if ((mill - last_millis) > 2ul) {     // data interleave (bit interleave is only 1ms)
    number_of_bits = 0;              // new data set, reset bit counter
  }
  last_millis = mill;                     // time of last received bit (normally 1ms each)
  
  if (digitalRead(_ck_in_pin)){         // led slot
    digitalWrite(_out_pin, HIGH);     // free the bus for led signal from central
  } else {                                      // key slot
    if (key_out & 0x0001) {
      digitalWrite(_out_pin, LOW);    // send key bit if any
    } else {
      digitalWrite(_out_pin, HIGH);   // free the bus if no key bit to send
    }
    key_out = key_out >> 1;          // prepare for next key bit
  }

  pinMode(_alarm_in_pin,INPUT);      // Changed to be used for sensing alarm state

_ok_out_pin is the same as _alarm_in_pin

Debug Messages

N/A

@devyte
Copy link
Collaborator

devyte commented Sep 9, 2017

@igrr what are your thought of this in view of #3579?

@igrr
Copy link
Member

igrr commented Sep 12, 2017

Yes, #3579 has to be implemented as a fix.

@rolpal if the issue is still relevant to you, then i suggest modifying the mode register of the GPIO directly in your ISR. Take a look at __pinMode implementation for details. This way you will get more free space in IRAM. I will close this issue, if you want to follow the progress, please follow the link to #3579 and click "Subscribe" button there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants