Skip to content
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

Fixes buildability with Arduino Core 2.5.1 #24

Closed
wants to merge 1 commit into from

Conversation

suculent
Copy link

Added ICACHE_RAM_ATTR to the isrPin otherwise Arduino core complains "ISR not in IRAM" in extern void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, voidFuncPtr userFunc, void *arg, int mode) in cores/esp8266/core_esp8266_wiring_digital.cpp

Added ICACHE_RAM_ATTR to the isrPin otherwise Arduino core complains "ISR not in IRAM" in extern void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, voidFuncPtr userFunc, void *arg, int mode) in cores/esp8266/core_esp8266_wiring_digital.cpp
@@ -46,7 +46,7 @@ bool XPT2046_Touchscreen::begin()
#ifdef ESP32
void IRAM_ATTR isrPin( void )
#else
void isrPin( void )
void ICACHE_RAM_ATTR isrPin( void )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this isn't breaking non-ESP8266 devices? Wouldn't a fix like this be a tad safer: https://github.com/sandeepmistry/arduino-LoRa/pull/257/files

@marcelstoer
Copy link

Side note: this is a companion PR to #17 from @CelliesProjects - this time for ESP8266 rather than ESP32.

@suculent
Copy link
Author

suculent commented Jun 25, 2019 via email

@marcelstoer
Copy link

marcelstoer commented Jun 25, 2019

That's not what I meant. I see in the code that ESP32 is properly handled. This library, however, can also be used on non-ESP chips. The photo in the README shows a Teensy I believe.

@PaulStoffregen
Copy link
Owner

{sigh} - why do people using ESP attempt to submit fixes that break libraries for all other non-ESP boards?

@PaulStoffregen
Copy link
Owner

I've committed a fix. Please confirm if this works for you on ESP8266.

26b691b

Please do NOT submit any pull request which you have not tested at least one 1 of the official Arduino boards. If you have only tested on ESP, do not send the pull request.

@PaulStoffregen
Copy link
Owner

Did anyone try the fix?

@marcelstoer
Copy link

I just tested with ESP8266 Arduino-core 2.5.2 and that works. I don't have an official Arduino board but I guess that's what you tested.

@marcelstoer
Copy link

@PaulStoffregen can I suggest you close this PR and instead release your fix as 1.3.1?

@marcelstoer
Copy link

@PaulStoffregen could please give us a rough estimate by when you plan to lease that fix?

@PaulStoffregen
Copy link
Owner

As far as I know, this issue is fixed by 26b691b

@marcelstoer
Copy link

marcelstoer commented Jul 24, 2019

Correct, thanks for that. I was wondering when we could expect a new release in the Arduino/PlatformIO library manager.

@txoof
Copy link

txoof commented Aug 11, 2019

I would like to add a +1 to @marcelstoer's request. I just encountered some problems when I upgraded to the latest revision of this board and could not build some ESP code.

Would you please consider releasing this to the Arduino library manager?

@hridder
Copy link

hridder commented Sep 16, 2019

@PaulStoffregen Can we please get a new release to the Library Manager? Using the TouchTestIRQ example on a Wemos D1 mini Pro ESP8266, I have just now verified that 26b691b fixes the the "ISR not in IRAM!" crash. @marcelstoer had also verified it back on Jun 25. Thank you very much for the fix. Is there anything further that needs testing or is blocking?

Yes, I know this PR (which will probably be closed unmerged) isn't the place to discuss 26b691b and I'm happy to open an issue if that's better, but all the context is here.

@marcelstoer
Copy link

I'm happy to open an issue if that's better, but all the context is here.

I did that a while back in #25 but I closed it when I found this PR.

@marcelstoer
Copy link

This can now be closed as it was fixed with 26b691b and released with https://github.com/PaulStoffregen/XPT2046_Touchscreen/releases/tag/v1.4.

@hridder
Copy link

hridder commented Aug 30, 2021

Thank you @PaulStoffregen for releasing a new version!

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

Successfully merging this pull request may close these issues.

5 participants