Skip to content
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 HAL: Fix random pauses during prints #16548

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions Marlin/src/HAL/HAL_ESP32/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,44 @@ void HAL_adc_start_conversion(uint8_t adc_pin);
void HAL_idletask();
void HAL_init();
void HAL_init_board();

//
// Delay in cycles (used by DELAY_NS / DELAY_US)
//
FORCE_INLINE static void DELAY_CYCLES(uint32_t x) {
unsigned long start, ccount, stop;

/**
* It's important to care for race conditions (and overflows) here.
* Race condition example: If `stop` calculates to being close to the upper boundary of
* `uint32_t` and if at the same time a longer loop interruption kicks in (e.g. due to other
* FreeRTOS tasks or interrupts), `ccount` might overflow (and therefore be below `stop` again)
* without the loop ever being able to notice that `ccount` had already been above `stop` once
* (and that therefore the number of cycles to delay has already passed).
* As DELAY_CYCLES (through DELAY_NS / DELAY_US) is used by software SPI bit banging to drive
* LCDs and therefore might be called very, very often, this seemingly improbable situation did
* actually happen in reality. It resulted in apparently random print pauses of ~17.9 seconds
* (0x100000000 / 240 MHz) or multiples thereof, essentially ruining the current print by causing
* large blobs of filament.
*/

__asm__ __volatile__ ( "rsr %0, ccount" : "=a" (start) );
stop = start + x;
ccount = start;

if (stop >= start) {
// no overflow, so only loop while in between start and stop:
// 0x00000000 -----------------start****stop-- 0xffffffff
while (ccount >= start && ccount < stop) {
__asm__ __volatile__ ( "rsr %0, ccount" : "=a" (ccount) );
}
}
else {
// stop did overflow, so only loop while outside of stop and start:
// 0x00000000 **stop-------------------start** 0xffffffff
while (ccount >= start || ccount < stop) {
__asm__ __volatile__ ( "rsr %0, ccount" : "=a" (ccount) );
}
}

}
2 changes: 1 addition & 1 deletion Marlin/src/HAL/HAL_ESP32/Servo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Servo::Servo() {

int8_t Servo::attach(const int inPin) {
if (channel >= CHANNEL_MAX_NUM) return -1;
if (pin > 0) pin = inPin;
if (inPin > 0) pin = inPin;

ledcSetup(channel, 50, 16); // channel X, 50 Hz, 16-bit depth
ledcAttachPin(pin, channel);
Expand Down
16 changes: 1 addition & 15 deletions Marlin/src/HAL/shared/Delay.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,21 +145,7 @@
}
#undef nop

#elif defined(ESP32)

FORCE_INLINE static void DELAY_CYCLES(uint32_t x) {
unsigned long ccount, stop;

__asm__ __volatile__ ( "rsr %0, ccount" : "=a" (ccount) );

stop = ccount + x; // This can overflow

while (ccount < stop) { // This doesn't deal with overflows
__asm__ __volatile__ ( "rsr %0, ccount" : "=a" (ccount) );
}
}

#elif defined(__PLAT_LINUX__)
#elif defined(__PLAT_LINUX__) || defined(ESP32)

// specified inside platform

Expand Down