FreeRTOS Timer Hard Fault Stm32

Kernel
,
1

Hello Dear Community,
Recently I’ve faced with an unpredictable issue.
I was working on my project with a few tasks, queues and timers and caught a happy HARD FAULT!!!
I’ve spend days to analyze what could be wrong with it by checking all the stack of my code and blaming myself for everything. I’ve read a lot of forums with many suggestion of increasing the task’s stack memory size, playing with priorities, increasing queue’s max element number, configuring debug parameters as theradX, freertos queue and timers, setting up the ASM values for reading them under debug, checking the queue elements and many other tricks just to understand the problem.
Long after I just found out that when I don’t run a timer it doesn’t break down. So my story went for the next round not least.
Three days were passed and all I knew was that I had the right timer handlers, working timers and nothing in the docs about this wonderful behavior.
You couldn’t imagine how my expectations were exceeded when I saw that if the timer hasn’t a callback it jumps to the happy hard fault without many words (they don’t check it for the NULL before they call it).

So, I have a proposal for the great dev’s team. I just wonder, why wouldn’t you just check it for the NULL before you call it OR return an invalid handler (if it affects for some ideological principles)?

Regards

2

Are you talking about the FreeRTOS software timer? If yes, do you mean to say that you called xTimerCreate with pxCallbackFunction parameter as NULL - FreeRTOS-Kernel/timers.c at main · FreeRTOS/FreeRTOS-Kernel · GitHub? If yes, then we can add an assert here to catch it -

configASSERT( ( pxCallbackFunction != NULL ) );
3

Sure, this is the right way you are thinking about, however why don’t you check it for the Null and if so, not to run the callback? Are there any obstacles preventing you from doing it?

Imagine I want to run the timer just to check if it’s active or not. You have the function
xTimerIsTimerActive(…)
and nothing stops me to set the Null for the callback parameter. Moreover I get the working handler after all. Suddenly something jumps to the Hard Fault. And it’s doubly fun, because you don’t have to know when the timer will go for it at runtime.

So why run the callback at all costs?
This is wrong behavior. It must be handled if you have good reasons for breaking it down, but the callback shouldn’t be necessary for the timer’s work by all means.

4

Why so aggressive?

Adding a 0 Check at runtime before invocation would mean additional runtime footprint, ie burnt cycles and mostly never used code for exotic use cases. FreeRTOS is designed scalable, you can benefit from it on platforms that have well less than 128k flash memory. To keep supporting these platforms, reducing footprint is an issue.

5

Why exotic use cases?

The way you go with a callback to handle an event isn’t right way for all cases. It may cause you to follow for all callbacks in a code to understand a chain of timers and their dependency. What would you do if some of them should be stopped before it finished - you will handle it in some callback of some timer. As a result you will get the hard readable spaghetti code.
The good practice is to place this logic at the same place. Therefore you may not need to call the callback at all. Isn’t it a point you’d like to achieve at run time?
There can be many ways to get it: adding one more function, check it once at the xTimerCreate(…).
Moreover, It’s already have many ptr-checks at runtime when the timer job is inserted to the queue. I was going through this during the investigation and you could also see this.

6

you are welcome to submit a PR to the FreeRTOS Github repo if you believe that there is code to be improved.

7

The idea of not using the callback, but just “polling” the active flag tends to lead to bad code, and you probably never really needed the timer in the first place, but should just be reading the system time to see if too much time has expired.

I think most of the other checks are all in “asserts”, which can go away in production code for efficiency. I suppose an assert could have been put on the callback pointer, but that still wouldn’t achieve what you wanted, it would have just shown the error sooner.

8

It doesn’t lead to bad code.
The idea of timer is not straight about the callback. Here you may use the callback as an event of the expired time ticks. The event can be as well as the flag checking.
On the other hand if you go to say that it’s better to check it by time difference, how would you name an entity that checks the time ticks. I suppose it’s exactly the timer.

9

That’s would be my pleasure.

10

I proposed assert because it seems like a programmer error which once fixed, wont happen at runtime.

Can you elaborate this with the example code snippet? I am interested to see a usecase for a timer without callback.

11

Easy peasy. Let’s say you need to open a door for some time period and close it after. You can open it, start a timer and close it in the callback. Seems clear.
But what if you have a lot of doors and they should depend on each other to be closed. Now you cannot just close a door in a callback. You should annalize system’s states. If you organize this handling in the callbacks, the logic can be shared with all these callbacks or you will do the flag checking in some callback as well. Besides you’d have to keep in your mind all states of the doors during the debug.
To write clean code you’d like to organize it in one controller. Thus there will be only flag checking.
In that case, you don’t need to create a stub just to run a timer and set a flag in its callback. Therefore you already have the function in the library called xTimerIsActive…

