-
Notifications
You must be signed in to change notification settings - Fork 898
Disabled interrupts for timing critical code section #2234
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
base: master
Are you sure you want to change the base?
Disabled interrupts for timing critical code section #2234
Conversation
crankyoldgit
left a comment
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.
Thanks for this. I was meaning to do something about this for ages.
| void IRsend::endCritical() { | ||
| #if defined(ESP32) && !defined(UNIT_TEST) | ||
| portEXIT_CRITICAL(&timingMux); | ||
| vTaskDelay(1); // Yield to the OS. |
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.
What are the implications & practicalities of NOT calling vTaskDelay(1) here?
I'd much prefer to have the critical calls moved inside the mark() method so it works seemlessly for all protocols & uses.
For space()s, we might be able to use the critical calls only when we are not making a delay() call. i.e. very small spaces. Thus it should yield for longer gaps when the timing doesn't matter as much.
Has there been any analysis of where abouts in the message the interrupts are causing the message to be corrupted/not understood? My guess is in the bit-banged PWM mark() messages and short space(), so if we effectively yield only for very long space, we could move the critical calls deeper inside, yield the CPU earlier with less impact & cleaner implementation.
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.
The vTaskDelay call is there to allow the OS task scheduler to assign CPU time for other tasks like the idle task. The delay does not implement a busy wait loop, but rather tells the scheduler to suspend the task for x milliseconds allowing other tasks to run. This is important for the idle loop that, among other things, handles the watchdog timer "kick" preventing the CPU from doing a hard reset. Not having it there could cause the task to enter a new critical section right away and possibly exhausting the scheduler. Critical section disables interrupts and prevent the OS task scheduler from doing its job and therefor should be used with care. I did try to limit the critical section to the senddata functions but it did not work. I am guessing the entire command section including preamp, header, data and footer is timing critical? The reason I kept the critical section within the repeat loop is to allow other tasks to run between each command, since delays between commands are tolerated. I will try to do some more testing and see if I can get the critical section closer to the core transmit logic. Hang on and I will get back with the results.....
| enableIROut(38); | ||
|
|
||
| for (uint16_t r = 0; r <= repeat; r++) { | ||
| beginCritical(); |
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.
See other comment. It would be great to remove this stuff if we can. i.e. Make it hidden and self-applying.
Same for the other protocols changed etc.
|
I tried entering a critical section when sending infrared codes, but doing so disables interrupts. The code seems to measure time by continuously polling, and once interrupts are disabled, this polling operation gets stuck. |
The library only uses interrupts directly for receiving IR signals (a timer and gpio) Unless you've isolated the transmitter & the receiver, you probably & typically don't want to be receiving when transmitting on the same device anyway. |
The delayMicroseconds function used by this library is provided by Arduino, and this Arduino interface continuously queries the current time using esp_timer_get_time; when interrupts are disabled, this time does not change. |
Add Critical Sections for Timing-Critical IR Transmission
Problem
IR protocols require precise microsecond-level timing. On ESP32, interrupts from WiFi,
Bluetooth, or other tasks can disrupt bit-banging operations, causing timing jitter that results
in unreliable IR transmissions or receiver decode failures.
Solution
Implemented
beginCritical()andendCritical()methods using FreeRTOS critical sections witha shared mutex (
portMUX_TYPE). Device-specific send methods now wrap their entire transmissionsequence (header + data + footer) in a single atomic critical section, ensuring uninterrupted
timing throughout the IR pulse train.
Implementation Details
timingMuxfor cross-platform (ESP32) critical section protectionbeginCritical(): Disables interrupts viaportENTER_CRITICAL(&timingMux)endCritical(): Re-enables interrupts and yields withvTaskDelay(1)to prevent watchdog timeoutsfrom unit tests (
#if defined(ESP32) && !defined(UNIT_TEST)), ensuringno impact on other platforms or test environments
This approach ensures reliable IR transmission even with WiFi/Bluetooth active, without requiring
RMT hardware peripheral support.