-
Notifications
You must be signed in to change notification settings - Fork 245
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
ESP8266 / ESP32: Place ISR in IRAM #38
base: master
Are you sure you want to change the base?
Conversation
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. |
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. |
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 |
Hello mcspr, Guru Meditation Error: Core 0 panic'ed (InstrFetchProhibited). Exception was unhandled. ELF file SHA256: 0000000000000000000000000000000000000000000000000000000000000000 Backtrace: 0x00000000:0x3ffbe270 0x7ffffffd:0x3ffbe290 0x4008133b:0x3ffbe2b0 0x4008135f:0x3ffbe2d0 0x40081729:0x3ffbe2f0 0x40081ef5:0x3ffbe310 0x40154907:0x3ffbc070 Rebooting... rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) Debu�@6 rst:0x8 (TG1WDT_SYS_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) |
@brunnwart Can you please decode the backtrace? Not sure if it is entirely related. |
@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): pushd $INSTALLDIR/tools 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 This is my last error message: Backtrace: 0x4009474c:0x3ffc04f0 0x4009494f:0x3ffc0510 0x40094968:0x3ffc0530 0x40091bb3:0x3ffc0550 0x40093584:0x3ffc0570 0x40093615:0x3ffc0590 0x400812f3:0x3ffc05b0 0x40081315:0x3ffc05d0 0x40081715:0x3ffc05f0 0x40081ee9:0x3ffc0610 0x40148673:0x00000000 Rebooting... rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) Backtrace: 0x400810a9:0x3ffc05b0 0x40081265:0x3ffc05d0 0x40081739:0x3ffc05f0 0x40081ee9:0x3ffc0610 0x4000bfed:0x00000000 Rebooting... rst:0x3 (SW_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) |
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 :)
Or, if available, just use python3 (and install pyserial). |
The encoder pins are 34, 35, 36 and 39 on my esp32. |
-> EspExceptionDecoder-1.1.0.zip |
@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" :-( |
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 |
Well, if I understand how to use it, I'll give it a try on Saturday :-) |
O.K., for Linux users this works for writing the output of the script idf_monitor.py into a file using Platformio, python3: |
... is there a difference between using framework=arduino or framework=espidf in platformio.ini with regards to the ISR problem? |
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. |
Does this fully solve the problem on ESP32? Please confirm if this issue should be closed? |
Same error as above with Moving the actual implementation to the Encoder.cpp and using ICACHE_RAM_ATTR there instead works though. - 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]); } |
@PaulStoffregen updated PR with the changes above 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 |
I also get |
Hopefully this resolves the issue? |
Not fully. For some reason, with recent arduino-esp32 (2.0.2) You can also check out it's results via something like |
This pull request worked for me using a D1 mini (8266), thanks |
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
esp8266/Arduino#5995
Things are moved around because just adding
IRAM_ATTR
does not work. For example, adding it to the ::update causes this issue:(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: