Skip to content

Bug in delayMicroseconds() #5000

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

Closed
hzoli17 opened this issue Apr 1, 2021 · 3 comments
Closed

Bug in delayMicroseconds() #5000

hzoli17 opened this issue Apr 1, 2021 · 3 comments

Comments

@hzoli17
Copy link

hzoli17 commented Apr 1, 2021

ESP32 crashes when I call the function after ~1 hour 20 minutes of usage, because esp_timer_get_time() return a int64_t value and is parsed as uint32_t in delayMicroseconds().

@maxgerhardt
Copy link
Contributor

Do you have a crashlog for that? And minimal example code?

the way I read it in

void IRAM_ATTR delayMicroseconds(uint32_t us)
{
uint32_t m = micros();
if(us){
uint32_t e = (m + us);
if(m > e){ //overflow
while(micros() > e){
NOP();
}
}
while(micros() < e){
NOP();
}
}
}

unsigned long IRAM_ATTR micros()
{
return (unsigned long) (esp_timer_get_time());
}

/**
* @brief Get time in microseconds since boot
* @return number of microseconds since esp_timer_init was called (this normally
* happens early during application startup).
*/
int64_t esp_timer_get_time();

The int64_t is indeed downcasted to unsigned long (which should be uint32_t) but then the value will just internally overflow and delayMicroseconds still handles the overflow case.

@gin66
Copy link

gin66 commented Apr 5, 2021

One issue in the code is this:

Assume m=micros() = 0xffff0000 and us = 0xffff.
So the calculated value will be e=0xffffffff and no overflow has occurred.
Consequently the second while loop will be executed.
If now an interrupt or a high priority task squeezes inbetween and returns after a couple of us, then the micros() may have wrapped around. So it will need wait 4 billion us = 4000s, which could trigger a watchdog.

Better is a while loop without overflow check:

     uint32_t m = micros(); 
     if(us){    
         while((uint32_t)(micros() - m) < us) { 
             NOP();
          }
    }

@gin66
Copy link

gin66 commented Apr 5, 2021

now I understand, why esp32 is not running stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants