From c73fda4b08ee71d815bcafad12913dfdbca535db Mon Sep 17 00:00:00 2001 From: Cervenka Dusan Date: Fri, 25 Feb 2022 17:24:49 +0100 Subject: [PATCH 1/3] Fix timeout for Freertos semaphore. It doesn't make sense to have portMaxDelay and then recalculate to lower value. Currently other ports are using simple implementation so i align with that. Signed-off-by: Cervenka Dusan --- erpc_c/port/erpc_setup_extensions_freertos.cpp | 2 +- erpc_c/port/erpc_threading_freertos.cpp | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/erpc_c/port/erpc_setup_extensions_freertos.cpp b/erpc_c/port/erpc_setup_extensions_freertos.cpp index b93e4a29e..acdc7674c 100644 --- a/erpc_c/port/erpc_setup_extensions_freertos.cpp +++ b/erpc_c/port/erpc_setup_extensions_freertos.cpp @@ -28,7 +28,7 @@ void erpc::erpc_pre_cb_default(void) (void)s_erpc_call_in_progress->get(s_erpc_call_in_progress->kWaitForever); erpc_assert(s_erpc_call_timer_cb && "If you want use default pre cb action, do not forget call erpc_init_call_progress_detection_default."); - xTimerStart(s_erpc_call_timer_cb, 0); + (void)xTimerStart(s_erpc_call_timer_cb, 0); } void erpc::erpc_post_cb_default(void) diff --git a/erpc_c/port/erpc_threading_freertos.cpp b/erpc_c/port/erpc_threading_freertos.cpp index 42affe272..fdf345e89 100644 --- a/erpc_c/port/erpc_threading_freertos.cpp +++ b/erpc_c/port/erpc_threading_freertos.cpp @@ -271,16 +271,17 @@ bool Semaphore::get(uint32_t timeout) { timeout = portMAX_DELAY; } + else if (timeout > (portMAX_DELAY - 1)) + { + timeout = portMAX_DELAY - 1; + } else { - if (timeout > (portMAX_DELAY - 1)) - { - timeout = portMAX_DELAY - 1; - } + /* Misra condition */ } #endif - return (pdTRUE == xSemaphoreTake(m_sem, timeout / 1000U / portTICK_PERIOD_MS)); + return (pdTRUE == xSemaphoreTake(m_sem, timeout)); } int Semaphore::getCount(void) const From 67a7cdddd1126c3bf3d6562e0c1edda79e63e905 Mon Sep 17 00:00:00 2001 From: Cervenka Dusan Date: Thu, 10 Mar 2022 16:16:07 +0100 Subject: [PATCH 2/3] Fix rtos implementation Signed-off-by: Cervenka Dusan --- erpc_c/port/erpc_threading.h | 6 ++--- erpc_c/port/erpc_threading_freertos.cpp | 29 ++++++++++++++++--------- erpc_c/port/erpc_threading_mbed.cpp | 16 ++++++++++++-- erpc_c/port/erpc_threading_threadx.cpp | 16 ++++++++++++-- erpc_c/port/erpc_threading_win32.cpp | 16 ++++++++++++-- erpc_c/port/erpc_threading_zephyr.cpp | 16 ++++++++++++-- 6 files changed, 78 insertions(+), 21 deletions(-) diff --git a/erpc_c/port/erpc_threading.h b/erpc_c/port/erpc_threading.h index 1ebe8a240..40a0404a0 100644 --- a/erpc_c/port/erpc_threading.h +++ b/erpc_c/port/erpc_threading.h @@ -140,7 +140,7 @@ class Thread /*! * @brief This function puts thread to sleep. * - * @param[in] usecs Time for sleeping. + * @param[in] usecs Time for sleeping in [us]. */ static void sleep(uint32_t usecs); @@ -477,12 +477,12 @@ class Semaphore /*! * @brief This function get semaphore. * - * @param[in] timeout Time how long can wait for getting semaphore. + * @param[in] usecs Time how long can wait for getting semaphore in [us]. * * @retval true When semaphore got successfully. * @retval false When mutex didn't get. */ - bool get(uint32_t timeout = kWaitForever); + bool get(uint32_t usecs = kWaitForever); /*! * @brief This function returns semaphore count number. diff --git a/erpc_c/port/erpc_threading_freertos.cpp b/erpc_c/port/erpc_threading_freertos.cpp index fdf345e89..aed943611 100644 --- a/erpc_c/port/erpc_threading_freertos.cpp +++ b/erpc_c/port/erpc_threading_freertos.cpp @@ -264,24 +264,33 @@ void Semaphore::putFromISR(void) portYIELD_FROM_ISR(xHigherPriorityTaskWoken); } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t usecs) { -#if configUSE_16_BIT_TICKS - if (timeout == kWaitForever) - { - timeout = portMAX_DELAY; - } - else if (timeout > (portMAX_DELAY - 1)) + if (usecs != kWaitForever) { - timeout = portMAX_DELAY - 1; + if (usecs > 0U) + { + usecs /= 1000U * portTICK_PERIOD_MS; + if (usecs == 0U) + { + usecs = 1U; + } +#if configUSE_16_BIT_TICKS + else if (usecs > (portMAX_DELAY - 1)) + { + usecs = portMAX_DELAY - 1; + } +#endif + } } +#if configUSE_16_BIT_TICKS else { - /* Misra condition */ + usecs = portMAX_DELAY; } #endif - return (pdTRUE == xSemaphoreTake(m_sem, timeout)); + return (pdTRUE == xSemaphoreTake(m_sem, (TickType_t)usecs)); } int Semaphore::getCount(void) const diff --git a/erpc_c/port/erpc_threading_mbed.cpp b/erpc_c/port/erpc_threading_mbed.cpp index c93832183..9e9570fcf 100644 --- a/erpc_c/port/erpc_threading_mbed.cpp +++ b/erpc_c/port/erpc_threading_mbed.cpp @@ -218,9 +218,21 @@ void Semaphore::put(void) m_sem->release(); } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t usecs) { - m_count = m_sem->wait(timeout); + if (usecs != kWaitForever) + { + if (usecs > 0U) + { + usecs /= 1000U; + if (usecs == 0U) + { + usecs = 1U; + } + } + } + + m_count = m_sem->wait(usecs); return (m_count < 0); } diff --git a/erpc_c/port/erpc_threading_threadx.cpp b/erpc_c/port/erpc_threading_threadx.cpp index d36c0ac93..cea31134c 100644 --- a/erpc_c/port/erpc_threading_threadx.cpp +++ b/erpc_c/port/erpc_threading_threadx.cpp @@ -256,9 +256,21 @@ void Semaphore::put(void) tx_semaphore_put(&m_sem); } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t usecs) { - UINT status = tx_semaphore_get(&m_sem, timeout); + if (usecs != kWaitForever) + { + if (usecs > 0U) + { + usecs /= 1000U; + if (usecs == 0U) + { + usecs = 1U; + } + } + } + + UINT status = tx_semaphore_get(&m_sem, usecs); return (status == TX_SUCCESS); } diff --git a/erpc_c/port/erpc_threading_win32.cpp b/erpc_c/port/erpc_threading_win32.cpp index a8665c6cc..b848787f1 100644 --- a/erpc_c/port/erpc_threading_win32.cpp +++ b/erpc_c/port/erpc_threading_win32.cpp @@ -194,9 +194,21 @@ void Semaphore::put(void) m_mutex.unlock(); } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t usecs) { - DWORD ret = WaitForSingleObject(m_sem, timeout); + if (usecs != kWaitForever) + { + if (usecs > 0U) + { + usecs /= 1000U; + if (usecs == 0U) + { + usecs = 1U; + } + } + } + + DWORD ret = WaitForSingleObject(m_sem, usecs); m_mutex.lock(); --m_count; m_mutex.unlock(); diff --git a/erpc_c/port/erpc_threading_zephyr.cpp b/erpc_c/port/erpc_threading_zephyr.cpp index 4c475165a..b3b74e60a 100644 --- a/erpc_c/port/erpc_threading_zephyr.cpp +++ b/erpc_c/port/erpc_threading_zephyr.cpp @@ -130,9 +130,21 @@ void Semaphore::put(void) k_sem_give(&m_sem); } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t usecs) { - return (k_sem_take(&m_sem, timeout / 1000) == 0); + if (usecs != kWaitForever) + { + if (usecs > 0U) + { + usecs /= 1000U; + if (usecs == 0U) + { + usecs = 1U; + } + } + } + + return (k_sem_take(&m_sem, usecs) == 0); } int Semaphore::getCount(void) const From 0ecaafbb4ecde07c006da028356375b1b7a2ec72 Mon Sep 17 00:00:00 2001 From: Cervenka Dusan Date: Fri, 18 Mar 2022 13:47:34 +0100 Subject: [PATCH 3/3] Renamed semaphore get timeout variable Signed-off-by: Cervenka Dusan --- erpc_c/port/erpc_threading.h | 4 ++-- erpc_c/port/erpc_threading_freertos.cpp | 20 ++++++++++---------- erpc_c/port/erpc_threading_mbed.cpp | 14 +++++++------- erpc_c/port/erpc_threading_pthreads.cpp | 10 +++++----- erpc_c/port/erpc_threading_threadx.cpp | 14 +++++++------- erpc_c/port/erpc_threading_win32.cpp | 14 +++++++------- erpc_c/port/erpc_threading_zephyr.cpp | 14 +++++++------- 7 files changed, 45 insertions(+), 45 deletions(-) diff --git a/erpc_c/port/erpc_threading.h b/erpc_c/port/erpc_threading.h index 40a0404a0..1f3d66ad3 100644 --- a/erpc_c/port/erpc_threading.h +++ b/erpc_c/port/erpc_threading.h @@ -477,12 +477,12 @@ class Semaphore /*! * @brief This function get semaphore. * - * @param[in] usecs Time how long can wait for getting semaphore in [us]. + * @param[in] timeoutUsecs Time how long can wait for getting semaphore in [us]. * * @retval true When semaphore got successfully. * @retval false When mutex didn't get. */ - bool get(uint32_t usecs = kWaitForever); + bool get(uint32_t timeoutUsecs = kWaitForever); /*! * @brief This function returns semaphore count number. diff --git a/erpc_c/port/erpc_threading_freertos.cpp b/erpc_c/port/erpc_threading_freertos.cpp index aed943611..c99c99779 100644 --- a/erpc_c/port/erpc_threading_freertos.cpp +++ b/erpc_c/port/erpc_threading_freertos.cpp @@ -264,21 +264,21 @@ void Semaphore::putFromISR(void) portYIELD_FROM_ISR(xHigherPriorityTaskWoken); } -bool Semaphore::get(uint32_t usecs) +bool Semaphore::get(uint32_t timeoutUsecs) { - if (usecs != kWaitForever) + if (timeoutUsecs != kWaitForever) { - if (usecs > 0U) + if (timeoutUsecs > 0U) { - usecs /= 1000U * portTICK_PERIOD_MS; - if (usecs == 0U) + timeoutUsecs /= 1000U * portTICK_PERIOD_MS; + if (timeoutUsecs == 0U) { - usecs = 1U; + timeoutUsecs = 1U; } #if configUSE_16_BIT_TICKS - else if (usecs > (portMAX_DELAY - 1)) + else if (timeoutUsecs > (portMAX_DELAY - 1)) { - usecs = portMAX_DELAY - 1; + timeoutUsecs = portMAX_DELAY - 1; } #endif } @@ -286,11 +286,11 @@ bool Semaphore::get(uint32_t usecs) #if configUSE_16_BIT_TICKS else { - usecs = portMAX_DELAY; + timeoutUsecs = portMAX_DELAY; } #endif - return (pdTRUE == xSemaphoreTake(m_sem, (TickType_t)usecs)); + return (pdTRUE == xSemaphoreTake(m_sem, (TickType_t)timeoutUsecs)); } int Semaphore::getCount(void) const diff --git a/erpc_c/port/erpc_threading_mbed.cpp b/erpc_c/port/erpc_threading_mbed.cpp index 9e9570fcf..00441b361 100644 --- a/erpc_c/port/erpc_threading_mbed.cpp +++ b/erpc_c/port/erpc_threading_mbed.cpp @@ -218,21 +218,21 @@ void Semaphore::put(void) m_sem->release(); } -bool Semaphore::get(uint32_t usecs) +bool Semaphore::get(uint32_t timeoutUsecs) { - if (usecs != kWaitForever) + if (timeoutUsecs != kWaitForever) { - if (usecs > 0U) + if (timeoutUsecs > 0U) { - usecs /= 1000U; - if (usecs == 0U) + timeoutUsecs /= 1000U; + if (timeoutUsecs == 0U) { - usecs = 1U; + timeoutUsecs = 1U; } } } - m_count = m_sem->wait(usecs); + m_count = m_sem->wait(timeoutUsecs); return (m_count < 0); } diff --git a/erpc_c/port/erpc_threading_pthreads.cpp b/erpc_c/port/erpc_threading_pthreads.cpp index 2c5611662..18ba49d38 100644 --- a/erpc_c/port/erpc_threading_pthreads.cpp +++ b/erpc_c/port/erpc_threading_pthreads.cpp @@ -185,7 +185,7 @@ void Semaphore::put(void) ++m_count; } -bool Semaphore::get(uint32_t timeout) +bool Semaphore::get(uint32_t timeoutUsecs) { Mutex::Guard guard(m_mutex); bool retVal = true; @@ -193,7 +193,7 @@ bool Semaphore::get(uint32_t timeout) while (m_count == 0) { - if (timeout == kWaitForever) + if (timeoutUsecs == kWaitForever) { err = pthread_cond_wait(&m_cond, m_mutex.getPtr()); if (err != 0) @@ -204,14 +204,14 @@ bool Semaphore::get(uint32_t timeout) } else { - if (timeout > 0U) + if (timeoutUsecs > 0U) { // Create an absolute timeout time. struct timeval tv; gettimeofday(&tv, NULL); struct timespec wait; - wait.tv_sec = tv.tv_sec + (timeout / sToUs); - wait.tv_nsec = (timeout % sToUs) * 1000U; + wait.tv_sec = tv.tv_sec + (timeoutUsecs / sToUs); + wait.tv_nsec = (timeoutUsecs % sToUs) * 1000U; err = pthread_cond_timedwait(&m_cond, m_mutex.getPtr(), &wait); if (err != 0) { diff --git a/erpc_c/port/erpc_threading_threadx.cpp b/erpc_c/port/erpc_threading_threadx.cpp index cea31134c..4599800af 100644 --- a/erpc_c/port/erpc_threading_threadx.cpp +++ b/erpc_c/port/erpc_threading_threadx.cpp @@ -256,21 +256,21 @@ void Semaphore::put(void) tx_semaphore_put(&m_sem); } -bool Semaphore::get(uint32_t usecs) +bool Semaphore::get(uint32_t timeoutUsecs) { - if (usecs != kWaitForever) + if (timeoutUsecs != kWaitForever) { - if (usecs > 0U) + if (timeoutUsecs > 0U) { - usecs /= 1000U; - if (usecs == 0U) + timeoutUsecs /= 1000U; + if (timeoutUsecs == 0U) { - usecs = 1U; + timeoutUsecs = 1U; } } } - UINT status = tx_semaphore_get(&m_sem, usecs); + UINT status = tx_semaphore_get(&m_sem, timeoutUsecs); return (status == TX_SUCCESS); } diff --git a/erpc_c/port/erpc_threading_win32.cpp b/erpc_c/port/erpc_threading_win32.cpp index b848787f1..459c5b0aa 100644 --- a/erpc_c/port/erpc_threading_win32.cpp +++ b/erpc_c/port/erpc_threading_win32.cpp @@ -194,21 +194,21 @@ void Semaphore::put(void) m_mutex.unlock(); } -bool Semaphore::get(uint32_t usecs) +bool Semaphore::get(uint32_t timeoutUsecs) { - if (usecs != kWaitForever) + if (timeoutUsecs != kWaitForever) { - if (usecs > 0U) + if (timeoutUsecs > 0U) { - usecs /= 1000U; - if (usecs == 0U) + timeoutUsecs /= 1000U; + if (timeoutUsecs == 0U) { - usecs = 1U; + timeoutUsecs = 1U; } } } - DWORD ret = WaitForSingleObject(m_sem, usecs); + DWORD ret = WaitForSingleObject(m_sem, timeoutUsecs); m_mutex.lock(); --m_count; m_mutex.unlock(); diff --git a/erpc_c/port/erpc_threading_zephyr.cpp b/erpc_c/port/erpc_threading_zephyr.cpp index b3b74e60a..b18c65bf3 100644 --- a/erpc_c/port/erpc_threading_zephyr.cpp +++ b/erpc_c/port/erpc_threading_zephyr.cpp @@ -130,21 +130,21 @@ void Semaphore::put(void) k_sem_give(&m_sem); } -bool Semaphore::get(uint32_t usecs) +bool Semaphore::get(uint32_t timeoutUsecs) { - if (usecs != kWaitForever) + if (timeoutUsecs != kWaitForever) { - if (usecs > 0U) + if (timeoutUsecs > 0U) { - usecs /= 1000U; - if (usecs == 0U) + timeoutUsecs /= 1000U; + if (timeoutUsecs == 0U) { - usecs = 1U; + timeoutUsecs = 1U; } } } - return (k_sem_take(&m_sem, usecs) == 0); + return (k_sem_take(&m_sem, timeoutUsecs) == 0); } int Semaphore::getCount(void) const