-
Notifications
You must be signed in to change notification settings - Fork 844
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
Esp32 Core version 3 #2144
base: master
Are you sure you want to change the base?
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 |
e23c19e
to
f858d28
Compare
Hi @NiKiZe @crankyoldgit @Jason2866, |
@NiKiZe @crankyoldgit I made changes as you request not if failing on code Lint but not sure if it because of me. |
This works for me on wemos d1 mini32. |
Are there any news about the merge of this PR? |
Those of us needing current support (including a compiler from the last 7 years and any of the newer Espressif parts) may need to start looking at libraries like https://github.com/Arduino-IRremote/Arduino-IRremote which added this support many months ago. This (and the similarly needed #2040) have been in the review queue with no visible motion forward for a long time. |
Include open upstream PR crankyoldgit#2144 to make IR learning compatible with IDF 5.x. Enhance with: * update esp32-hal* to IDF v5.4 * C++20 compatibility A small fix to remove "volatile ++" build warning. Source: crankyoldgit#2040 --------- Co-authored-by: BorisKofman <[email protected]> Co-authored-by: Christian I. Nilsson <[email protected]>
Is this project still active? #2039 The maintainer(s) (Are NikiZe and crankyolddigit the same person?) are clearly getting cranky with people not finding this, but fixes for this problem seem to have been around for over a year now. Now that Pioarduino has broken the Arduino3 logjam for more devs/users, the number of affected people will only grow, further amplifying the feedback loop. It's been confirmed to work. I haven't seen that the patches are pending further changes from authors. What will it take to get these changes in the hands of the people that need them? Different patches have had some additional stuff around them, but the actual timer changes in ESP-IDF seem pretty mechanical. Yes, it's annoying for Espressif to break our code renaming our symbols, but we can't control that and the rest of our code was broken by similary annoying changes in Arduino3. I searched GitHub to see if someone had just forked it with these changes applied, but they don't seem to have done so. I don't particularly want to become a project maintainer of a fork of this. (My requirements are low. I need to receive NEC; I can probably pluck the IR frames right off the RMT via ESP-IDF, I think) I don't want to patch PlatformIO and then tell our developers where to find a crossplatform version of patch to apply it. If it "just" needs some further programmer attention, I'll block off some time and try to help. I can't help test a zillion remotes I've never heard of. Honestly, I'd rather spend two hours replacing this than an hour fixing this at this point, but not everyone has that luxury. Please consider applying one of the above fixes for this problem for later ESP-IDF - which is also breaking Arduino/ESP32 and pushing it out as a release. That'll reduce/stop the flow of incoming duplicate reports. I'm willing to help a little, but I don't want to take over a fork of the project. Thank you for listening. I hope we can get a delivery of this soon. |
Fix the name, essentially replace the last commit here, and ensure there is no limit issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a swapped comment pair, this looks pretty minimal to me. The only things that aren't strictly minimal were requested and implemented in earlier rounds of review.
If there's something specific that remains, please highlight it. I don't want to scoop the author, but if there's something more to do here, I'll help if needed.
Seems it's been pretty widely tested by many of us already.
Thank you for the quick pickup.
src/IRrecv.cpp
Outdated
timer = timerBegin(_timer_num, 80, true); | ||
// Check for ESP32 core version and handle timerBegin differently for each version | ||
#if defined(ESP_ARDUINO_VERSION) && (ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(3, 0, 0)) | ||
// For ESP32 core version 3.x (three arguments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think last half of this comment and the one on 377 are swapped.
Fixes #2039