Skip to content

Commit

Permalink
Remove support for tmrCOMMAND_START_DONT_TRACE (#305)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@jefftenney @n9wxu
jefftenney and n9wxu authored Apr 13, 2021
1 parent a22b438 commit 05ded5b
Showing 1 changed file with 44 additions and 69 deletions.
113 changes: 44 additions & 69 deletions timers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -172,6 +173,15 @@
const TickType_t xTimeNow,
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.
Expand Down Expand Up @@ -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 );
}
/*-----------------------------------------------------------*/
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -802,27 +814,25 @@
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;

if( prvInsertTimerInActiveList( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow, xMessage.u.xTimerParameters.xMessageValue ) != pdFALSE )
{
/* 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
{
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down

0 comments on commit 05ded5b

Please sign in to comment.