ESP8266 / ESP32: Place ISR in IRAM

[mcspr]

supercede #15

Update is for both ESP8266 and ESP32, by combining attribute define. ESP8266 Core will crash when used without this attribute on current git version (future 2.5.1). And it's likely to crash anyways with more complex examples / fast encoders, even without that check. ref: https://arduino-esp8266.readthedocs.io/en/latest/faq/a02-my-esp-crashes.html#other-causes-for-crashes https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram https://github.com/esp8266/Arduino/pull/5995

Things are moved around because just adding IRAM_ATTR does not work. For example, adding it to the ::update causes this issue:

$ pio platform show espressif32
Version: 2.10002.190416
Original version: 1.0.2
$ PLATFORMIO_CI_SRC=examples/Basic/Basic.pde pio ci --lib . -b lolin32
...
Linking .pioenvs/lolin32/firmware.elf
.pioenvs/lolin32/src/Basic.pde.cpp.o: In function `Encoder::update(Encoder_internal_state_t*)':
Basic.pde.cpp:(.iram1[Encoder::update(Encoder_internal_state_t*)]+0x3d): dangerous relocation: l32r: literal placed after use: .literal._ZN7Encoder6updateEP24Encoder_internal_state_t

(same for each isrN)

*This PR would also allow to add Functional Interrupt support (ESP8266&32 Cores feature) attach_interrupt could be replaced with just:

static uint8_t attach_interrupt(uint8_t pin, Encoder_internal_state_t *state) {
    attachInterrupt(pin, std::bind(&update, state), CHANGE);
    return 1;
}

[brunnwart]

YES, I hope this patch will solve the problem of esp8266 and esp32 crashing accidentially when using this nice encoder lib and I hope Paul will merge it. My mirror following the sun using gear motors and encoders was ready built and programmened but could never be used because of those esp crashes in two different ways: first type of crash leading to (just annoying) reboot of the esp and the second type of crashing to hang, in my case leading to still running gear motors, breaking through the limit switches (with no more function) and thus destroying the construction ... :-( And yes: the prefixing of ICACHE_RAM_ATTR to the interrupt routines as proposed here did not solve the problem. So I'm really looking forward to another try with this modification (after repairing my construction) ...

[PaulStoffregen]

I generally do not merge patches which claim to just make a small change like adding some attribute, but in fact modify nearly all the code and likely will affect ALL boards, not just the ESP.

Please submit a cleaner pull request.

[mcspr]

I generally do not merge patches which claim to just make a small change like adding some attribute, but in fact modify nearly all the code and likely will affect ALL boards, not just the ESP.

Note that I did exactly that. "Large" code change is merely copy paste, changing place. Am I missing some easier way to place each of 59 isrN methods + "update" inside a specific section?

39 re-does it. Only update() is moved and ESP now requires FunctionalInterrupt

[brunnwart]

Hello mcspr, I've just tried Encoder.[h,cpp] from your esp/isr-in-iram fork but (under my circumstances) still get Guru meditation errors leading to a reboot. The error message is:

Guru Meditation Error: Core 0 panic'ed (InstrFetchProhibited). Exception was unhandled. Core 0 register dump: PC : 0x00000000 PS : 0x00050933 A0 : 0x00000000 A1 : 0x3ffbe270 A2 : 0x3ffbe2b0 A3 : 0x3ffc190d A4 : 0x00000000 A5 : 0x80096dcc A6 : 0x3ffbe290 A7 : 0x00000003 A8 : 0x00060023 A9 : 0x8008172c A10 : 0x3ffbe2d0 A11 : 0x00000000 A12 : 0x3ffbbd60 A13 : 0x80096dcc A14 : 0x3ffbc010 A15 : 0x00000003 SAR : 0x00000023 EXCCAUSE: 0x00000014 EXCVADDR: 0x00000000 LBEG : 0x00000013 LEND : 0x3ffc1b98 LCOUNT : 0x800df153

ELF file SHA256: 0000000000000000000000000000000000000000000000000000000000000000

Backtrace: 0x00000000:0x3ffbe270 0x7ffffffd:0x3ffbe290 0x4008133b:0x3ffbe2b0 0x4008135f:0x3ffbe2d0 0x40081729:0x3ffbe2f0 0x40081ef5:0x3ffbe310 0x40154907:0x3ffbc070 0x400dcf12:0x3ffbc090 0x40097441:0x3ffbc0b0 0x40095541:0x3ffbc0d0

Rebooting... ets Jun 8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) configsip: 0, SPIWP:0xee clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00 mode:DIO, clock div:2 load:0x3fff0018,len:4 load:0x3fff001c,len:928 ho 0 tail 12 room 4 load:0x40078000,len:9504 load:0x40080400,len:5900 entry 0x40080698 Guru Meditation Error: Core 0 panic'ed (Cache disabled but cached memory region accessed) Guru Meditation Error: Core 0 panic'ed (IllegalInstruction). Exception was unhandled. Memory dump at 0x40154640: bad00bad bad00bad bad00bad .... Guru Meditation Error: Core 0 panic'ed (IllegalInstruction). Exception was unhandled. Memory dump at 0x40154640: bad00bad bad00bad bad00bad Guru Meditation Error: Core 0 panic'ed (Double exception) Guru Meditation Error: Core 0 panic'ed (Double exception) 0�@���?d (Unhandled debug exception)

Debu�@6 ets Jun 8 2016 00:22:57

rst:0x8 (TG1WDT_SYS_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) configsip: 0, SPIWP:0xee clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00 mode:DIO, clock div:2 load:0x3fff0018,len:4 load:0x3fff001c,len:928 ho 0 tail 12 room 4 load:0x40078000,len:9504 load:0x40080400,len:5900 entry 0x40080698

[mcspr]

@brunnwart Can you please decode the backtrace? Not sure if it is entirely related. If you are using ArduinoIDE, there is a GUI for it: https://github.com/me-no-dev/EspExceptionDecoder

[brunnwart]

@mcspr Thanks for your hint an EspExceptionDecoder. So far I used Atom/Platformio and it took me a while to find out how to get EspExceptionDecoder.jar compiled. Got that done except those commands in the EspExceptionDecoder/make.sh file (the "git" command didn't work and I didn't understand what it should do): dist=$PWD/dist rev=$(git describe --tags) mkdir -p $dist

pushd $INSTALLDIR/tools zip -r $dist/$PROJECT-$rev.zip $PROJECT/

Now the EspExceptionDecoder.jar is in the tool dir and is listed in the menu of the Arduino IDE. I just got another crash but after pasting the serial output into the EspException window the only response I get is "Decode Failed" so decoding does not seem to work :-(

This is my last error message: ERROR A stack overflow in task IDLE has been detected. abort() was called at PC 0x40094968 on core 0

Backtrace: 0x4009474c:0x3ffc04f0 0x4009494f:0x3ffc0510 0x40094968:0x3ffc0530 0x40091bb3:0x3ffc0550 0x40093584:0x3ffc0570 0x40093615:0x3ffc0590 0x400812f3:0x3ffc05b0 0x40081315:0x3ffc05d0 0x40081715:0x3ffc05f0 0x40081ee9:0x3ffc0610 0x40148673:0x00000000

Rebooting... ets Jun 8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) configsip: 0, SPIWP:0xee clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00 mode:DIO, clock div:1 load:0x3fff0018,len:4 load:0x3fff001c,len:952 load:0x40078000,len:6084 load:0x40080000,len:7936 entry 0x40080310 Guru Meditation Error: Core 0 panic'ed (Cache disabled but cached memory region accessed) Core 0 register dump: PC : 0x400810a9 PS : 0x00060034 A0 : 0x80081268 A1 : 0x3ffc05b0
A2 : 0x3ffc47b0 A3 : 0x3ffd0d20 A4 : 0x80091244 A5 : 0x3ffcf1e0
A6 : 0x00000000 A7 : 0x00000000 A8 : 0xbad00bad A9 : 0x3f400020
A10 : 0x00000010 A11 : 0x00000080 A12 : 0x80092178 A13 : 0x3ffcf1d0
A14 : 0x00000003 A15 : 0x00060a23 SAR : 0x0000001a EXCCAUSE: 0x00000007
EXCVADDR: 0x00000000 LBEG : 0x400012c5 LEND : 0x400012d5 LCOUNT : 0xfffffffd
Core 0 was running in ISR context: EPC1 : 0x4000bff0 EPC2 : 0x00000000 EPC3 : 0x00000000 EPC4 : 0x400810a9

Backtrace: 0x400810a9:0x3ffc05b0 0x40081265:0x3ffc05d0 0x40081739:0x3ffc05f0 0x40081ee9:0x3ffc0610 0x4000bfed:0x00000000

Rebooting... ets Jun 8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) configsip: 0, SPIWP:0xee clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00 mode:DIO, clock div:1 load:0x3fff0018,len:4 load:0x3fff001c,len:952 load:0x40078000,len:6084 load:0x40080000,len:7936 entry 0x40080310

[mcspr]

How do you set up the Encoder object? If based on examples here, note that ESP32 boards cannot normally use pins 6 to 11

EspExceptionDecoder releases has pre-built jar btw :) I just found a pending platformio issue about simple decoder integration and it mentions monitor tool from esp-idf: https://raw.githubusercontent.com/espressif/esp-idf/master/tools/idf_monitor.py PlatformIO's virtualenv can be used (~/.platformio/penv/Scripts/python{,.exe}) to run the monitor script. It does require to install future package` beforehand, because it is python2. For example, using Windows:

