PaulStoffregen / XPT2046_Touchscreen

Touchscreen Arduino Library for XPT2046 Touch Controller Chip
249 stars 89 forks source link

Fixes buildability with Arduino Core 2.5.1 #24

Closed suculent closed 3 years ago

suculent commented 5 years ago

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

marcelstoer commented 5 years ago

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

suculent commented 5 years ago

I am pretty sure. I am testing / using the same lib on both ESP8266/32

Odesláno z iPhonu

    1. 2019 v 22:16, Marcel Stör notifications@github.com:

@marcelstoer commented on this pull request.

In XPT2046_Touchscreen.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 ) 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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

marcelstoer commented 5 years ago

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 commented 5 years ago

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

PaulStoffregen commented 5 years ago

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

https://github.com/PaulStoffregen/XPT2046_Touchscreen/commit/26b691b2c84ad93884e262088d3509bc17241278

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 commented 5 years ago

Did anyone try the fix?

marcelstoer commented 5 years ago

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 commented 5 years ago

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

marcelstoer commented 5 years ago

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

PaulStoffregen commented 5 years ago

As far as I know, this issue is fixed by https://github.com/PaulStoffregen/XPT2046_Touchscreen/commit/26b691b2c84ad93884e262088d3509bc17241278

marcelstoer commented 5 years ago

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

txoof commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 3 years ago

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 commented 3 years ago

Thank you @PaulStoffregen for releasing a new version!