Esp32 Core version 3 #2144
Esp32 Core version 3 #2144
Conversation
|
Feature: Make IRremoteESP8266 compatible with IDF 5.x ESP32 Version 3 |
|
@NiKiZe code is running on ESP32 S3 With ESP32 Version 3.0.4 without any errors I will try to test with ESP32-DEV & ESP32-C6 this weak |
|
As I understand it, so is #2040, we are grateful for your PR. Just want to limit duplication if there already is a fully working implementation. |
|
#2040 is working perfectly fine. |
|
@Jason2866 - #2040 isn't working on the M5Stack NanoC6 under Arduino IDE. Have commented on the relevant PR I'm able to decode my remote using this pull request |
|
@Jason2866 Tested with ESP32-C6 WROOM1, ESP32 WROVER and the code is working on Esp32 Core version 3. |
|
@NiKiZe @Jason2866 Thank you for feedback did the required changes |
|
I would suggest this: https://github.com/crankyoldgit/IRremoteESP8266/compare/master...NiKiZe:IRremoteESP8266:ForPr2144?expand=1 Or to see my suggestions on their own: 4ef02e5 I would like to improve on the |
|
I have updated and pushed "reverts" of lines that don't need change. (mostly indentation and comments) @crankyoldgit if you have the time I would especially like your input of how to deal with the comments on |
NiKiZe
requested review from
crankyoldgit
and removed request for
Jason2866
src/IRrecv.cpp
Outdated
| #if ( defined(ESP_ARDUINO_VERSION) && \ | ||
| (ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(3, 0, 0)) ) | ||
| #define _ESP32_ARDUINOV3 | ||
| #endif // Version > 3 |
There was a problem hiding this comment.
The cruelest of nitpicks (so feel free to ignore :-)
_ESP32_ARDUINOV3 is a poor/technically incorrect name. It's really a "V3 and/or higher" definition looking at the code.
Perhaps flip the logic and use/create _ESP32_ARDUINO_PRE_V3 (obviously changing the use of it elsewhere in the code.
Or keep it as is, and simply rename to _ESP32_ARDUINO_POST_V3_0_0 ??
Either one is fine IMHO.
There was a problem hiding this comment.
We have _ESP32_IRRECV_TIMER_HACK
It would probably also be wise to consider V4 etc. as well.
So yes _ESP32_ARDUINO_CORE_V3PLUS and rename _ESP32_IRRECV_TIMER_HACK to _ESP32_ARDUINO_CORE_V2PLUS ?
But for now also consider rename being done if/when moving to separate file for this logic
src/IRrecv.cpp
Outdated
| #elif defined(_ESP32_ARDUINOV3) | ||
| timerWrite(timer, 0); // Reset the timer | ||
| timerDetachInterrupt(timer); | ||
| timerEnd(timer); | ||
| #elif defined(ESP32) | ||
| timerAlarmDisable(timer); | ||
| timerDetachInterrupt(timer); | ||
| timerEnd(timer); | ||
| #endif // ESP32 |
There was a problem hiding this comment.
Suggestion: Try to avoid #elsif where possible
#endif // ESP8266
#if defined(ESP32)
// Check for ESP32 core version and handle timer functions differently
#if defined(_ESP32_ARDUINOV3)
// Code for Arduino cores V3 and beyond
timerWrite(timer, 0); // Reset the timer
timerDetachInterrupt(timer);
timerEnd(timer);
#else // _ESP32_ARDUINOV3
// Code for Arduino cores before V3
timerAlarmDisable(timer);
timerDetachInterrupt(timer);
timerEnd(timer);
#endif // _ESP32_ARDUINOV3
#endif // ESP32There was a problem hiding this comment.
Any particular reason to why? it is becoming messy quite fast, and it is used elsewhere already.
I am however considering that we should split out this logic to a separate file or something to reduce the mess a bit.
src/IRrecv.cpp
Outdated
| timerWrite(timer, 0); // Reset the timer (no need for timerAlarmDisable) | ||
| #else // _ESP32_ARDUINOV3 | ||
| timerAlarmDisable(timer); | ||
| #endif |
src/IRrecv.cpp
Outdated
| #if ( defined(ESP_ARDUINO_VERSION) && \ | ||
| (ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(3, 0, 0)) ) | ||
| #define _ESP32_ARDUINOV3 | ||
| #endif // Version > 3 |
There was a problem hiding this comment.
#endif // ESP_ARDUINO_VERSION >= 3
Not > 3
NiKiZe
force-pushed
the
Espressif-version-3
branch
from
e23c19e
to
f858d28
Compare
Fixes #2039