12

Can we not use vTaskSetTimeOutState and xTaskCheckForTimeOut for this - RTOS - xTaskCheckForTimeOut()? Would you please be kind enough to write small pseudo code (just for 2 doors) so that we can see if the same can be achieved in some other way.

13

Here an example of the control task:

typedef enum {
	kRequest1 = 0,
	kRequest2,
	kRequestLast
} Request;

TimerHandle_t xTimers[2];

void DoThis(/* ... */);
void DoThat(/* ... */);

void vTaskUnitControl(void * pvParameters);	

//------------------------------------------------------

void DoThis(/* ... */){
	if (xTimerIsTimerActive(xTimers[0]) == pdFALSE) { 				
		if (xTimerIsTimerActive(xTimers[1]) == pdFALSE) { 
			// No need to wait for the callback					
			SendResponse();
		}
		else {
			// do something else
		}
	}
	else if 
	{
		SendResponse();
		// do something
	}
}

void DoThat(/* ... */) {
	// do before
	if (xTimerStart(xTimers[1], 10) != pdFALSE) {
		// do something	
	}
	// do after
}	

void vTaskUnitControl(void * pvParameters)
{
	TimerHandle_t xTimers[2];
	xTimers[0] = xTimerCreate("Unit1", 5000, pdFALSE, NULL, NULL);
	xTimers[1] = xTimerCreate("Unit2", 1000, pdFALSE, NULL, NULL);

    for( ;; )
    {
		Request request = GetRequest();	
		
		switch(request){
		case kRequest1: {			
			DoThis(/* ... */);						
			break;
		}
		case kRequest2: {
			DoThat(/* ... */);	
			break;
		}
		default: 
			break;
		}		    
    }
}
14

I think the point is that the call to xTimerIxTimerActive (and the related code) could have been replaced with just storing the Tick Count when you wanted to start the timer, and then checking if more than the timer period had passed when you check.

No need for a “Timer” at all. If time equal to the rollover time of the tick counter might be happening, then the xTaskCheckForTimeOut can be used.

2 Likes
15

Here is the modified code for what @richard-damon described:

typedef enum {
	kRequest1 = 0,
	kRequest2,
	kRequestLast
} Request;

TimeOut_t xTimeOutState[2];
TickType_t xTicksToWait[2] = {5000, 1000};

void DoThis(/* ... */);
void DoThat(/* ... */);

void vTaskUnitControl(void * pvParameters);	

//------------------------------------------------------ 

void DoThis(/* ... */){
	if (xTaskCheckForTimeOut( &( xTimeOutState[0] ), &( xTicksToWait[ 0 ] ) ) == pdFALSE) { 				
		if (xTaskCheckForTimeOut( &( xTimeOutState[1] ), &( xTicksToWait[ 1 ] ) ) == pdFALSE) { 
			// No need to wait for the callback					
			SendResponse();
		}
		else {
			// do something else
		}
	}
	else if 
	{
		SendResponse();
		// do something
	}
}

void DoThat(/* ... */) {
	// do before
  vTaskSetTimeOutState( &( xTimeOutState[ 0 ] ) );
	// do after
}	

void vTaskUnitControl(void * pvParameters)
{
    for( ;; )
    {
		Request request = GetRequest();	
		
		switch(request){
		case kRequest1: {			
			DoThis(/* ... */);						
			break;
		}
		case kRequest2: {
			DoThat(/* ... */);	
			break;
		}
		default: 
			break;
		}		    
    }
}
16

Thank you Aggard for your concern.
I see your point and I’m aware of that. Of course it can be managed this way as well.
The name “vTaskSetTimeOutState(…)” says mostly about operations with a task and the function has some tricks of its Blocked state to measure the time properly. It’s not critical for many cases, but it has to be handled otherwise.
As the documentation says that it “is intended for advanced users only” at first sight the timer’s functions are more attractive and less confusing to work with.
In this case I’d propose to add some new entity and name it xTime() for any purposes in the user’s code with no obligations as the xTimer() has.

17

Have you considered using a timer with an “empty” callback function? Something like this:

static void emptyTimerCallback( TimerHandle_t timer )
{
   UNUSED(timer);
}
18

Do you mean something like xTaskGetTickCount?