From b47f25ac23bcc77851522c501752b5339748394b Mon Sep 17 00:00:00 2001 From: Stephen Noonan Date: Fri, 3 Jan 2025 16:47:19 -0500 Subject: [PATCH 1/3] fix(newlib): usleep returning early This commit updates usleep() to always sleep for the required sleep period or more. This fixes a bug where the usleep() could sleep for less than the request sleep period. Closes https://github.com/espressif/esp-idf/pull/15132 --- components/newlib/time.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/components/newlib/time.c b/components/newlib/time.c index 00e055f4f7..6e71c0d38b 100644 --- a/components/newlib/time.c +++ b/components/newlib/time.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -201,10 +201,26 @@ int usleep(useconds_t us) if (us < us_per_tick) { esp_rom_delay_us((uint32_t) us); } else { - /* since vTaskDelay(1) blocks for anywhere between 0 and portTICK_PERIOD_MS, - * round up to compensate. - */ - vTaskDelay((us + us_per_tick - 1) / us_per_tick); + /* vTaskDelay may return up to (n-1) tick periods due to the tick ISR + being asynchronous to the call. We must sleep at least the specified + time, or longer. Checking the monotonic clock allows making an + additional call to vTaskDelay when needed to ensure minimal time is + actually slept. Adding `us_per_tick - 1` prevents ever passing 0 to + vTaskDelay(). + */ + uint64_t now_us = esp_time_impl_get_time(); + uint64_t target_us = now_us + us; + do { + vTaskDelay((((target_us - now_us) + us_per_tick - 1) / us_per_tick)); + now_us = esp_time_impl_get_time(); + /* It is possible that the time left until the target time is less + * than a tick period. However, we let usleep() to sleep for an + * entire tick period. This, could result in usleep() sleeping for + * a longer time than the requested time but that does not violate + * the spec of usleep(). Additionally, it allows FreeRTOS to schedule + * other tasks while the current task is sleeping. + */ + } while (now_us < target_us); } return 0; } From 2793add0b298661f21aea7825542e590cbfffd9a Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Thu, 17 Apr 2025 10:38:04 +0200 Subject: [PATCH 2/3] test(newlib): Added unit tests for usleep and sleep_for functions This commit adds unit tests to verify the basic functionality of usleep() and this_thread::sleep_for() std functions. --- .../cxx/test_apps/general/main/CMakeLists.txt | 2 +- .../general/main/test_cxx_general.cpp | 28 ++++++++++++++++++- .../newlib/test_apps/newlib/main/test_time.c | 24 +++++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/components/cxx/test_apps/general/main/CMakeLists.txt b/components/cxx/test_apps/general/main/CMakeLists.txt index d66ec8e1b3..20c201549d 100644 --- a/components/cxx/test_apps/general/main/CMakeLists.txt +++ b/components/cxx/test_apps/general/main/CMakeLists.txt @@ -1,2 +1,2 @@ idf_component_register(SRCS "test_cxx_general.cpp" - PRIV_REQUIRES unity driver) + PRIV_REQUIRES unity driver esp_timer) diff --git a/components/cxx/test_apps/general/main/test_cxx_general.cpp b/components/cxx/test_apps/general/main/test_cxx_general.cpp index b4ec100179..04d494c801 100644 --- a/components/cxx/test_apps/general/main/test_cxx_general.cpp +++ b/components/cxx/test_apps/general/main/test_cxx_general.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -13,6 +13,9 @@ #include "unity.h" #include "unity_test_utils.h" #include "soc/soc.h" +#include +#include +#include "esp_timer.h" extern "C" void setUp() { @@ -292,6 +295,29 @@ TEST_CASE("stack smashing protection CXX", "[stack_smash]") recur_and_smash_cxx(); } +TEST_CASE("test std::this_thread::sleep_for basic functionality", "[misc]") +{ + const int us_per_tick = portTICK_PERIOD_MS * 1000; + + // Test sub-tick sleep + const auto short_sleep = std::chrono::microseconds(us_per_tick / 4); + int64_t start = esp_timer_get_time(); + std::this_thread::sleep_for(short_sleep); + int64_t end = esp_timer_get_time(); + int64_t elapsed_us = end - start; + printf("short sleep: %lld us\n", elapsed_us); + TEST_ASSERT_GREATER_OR_EQUAL(short_sleep.count(), elapsed_us); + + // Test multi-tick sleep + const auto long_sleep = std::chrono::microseconds(us_per_tick * 2); + start = esp_timer_get_time(); + std::this_thread::sleep_for(long_sleep); + end = esp_timer_get_time(); + elapsed_us = end - start; + printf("long sleep: %lld us\n", elapsed_us); + TEST_ASSERT_GREATER_OR_EQUAL(long_sleep.count(), elapsed_us); +} + extern "C" void app_main(void) { printf("CXX GENERAL TEST\n"); diff --git a/components/newlib/test_apps/newlib/main/test_time.c b/components/newlib/test_apps/newlib/main/test_time.c index 0ce0f18286..5fb208eefc 100644 --- a/components/newlib/test_apps/newlib/main/test_time.c +++ b/components/newlib/test_apps/newlib/main/test_time.c @@ -1,11 +1,12 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include #include #include +#include #include "unity.h" #include #include @@ -87,6 +88,27 @@ TEST_CASE("Reading RTC registers on APP CPU doesn't affect clock", "[newlib]") #endif // portNUM_PROCESSORS == 2 +TEST_CASE("test usleep basic functionality", "[newlib]") +{ + const int us_per_tick = portTICK_PERIOD_MS * 1000; + + // Test sub-tick sleep such that usleep() uses ROM delay path + const int short_sleep_us = us_per_tick / 4; + int64_t start = esp_timer_get_time(); + TEST_ASSERT_EQUAL(0, usleep(short_sleep_us)); + int64_t end = esp_timer_get_time(); + printf("short sleep: %lld us\n", end - start); + TEST_ASSERT_GREATER_OR_EQUAL(short_sleep_us, end - start); + + // Test multi-tick sleep using vTaskDelay path + const int long_sleep_us = us_per_tick * 2; + start = esp_timer_get_time(); + TEST_ASSERT_EQUAL(0, usleep(long_sleep_us)); + end = esp_timer_get_time(); + printf("long sleep: %lld us\n", end - start); + TEST_ASSERT_GREATER_OR_EQUAL(long_sleep_us, end - start); +} + TEST_CASE("test adjtime function", "[newlib]") { struct timeval tv_time; From f7c21372ca2791a8a1578f72ec8ccd97e760c07f Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Mon, 28 Apr 2025 17:17:24 +0200 Subject: [PATCH 3/3] change(newlib): Added unit tests for usleep and sleep_for functions This commit adds unit tests to verify the basic functionality of usleep() and this_thread::sleep_for() std functions. --- .../test_apps/legacy_mcpwm_driver/main/test_legacy_mcpwm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/driver/test_apps/legacy_mcpwm_driver/main/test_legacy_mcpwm.c b/components/driver/test_apps/legacy_mcpwm_driver/main/test_legacy_mcpwm.c index a3980cbf58..acc1065098 100644 --- a/components/driver/test_apps/legacy_mcpwm_driver/main/test_legacy_mcpwm.c +++ b/components/driver/test_apps/legacy_mcpwm_driver/main/test_legacy_mcpwm.c @@ -159,7 +159,7 @@ static uint32_t pcnt_get_pulse_number(pcnt_unit_handle_t pwm_pcnt_unit, int capt int count_value = 0; TEST_ESP_OK(pcnt_unit_clear_count(pwm_pcnt_unit)); TEST_ESP_OK(pcnt_unit_start(pwm_pcnt_unit)); - usleep(capture_window_ms * 1000); + vTaskDelay(pdMS_TO_TICKS(capture_window_ms)); TEST_ESP_OK(pcnt_unit_stop(pwm_pcnt_unit)); TEST_ESP_OK(pcnt_unit_get_count(pwm_pcnt_unit, &count_value)); printf("count value: %d\r\n", count_value);