From d5d7bb61e59949871feb732d2e8117dd4b321d6f Mon Sep 17 00:00:00 2001 From: Stephen Noonan Date: Fri, 3 Jan 2025 16:47:19 -0500 Subject: [PATCH 1/2] 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/src/time.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/components/newlib/src/time.c b/components/newlib/src/time.c index 1338803af0..3dfbc99115 100644 --- a/components/newlib/src/time.c +++ b/components/newlib/src/time.c @@ -207,10 +207,25 @@ 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(); + /* If the time left until the target is less than 1 tick, then we use ROM delay to fill the gap */ + uint64_t time_left = target_us - now_us; + if (time_left != 0 && time_left < us_per_tick) { + esp_rom_delay_us(time_left); + break; + } + } while (now_us < target_us); } return 0; } From ec07f612912b92674dcf01deeb2be4aead2d2e5d Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Thu, 17 Apr 2025 10:38:04 +0200 Subject: [PATCH 2/2] 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 858b16bc16..9d906bfe40 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-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -14,6 +14,9 @@ #include "unity.h" #include "unity_test_utils.h" #include "soc/soc.h" +#include +#include +#include "esp_timer.h" extern "C" void setUp() { @@ -319,6 +322,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) { s_testTLS.foo(); /* allocates memory that will be reused */ diff --git a/components/newlib/test_apps/newlib/main/test_time.c b/components/newlib/test_apps/newlib/main/test_time.c index 43a48600d7..2563e84bc5 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-2024 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 @@ -77,6 +78,27 @@ TEST_CASE("Reading RTC registers on APP CPU doesn't affect clock", "[newlib]") #endif // (CONFIG_FREERTOS_NUMBER_OF_CORES == 2) && CONFIG_IDF_TARGET_ARCH_XTENSA +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;