Remove support for tmrCOMMAND_START_DONT_TRACE (#305)
* Remove support for tmrCOMMAND_START_DONT_TRACE
Prior to this commit, the timer task used tmrCOMMAND_START_DONT_TRACE
to reload a "backlogged" auto-reload timer -- one for which a reload
operation would have started a period that had already elapsed. If the
command queue contained a stop or delete command when
tmrCOMMAND_START_DONT_TRACE was put into the queue for a reload, the
timer unexpectedly restarted when the timer task processed
tmrCOMMAND_START_DONT_TRACE. This commit implements a new method of
reloading auto-reload timers and eliminates support for
tmrCOMMAND_START_DONT_TRACE. No other code sends this private command.
However, the symbol tmrCOMMAND_START_DONT_TRACE remains defined, with
its original command value, so as not to impact trace applications.
Also fix one-shot timers that were not reliably being marked as not
active:
- when they expired before the start command could be processed
- when the expiration was processed during the timer list switch
Also improve consistency:
- Always reload auto-reload timers *before* calling the callback.
- Always call traceTIMER_EXPIRED() just prior to the callback.
* fix indent
* Revert unnecessary change to prvTimerTask()
Change was intended to faithfully work through a backlog that spanned a
list switch, and before processing a stop or delete command. But, (1)
it isn't important to do that, and (2) the code didn't accomplish the
intention when *two* auto-reload timers were backlogged across a list
switch. Best to simply leave this part of the code as it was before.
* fix style
Co-authored-by: Joseph Julicher <jjulicher@mac.com>
diff --git a/timers.c b/timers.c
index 203a077..8032a1f 100644
--- a/timers.c
+++ b/timers.c
@@ -55,7 +55,8 @@
#if ( configUSE_TIMERS == 1 )
/* Misc definitions. */
- #define tmrNO_DELAY ( TickType_t ) 0U
+ #define tmrNO_DELAY ( ( TickType_t ) 0U )
+ #define tmrMAX_TIME_BEFORE_OVERFLOW ( ( TickType_t ) -1 )
/* The name assigned to the timer service task. This can be overridden by
* defining trmTIMER_SERVICE_TASK_NAME in FreeRTOSConfig.h. */
@@ -173,6 +174,15 @@
const TickType_t xCommandTime ) PRIVILEGED_FUNCTION;
/*
+ * Reload the specified auto-reload timer. If the reloading is backlogged,
+ * clear the backlog, calling the callback for each additional reload. When
+ * this function returns, the next expiry time is after xTimeNow.
+ */
+ static void prvReloadTimer( Timer_t * const pxTimer,
+ TickType_t xExpiredTime,
+ const TickType_t xTimeNow ) PRIVILEGED_FUNCTION;
+
+/*
* An active timer has reached its expire time. Reload the timer if it is an
* auto-reload timer, then call its callback.
*/
@@ -502,45 +512,47 @@
}
/*-----------------------------------------------------------*/
+ static void prvReloadTimer( Timer_t * const pxTimer,
+ TickType_t xExpiredTime,
+ const TickType_t xTimeNow )
+ {
+ /* Insert the timer into the appropriate list for the next expiry time.
+ * If the next expiry time has already passed, advance the expiry time,
+ * call the callback function, and try again. */
+ while ( prvInsertTimerInActiveList( pxTimer, ( xExpiredTime + pxTimer->xTimerPeriodInTicks ), xTimeNow, xExpiredTime ) != pdFALSE )
+ {
+ /* Advance the expiry time. */
+ xExpiredTime += pxTimer->xTimerPeriodInTicks;
+
+ /* Call the timer callback. */
+ traceTIMER_EXPIRED( pxTimer );
+ pxTimer->pxCallbackFunction( ( TimerHandle_t ) pxTimer );
+ }
+ }
+/*-----------------------------------------------------------*/
+
static void prvProcessExpiredTimer( const TickType_t xNextExpireTime,
const TickType_t xTimeNow )
{
- BaseType_t xResult;
Timer_t * const pxTimer = ( Timer_t * ) listGET_OWNER_OF_HEAD_ENTRY( pxCurrentTimerList ); /*lint !e9087 !e9079 void * is used as this macro is used with tasks and co-routines too. Alignment is known to be fine as the type of the pointer stored and retrieved is the same. */
/* Remove the timer from the list of active timers. A check has already
* been performed to ensure the list is not empty. */
-
( void ) uxListRemove( &( pxTimer->xTimerListItem ) );
- traceTIMER_EXPIRED( pxTimer );
/* If the timer is an auto-reload timer then calculate the next
* expiry time and re-insert the timer in the list of active timers. */
if( ( pxTimer->ucStatus & tmrSTATUS_IS_AUTORELOAD ) != 0 )
{
- /* The timer is inserted into a list using a time relative to anything
- * other than the current time. It will therefore be inserted into the
- * correct list relative to the time this task thinks it is now. */
- if( prvInsertTimerInActiveList( pxTimer, ( xNextExpireTime + pxTimer->xTimerPeriodInTicks ), xTimeNow, xNextExpireTime ) != pdFALSE )
- {
- /* The timer expired before it was added to the active timer
- * list. Reload it now. */
- xResult = xTimerGenericCommand( pxTimer, tmrCOMMAND_START_DONT_TRACE, xNextExpireTime, NULL, tmrNO_DELAY );
- configASSERT( xResult );
- ( void ) xResult;
- }
- else
- {
- mtCOVERAGE_TEST_MARKER();
- }
+ prvReloadTimer( pxTimer, xNextExpireTime, xTimeNow );
}
else
{
pxTimer->ucStatus &= ~tmrSTATUS_IS_ACTIVE;
- mtCOVERAGE_TEST_MARKER();
}
/* Call the timer callback. */
+ traceTIMER_EXPIRED( pxTimer );
pxTimer->pxCallbackFunction( ( TimerHandle_t ) pxTimer );
}
/*-----------------------------------------------------------*/
@@ -741,7 +753,7 @@
{
DaemonTaskMessage_t xMessage;
Timer_t * pxTimer;
- BaseType_t xTimerListsWereSwitched, xResult;
+ BaseType_t xTimerListsWereSwitched;
TickType_t xTimeNow;
while( xQueueReceive( xTimerQueue, &xMessage, tmrNO_DELAY ) != pdFAIL ) /*lint !e603 xMessage does not have to be initialised as it is passed out, not in, and it is not used unless xQueueReceive() returns pdTRUE. */
@@ -802,7 +814,6 @@
case tmrCOMMAND_START_FROM_ISR:
case tmrCOMMAND_RESET:
case tmrCOMMAND_RESET_FROM_ISR:
- case tmrCOMMAND_START_DONT_TRACE:
/* Start or restart a timer. */
pxTimer->ucStatus |= tmrSTATUS_IS_ACTIVE;
@@ -810,19 +821,18 @@
{
/* The timer expired before it was added to the active
* timer list. Process it now. */
- pxTimer->pxCallbackFunction( ( TimerHandle_t ) pxTimer );
- traceTIMER_EXPIRED( pxTimer );
-
if( ( pxTimer->ucStatus & tmrSTATUS_IS_AUTORELOAD ) != 0 )
{
- xResult = xTimerGenericCommand( pxTimer, tmrCOMMAND_START_DONT_TRACE, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, NULL, tmrNO_DELAY );
- configASSERT( xResult );
- ( void ) xResult;
+ prvReloadTimer( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow );
}
else
{
- mtCOVERAGE_TEST_MARKER();
+ pxTimer->ucStatus &= ~tmrSTATUS_IS_ACTIVE;
}
+
+ /* Call the timer callback. */
+ traceTIMER_EXPIRED( pxTimer );
+ pxTimer->pxCallbackFunction( ( TimerHandle_t ) pxTimer );
}
else
{
@@ -889,10 +899,8 @@
static void prvSwitchTimerLists( void )
{
- TickType_t xNextExpireTime, xReloadTime;
+ TickType_t xNextExpireTime;
List_t * pxTemp;
- Timer_t * pxTimer;
- BaseType_t xResult;
/* The tick count has overflowed. The timer lists must be switched.
* If there are any timers still referenced from the current timer list
@@ -902,43 +910,10 @@
{
xNextExpireTime = listGET_ITEM_VALUE_OF_HEAD_ENTRY( pxCurrentTimerList );
- /* Remove the timer from the list. */
- pxTimer = ( Timer_t * ) listGET_OWNER_OF_HEAD_ENTRY( pxCurrentTimerList ); /*lint !e9087 !e9079 void * is used as this macro is used with tasks and co-routines too. Alignment is known to be fine as the type of the pointer stored and retrieved is the same. */
- ( void ) uxListRemove( &( pxTimer->xTimerListItem ) );
- traceTIMER_EXPIRED( pxTimer );
-
- /* Execute its callback, then send a command to restart the timer if
- * it is an auto-reload timer. It cannot be restarted here as the lists
- * have not yet been switched. */
- pxTimer->pxCallbackFunction( ( TimerHandle_t ) pxTimer );
-
- if( ( pxTimer->ucStatus & tmrSTATUS_IS_AUTORELOAD ) != 0 )
- {
- /* Calculate the reload value, and if the reload value results in
- * the timer going into the same timer list then it has already expired
- * and the timer should be re-inserted into the current list so it is
- * processed again within this loop. Otherwise a command should be sent
- * to restart the timer to ensure it is only inserted into a list after
- * the lists have been swapped. */
- xReloadTime = ( xNextExpireTime + pxTimer->xTimerPeriodInTicks );
-
- if( xReloadTime > xNextExpireTime )
- {
- listSET_LIST_ITEM_VALUE( &( pxTimer->xTimerListItem ), xReloadTime );
- listSET_LIST_ITEM_OWNER( &( pxTimer->xTimerListItem ), pxTimer );
- vListInsert( pxCurrentTimerList, &( pxTimer->xTimerListItem ) );
- }
- else
- {
- xResult = xTimerGenericCommand( pxTimer, tmrCOMMAND_START_DONT_TRACE, xNextExpireTime, NULL, tmrNO_DELAY );
- configASSERT( xResult );
- ( void ) xResult;
- }
- }
- else
- {
- mtCOVERAGE_TEST_MARKER();
- }
+ /* Process the expired timer. For auto-reload timers, be careful to
+ * process only expirations that occur on the current list. Further
+ * expirations must wait until after the lists are switched. */
+ prvProcessExpiredTimer( xNextExpireTime, tmrMAX_TIME_BEFORE_OVERFLOW );
}
pxTemp = pxCurrentTimerList;
