Skip to content

delay() with small values (e.g. delay(5)) does not delay if system HZ (CONFIG_FREERTOS_HZ) <1000 #6946

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
1 task done
egnor opened this issue Jul 5, 2022 · 10 comments
Closed
1 task done

Comments

@egnor
Copy link
Contributor

egnor commented Jul 5, 2022

Board

OLIMEX ESP32-POE (but will happen anywhere I think)

Device Description

Stock OLIMEX ESP32-POE, but I believe this will happen on any board

Hardware Configuration

Nothing attached

Version

latest development Release Candidate (RC-X)

IDE Name

PlatformIO

Operating System

Ubuntu 22.04

Flash frequency

40MHz

PSRAM enabled

yes

Upload speed

115200

Description

The Arduino delay() function is implemented as follows:

vTaskDelay(ms / portTICK_PERIOD_MS);

If portTICK_PERIOD_MS > 1 (i.e. CONFIG_FREERTOS_HZ < 1000), then small values will round down to 0, and no delay will happen. I'm not super familiar with the delay facilities in FreeRTOS / ESP-IDF but there has to be some way to do a shorter delay (busy-wait, if nothing else)?

See also: platformio/platform-espressif32#846

Sketch

// Run me with CONFIG_FREERTOS_HZ=100 to see the problem

void setup() {
  Serial.begin(115200);
}

void loop() {
  Serial.printf("BEFORE msec=%lu\n", millis());
  for (int i = 0; i < 20; ++i) delay(5);
  Serial.printf("AFTER msec=%lu\n\n", millis());
}

Debug Message

(none)

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@egnor egnor added the Status: Awaiting triage Issue is waiting for triage label Jul 5, 2022
@me-no-dev
Copy link
Member

Arduino comes configured with 1000HZ. Why is this an issue? You can switch your project to 1000HZ as well and delay properly.

@egnor
Copy link
Contributor Author

egnor commented Jul 6, 2022

When using Arduino as an ESP32 component, it becomes an issue, because CONFIG_FREERTOS_HZ is not necessarily 1000. In fact, it defaults to 100. This caveat is mentioned in the doc, but it seemed preferable for the API to generally work? It might be better to fail at compile time by default if HZ<1000? Or somehow emit a warning somewhere?

It certainly can be worked around (assuming you don't mind the higher tick rate), so I don't expect this would be particularly high priority, but it did seem to arguably be a defect worth reporting.

@lbernstone
Copy link
Contributor

delayMicroseconds() is a blocking delay. This is not a defect, if you ask for a delay(0), you get it...

@egnor
Copy link
Contributor Author

egnor commented Jul 6, 2022

If there's a semantic difference between delayMicroseconds() and delay(), it would be nice to document that! There's certainly no such difference in the general Arduino interface expectations. They're both "blocking".

(I'd been hoping for a delay(5), not a delay(0)...!)

BTW I'd be happy to submit a PR if there's an agreement on what it might look like. For example delay() could use vTaskDelay() for whole-tick periods and then delegate to delayMicroseconds() (or whatever approach it uses) for the "remainder". If HZ>=1000, the remainder would always be 0, and the extra code would just compile out.

@egnor egnor changed the title delay() with small values (e.g. delay(1)) does not delay if system HZ (CONFIG_FREERTOS_HZ) <1000 delay() with small values (e.g. delay(5)) does not delay if system HZ (CONFIG_FREERTOS_HZ) <1000 Jul 6, 2022
@me-no-dev
Copy link
Member

"Regular" Arduino does not run on top of FreeRTOS, so blocking is not an issue. Here you have watchdogs and whatnot to ensure that the operating system is running correctly. I am really not sure why any of this is an issue, if it's documented that you need to set it to 1KHz. Supporting other frequencies (could be any other than 100Hz as well) will unnecessarily complicate the code. Given that you are using Arduino as component, we would assume that you are not a noob and know how to adjust the frequency and understand the consequences. 1KHz is there because it makes most sense in Arduino, where users expect somewhat "blocking" behavior, but we still need to allow the OS to run.

The only "solution" here might be to give an error of frequency is not 1KHz.

@egnor
Copy link
Contributor Author

egnor commented Jul 6, 2022

Erroring out (ideally at compile time!) if HZ != 1000 would certainly seem reasonable to me. That would have forced the bug I filed against platformio (example arduino-in-ESPIDF projects should set HZ to 1000) to get fixed.

Would you take that PR?

@me-no-dev
Copy link
Member

Would you take that PR?

Definitely!

@VojtechBartoska
Copy link
Contributor

Thanks for the PR @egnor!

@VojtechBartoska VojtechBartoska added Status: In Progress ⚠️ Issue is in progress and removed Status: Awaiting triage Issue is waiting for triage labels Jul 11, 2022
@egnor
Copy link
Contributor Author

egnor commented Jul 15, 2022

Thanks for the PR @egnor!

Of course! It seems a bit stuck now, I have an open question in the PR on how to address the test failure? (I can sit patiently if needed, not sure how the review workflow works!)

@Parsaabasi
Copy link

Hello,

Due to the overwhelming volume of issues currently being addressed, we have decided to close the previously received tickets. If you still require assistance or if the issue persists, please don't hesitate to reopen the ticket.

Thanks.

@Parsaabasi Parsaabasi removed the Status: In Progress ⚠️ Issue is in progress label Jan 16, 2025
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

5 participants