> C:\Users\Maxim\.platformio\penv\Scripts\pip2.exe install future
> C:\Users\Maxim\.platformio\penv\Scripts\python.exe idf_monitor.py --port COM15 --toolchain-prefix C:\Users\Maxim\.platformio\packages\toolchain-xtensa32\bin\xtensa-esp32-elf-  .pioenvs\lolin32\firmware.elf
...watch crash and decode result...

Or, if available, just use python3 (and install pyserial).

[brunnwart]

The encoder pins are 34, 35, 36 and 39 on my esp32. And I still do not find a precompiled EspExceptionDecoder.jar file anywhere ...

[mcspr]

-> EspExceptionDecoder-1.1.0.zip It was manually added, not generated like the "source code" archive.

[brunnwart]

@mcspr: thanks for your hint to EspExceptionDecoder-1.1.0.zip with the precompiled jar-file. I've used that instead but still get the response "Decode Failed" :-(

[mcspr]

Can you try idf_monitor.py script instead? It will behave just as normal serial monitor plus decode any backtrace it finds in real time. You just need to manually specify correct toolchain, .elf paths and hw port

[brunnwart]

Well, if I understand how to use it, I'll give it a try on Saturday :-) python 2&3, pyserial, pip and virtualenv is installed on my OpenSuSE distro

