From 9632843b95487e5e3ed2f84f4c7e6e8d282c70ad Mon Sep 17 00:00:00 2001 From: Brent Kowal Date: Wed, 8 May 2024 09:09:06 -0400 Subject: [PATCH] Fixed error in FreeRTOS Timer queue Resolved an issue in WsfTimerStop where if the head of the queue is removed, both the tail and head are set to NULL, effectively resetting the queue, rather than the head becoming the next list item. Updated file style based on clang-format and cpplint outputs. --- .../wsf/sources/targets/freertos/wsf_timer.c | 218 +++++++++--------- 1 file changed, 109 insertions(+), 109 deletions(-) diff --git a/Libraries/Cordio/wsf/sources/targets/freertos/wsf_timer.c b/Libraries/Cordio/wsf/sources/targets/freertos/wsf_timer.c index 84654c692e..d539d7f183 100644 --- a/Libraries/Cordio/wsf/sources/targets/freertos/wsf_timer.c +++ b/Libraries/Cordio/wsf/sources/targets/freertos/wsf_timer.c @@ -21,10 +21,11 @@ * limitations under the License. */ /*************************************************************************************************/ +#include "wsf_timer.h" +#include #include "wsf_types.h" #include "wsf_queue.h" -#include "wsf_timer.h" #include "wsf_assert.h" #include "wsf_cs.h" @@ -36,22 +37,20 @@ #include "timers.h" #include "queue.h" -#include - -#if ( configUSE_TIMERS != 1 ) +#if (configUSE_TIMERS != 1) #error Enable timers in FreeRTOSConfig.h by definiing configUSE_TIMERS as 1 #endif typedef struct TimerStruct { - struct TimerStruct *next; - struct TimerStruct *prev; - wsfTimer_t *wsfTimerStruct; - TimerHandle_t tmr; + struct TimerStruct *next; + struct TimerStruct *prev; + wsfTimer_t *wsfTimerStruct; + TimerHandle_t tmr; } TimerStruct_t; typedef struct TimerList { - TimerStruct_t *head; - TimerStruct_t *tail; + TimerStruct_t *head; + TimerStruct_t *tail; } TimerList_t; static TimerList_t s_timers = { NULL, NULL }; @@ -59,131 +58,132 @@ static QueueHandle_t s_queue = NULL; void prvTimerCallback(TimerHandle_t xTimer) { - WsfCsEnter(); - TimerStruct_t *firedTimer = NULL; - for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { - if (ts->tmr == xTimer) { - firedTimer = ts; - break; + WsfCsEnter(); + TimerStruct_t *firedTimer = NULL; + for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { + if (ts->tmr == xTimer) { + firedTimer = ts; + break; + } } - } - if (firedTimer) { - /* Get the timer handler */ - wsfHandlerId_t handler = firedTimer->wsfTimerStruct->handlerId; - if (!xQueueSend(s_queue, &firedTimer->wsfTimerStruct, portMAX_DELAY)) { - WSF_ASSERT(0); + if (firedTimer) { + /* Get the timer handler */ + wsfHandlerId_t handler = firedTimer->wsfTimerStruct->handlerId; + if (!xQueueSend(s_queue, &firedTimer->wsfTimerStruct, portMAX_DELAY)) { + WSF_ASSERT(0); + } + WsfTaskSetReady(handler, WSF_TIMER_EVENT); + firedTimer->wsfTimerStruct->isStarted = FALSE; + } else { + /* Timer not found */ + WSF_ASSERT(0); } - WsfTaskSetReady(handler, WSF_TIMER_EVENT); - firedTimer->wsfTimerStruct->isStarted = FALSE; - } else { - /* Timer not found */ - WSF_ASSERT(0); - } - WsfCsExit(); + WsfCsExit(); } void WsfTimerInit(void) { - s_queue = xQueueCreate(configTIMER_QUEUE_LENGTH, sizeof(wsfTimer_t*)); + s_queue = xQueueCreate(configTIMER_QUEUE_LENGTH, sizeof(wsfTimer_t *)); } void WsfTimerStartMs(wsfTimer_t *pTimer, wsfTimerTicks_t ms) { - WsfCsEnter(); - /* Look if there is existing timer */ - TimerStruct_t *existingTimer = NULL; - for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { - if (ts->wsfTimerStruct == pTimer) { - existingTimer = ts; - break; + WsfCsEnter(); + /* Look if there is existing timer */ + TimerStruct_t *existingTimer = NULL; + for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { + if (ts->wsfTimerStruct == pTimer) { + existingTimer = ts; + break; + } } - } - uint32_t ticks = pdMS_TO_TICKS(ms); - if (existingTimer) { - if (ticks != pTimer->ticks) { - /* Update timer interval */ - xTimerChangePeriod(existingTimer->tmr, ticks, 0); - } - /* Restart the existing timer */ - pTimer->isStarted = xTimerReset(existingTimer->tmr, 0); - } else { - TimerHandle_t tmr = xTimerCreate(NULL, ticks, - pdFALSE, - NULL, prvTimerCallback); - pTimer->isStarted = xTimerStart(tmr, 0); - - TimerStruct_t *ts = (TimerStruct_t*) WsfBufAlloc( - sizeof(TimerStruct_t)); - memset(ts, 0, sizeof(TimerStruct_t)); - ts->wsfTimerStruct = pTimer; - ts->tmr = tmr; - ts->wsfTimerStruct->ticks = ticks; - if (s_timers.tail) { - /* Append to the list end */ - TimerStruct_t *prev = s_timers.tail; - prev->next = ts; - ts->prev = prev; - s_timers.tail = ts; + uint32_t ticks = pdMS_TO_TICKS(ms); + if (existingTimer) { + if (ticks != pTimer->ticks) { + /* Update timer interval */ + xTimerChangePeriod(existingTimer->tmr, ticks, 0); + } + /* Restart the existing timer */ + pTimer->isStarted = xTimerReset(existingTimer->tmr, 0); } else { - /* Create first element */ - s_timers.head = ts; - s_timers.tail = ts; + TimerHandle_t tmr = xTimerCreate(NULL, ticks, pdFALSE, NULL, prvTimerCallback); + pTimer->isStarted = xTimerStart(tmr, 0); + + TimerStruct_t *ts = (TimerStruct_t *)WsfBufAlloc(sizeof(TimerStruct_t)); + memset(ts, 0, sizeof(TimerStruct_t)); + ts->wsfTimerStruct = pTimer; + ts->tmr = tmr; + ts->wsfTimerStruct->ticks = ticks; + if (s_timers.tail) { + /* Append to the list end */ + TimerStruct_t *prev = s_timers.tail; + prev->next = ts; + ts->prev = prev; + s_timers.tail = ts; + } else { + /* Create first element */ + s_timers.head = ts; + s_timers.tail = ts; + } } - } - WsfCsExit(); + WsfCsExit(); } void WsfTimerStartSec(wsfTimer_t *pTimer, wsfTimerTicks_t sec) { - WsfTimerStartMs(pTimer, sec * 1000); + WsfTimerStartMs(pTimer, sec * 1000); } void WsfTimerStop(wsfTimer_t *pTimer) { - WsfCsEnter(); - TimerStruct_t *itemToRemove = NULL; - for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { - if (ts->wsfTimerStruct == pTimer) { - itemToRemove = ts; - break; + WsfCsEnter(); + TimerStruct_t *itemToRemove = NULL; + for (TimerStruct_t *ts = s_timers.head; ts != NULL; ts = ts->next) { + if (ts->wsfTimerStruct == pTimer) { + itemToRemove = ts; + break; + } } - } - if (itemToRemove) { - /* Delete the timer and its list item */ - xTimerStop(itemToRemove->tmr, portMAX_DELAY); - xTimerDelete(itemToRemove->tmr, portMAX_DELAY); - TimerStruct_t *prev = itemToRemove->prev; - TimerStruct_t *next = itemToRemove->next; - if (prev) { - prev->next = next; + if (itemToRemove) { + /* Delete the timer and its list item */ + xTimerStop(itemToRemove->tmr, portMAX_DELAY); + xTimerDelete(itemToRemove->tmr, portMAX_DELAY); + TimerStruct_t *prev = itemToRemove->prev; + TimerStruct_t *next = itemToRemove->next; + if (prev) { + prev->next = next; + } else { + /* Removing head */ + s_timers.head = NULL; + } + if (next) { + next->prev = prev; + } + + /* Update Head if NULL / was removed */ + if (s_timers.head == NULL) { + s_timers.head = next; + } + + /* Update tail if removing tail */ + if (s_timers.tail == itemToRemove) { + s_timers.tail = prev; + } + + itemToRemove->wsfTimerStruct->isStarted = FALSE; + WsfBufFree(itemToRemove); } else { - /* Removing head */ - s_timers.head = NULL; - s_timers.tail = NULL; - } - if (next) { - next->prev = prev; - } - - /* Update tail if removing tail */ - if(s_timers.tail == itemToRemove) { - s_timers.tail = prev; + /* Timer not found; */ } - - itemToRemove->wsfTimerStruct->isStarted = FALSE; - WsfBufFree(itemToRemove); - } else { - /* Timer not found; */ - } - WsfCsExit(); + WsfCsExit(); } -wsfTimer_t* WsfTimerServiceExpired(wsfTaskId_t taskId) +wsfTimer_t *WsfTimerServiceExpired(wsfTaskId_t taskId) { - wsfTimer_t *tmr; - if (xQueueReceive(s_queue, &tmr, 0)) { - return tmr; - } - return NULL; + wsfTimer_t *tmr; + if (xQueueReceive(s_queue, &tmr, 0)) { + return tmr; + } + return NULL; }