-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
ESP32 RMT High-priority Interrupt driver #4797
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
Conversation
Ensure repeatable test builds.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| // something went wrong | ||
| if (vReal) free(vReal); vReal = nullptr; | ||
| if (vImag) free(vImag); vImag = nullptr; | ||
| if (vReal) { free(vReal); vReal = nullptr; } |
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.
AFAIK free() can "release" a nullptr. So if() is superfluous. Right?
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'd always thought it was UB, but you're right, it's defined as a no-op by the C standard. (Whether any given allocator is standards-compliant is another story...)
In this case the objective was to shut the compiler up about "deceptive if statements" -- building the IDF brings in '-Wall -Werror'. Actually I thought I'd left this commit out of the PR branch as it's mostly a testing/debugging thing. I'll re-arrange and resubmit.
Temporarily switch to a NeoPixelBus fork with a custom high-priority interrupt based driver for using the RMT hardware on ESP32 platforms. This driver is currently draft PR'd upstream at Makuna/NeoPixelBus/pull/892. Fixes #4389 and related issues.
Currently I've tested ESP32 (IDF v3 and v4), ESP32-S2, and ESP32-S3; it works well for me. Further testing would be very helpful!
ESP32-C3 support is still pending as it will require a different IRQ shim layer.
I'm opening this draft to provide a link to builds with the current WIP version.