[brunnwart]

O.K., for Linux users this works for writing the output of the script idf_monitor.py into a file using Platformio, python3: python3 /_where_to_find_pythonscript/idf_monitor.py --port /dev/ttyUSB0 --toolchain-prefix $HOME/.platformio/packages/toolchain-xtensa32/bin/xtensa-esp32-elf- $HOME/Documents/PlatformIO/Projects/$PROJECT/.pioenvs/esp32dev/firmware.elf > $HOME/Documents/PlatformIO/Projects/$PROJECT/idf_monitor_serial_record My sketch seems to be a bit shy to be monitored all the time - the 2 gearmotors with reading the encoders using interrupts and your modified files Encoder.[h, cpp] run for quite a while with NO CRASH so far ... will come back with results ...

[brunnwart]

... is there a difference between using framework=arduino or framework=espidf in platformio.ini with regards to the ISR problem?

[brunnwart]

The sketch runs since yesterday with your modified files Encoder.[h, cpp] (PlatformIO, framework arduino (espidf does not compile), platform = espressif32, board esp32dev) beeing monitored by idf_monitor.py as written above on my esp32 and so far NO CRASH anymore. Will continue testing on Friday, 17th.

[mcspr]

Good to hear. Please also check out how #39 is working. I'm inclined to believe #39 is a better approach, to avoid mixing ESP8266/ESP32-specific code with original arduino api (based on Paul's comment).

