-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
RMT Legacy Driver option #9941
RMT Legacy Driver option #9941
Conversation
👋 Hello SuGlider, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
#ifndef ESP32_ARDUINO_NEW_RMT_DRV_OFF | ||
|
||
// add the file "build_opt.h" to your Arduino project folder with "-DESP32_ARDUINO_NEW_RMT_DRV_OFF" to use the RMT Legacy driver | ||
#warning "ESP32_ARDUINO_NEW_RMT_DRV_OFF is not defined, using new RMT driver" |
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.
Imho a warning for using a actual function is not correct.
It should be the other way around.
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.
This is part of the example, it is just a way to say "this example is intended to demonstrate how to use RMT Legacy Driver within Arduino Core 3.0.x + IDF 5.x"
Please elaborate futher.
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 would say: make it an error and no extra code. It should never fail, right? except if we mess up platform.txt, in which case it will be used as notification that the feature broke.
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 have modified it. Please review it.
@P-R-O-C-H-Y | @lucasssvaz - CI Pre-commit hooks has failed. Could you check it, please. |
@SuGlider Fixed |
@SuGlider : What about adding the legacy driver code from That would still allow use of |
@Xylopyrographer Why? If you want old stuff use core 2.0.x. Don't litter the core 3.0.x with deprecated outdated stuff. |
This is not good because we will move to Core 3.1 with IDF 5.3 to add the ESP32-P4. You can copy the code from 2.0.17 into your project, but have in mind that deprecated Legacy drivers won't have future within Arduino Core and IDF. |
Good info on the -P4. Thanks for the heads up. Is there a timeframe for incorporating IDF 5.3? My suggestion is a bad idea as it would also mean bringing in the v2.0.17 |
First IDF 5.3 must be released as a stable version. Maybe a final release of Arduino Core 3.1.0 with IDF 5.3 may be ready by Q4-24 or Q1-25, in the worst case. |
Description of Change
IDF 5.1 and Arduino Core 3.0.x still support RMT Legacy Driver which is used in many popular RGB LED libraries.
This PR creates a method to allow using RMT Legacy Driver within Arduino Core 3.0.x.
There is an example about how to achieve it.
It is necessary to add the
build_opt.h
file ad part of the Arduino Sketch project and insert-DESP32_ARDUINO_NO_RGB_BUILTIN
in this file in order to defineESP32_ARDUINO_NO_RGB_BUILTIN
while building the project. This will avoid linking problem with the New RMT Driver of IDF 5.1 and the Legacy RMT Driver.Tests scenarios
Using ESP32-S3 and the Testing Example.
Tested also using the sketch provided in the issue #9866
Tested using Fast LED library after a few adjustments.
Related links
Fix #9866