[PaulStoffregen]

Does this fully solve the problem on ESP32?

https://github.com/PaulStoffregen/Encoder/commit/023f33752399e07a6a4ae3bde99456a5dcaa0288

Please confirm if this issue should be closed?

[mcspr]

Same error as above with dangerous relocation: l32r: literal placed after use: .literal._ZN7Encoder4isr#Ev for each isr method, as gcc-xtensa still goes insane from the __attribute__((section(...))) in the header

Moving the actual implementation to the Encoder.cpp and using ICACHE_RAM_ATTR there instead works though. i.e. for both isr() and update()

-       static void update(Encoder_internal_state_t *arg) {
.... etc ...
+      static void update(Encoder_internal_state_t *arg);

+ ENCODER_ISR_ATTR void Encoder::update(Encoder_internal_state_t *arg) {

 #if defined(ENCODER_USE_INTERRUPTS) && !defined(ENCODER_OPTIMIZE_INTERRUPTS)
        #ifdef CORE_INT0_PIN
-       static ENCODER_ISR_ATTR void isr0(void) { update(interruptArgs[0]); }
+       static void isr0(void);

 // Yes, all the code is in the header file, to provide the user
 // configure options with #define (before they include it), and
 // to facilitate some crafty optimizations!
+//
+#if defined(ENCODER_USE_INTERRUPTS) && !defined(ENCODER_OPTIMIZE_INTERRUPTS)
+       #ifdef CORE_INT0_PIN
+        ENCODER_ISR_ATTR void Encoder::isr0(void) { Encoder::update(Encoder::interruptArgs[0]); }

[mcspr]

@PaulStoffregen updated PR with the changes above Since section attribute can't appear in the header shared between .ino and lib .cpp, now it is only specified in the .cpp file of the library. ISR functions were never inline to begin with, so I don't think there's any change in the behaviour?

edit: although, I guess this can be removed from the .cpp

#if defined(ENCODER_USE_INTERRUPTS) && !defined(ENCODER_OPTIMIZE_INTERRUPTS)
...
#endif

since it may not be applicable to what happens in the .ino, and thus should always be here

[kescholm]

I also get dangerous relocation: for ESP32 due to ICACHE_RAM_ATTR. I'm using Platform.io board https://docs.platformio.org/en/latest/boards/espressif32/esp32thing.html My (bad) hack is to revert commit 023f337 I tried with https://github.com/mcspr/Encoder and the build succeeds without the dangerous relocation linker error. I have not tested it yet.

[PaulStoffregen]

Hopefully this resolves the issue? https://github.com/PaulStoffregen/Encoder/commit/eb73cf763504c6d0159fc2326fc9142a801e4e8a

[mcspr]

Hopefully this resolves the issue? eb73cf7

Not fully. For some reason, with recent arduino-esp32 (2.0.2) ICACHE_RAM_ATTR (what encoder header declares via #define ENCODER_ISR_ATTR ICACHE_RAM_ATTR) no longer gets translated into IRAM_ATTR, so isr#() funcs are still in flash; see .text.Encoder.*isr in the linker .map file

You can also check out it's results via something like arduino-cli compile --library ../../ --verbose -b esp32:esp32:esp32:FlashSize=2M in Basic example dir. Note that it should be the xtensa variant: esp32 or esp32s2; Since 2019 there's also esp32c3 variant based on riscv that does not seem to care and builds OK, but I'd guess it will also break at runtime. Can't test more at this time.

[Storfoten]

This pull request worked for me using a D1 mini (8266), thanks