Semaphore hang

[mwilt-rapidmicrobio]

At the end of StartTaskModbusMaster, it only releases the semaphore in the no error case. Releasing the semaphore unconditionally before falling out makes it much less likely to hang. There is still some issue causing this task to hang in a busy environment so I will keep digging.

[pumicestone]

I did the following to prevent the Modbus task from freezing when some device decides not to respond. Maybe that one could be useful for you.

@@ -998,9 +998,9 @@ void StartTaskModbusMaster(void *argument) {
 
 		// This is the case for implementations with only USART support
 		SendQuery(modH, telegram);
 		/* Block indefinitely until a Modbus Frame arrives or query timeouts*/
-		ulNotificationValue = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+		ulNotificationValue = ulTaskNotifyTake(pdTRUE, 100 / portTICK_PERIOD_MS);

[mwilt-rapidmicrobio]

The timeout timer is initiated in SendTxBuffer, using the timeout value in ticks that was configured when the modbus object was created. This timer sends a message that unblocks ulTaskNotifyTake if it expires. It sends a notification value of ERR_TIME_OUT. The problem comes later in StartTaskModbusMaster, toward the end. In the timeout case, modH->i8lastError is not zero (it is ERR_TIME_OUT), so it does not release the semaphore it took earlier.

The system I am developing runs a TCP slave and four RS-485 masters. The TCP interface is polled every few milliseconds. The masters are polling their devices every few milliseconds. I added a mutex to protect the master's register store. The master will take it briefly when it is accessing the register store and the slaves take it briefly when they access the register store. Once the system gets busy the master task will hang. Moving xSemaphoreGive outside the conditional at the end of the master task makes it work much better, perhaps an hour rather than a minute. There is still something else going on that makes the master task hang so I am still digging.

[alejoseb]

I was reading the code and I think the semaphore is not taken at all if there is a timeout or any other error. So the condition that checks if there are no errors to release the semaphore is not needed. It will always be executed (released) and the MCU should not hang there. If your code is hanging the error could be in a different part. There is another issue reporting that if one slave fails then all the slaves fail.

[mwilt-rapidmicrobio]

I agree it looks like the error cases will hit a continue statement and skip around the line that takes the semaphore.

New news today- if I don't create and start the other three serial port masters I don't get this hang. It ran fine for at least 20 hours.

If there is a situations where slaves affect each other, the same issue might be giving me trouble with these multiple masters. This issue comes up when I create and start all four masters. I only have traffic on one of them, but the mere presence of the others appears to facilitate this problem. It is also related to the ethernet interface being really busy. If anything takes more than a few milliseconds extra it makes sense in this application to simply wait for the next refresh. I am setting T35 to 5 ticks and u16timeOut to 20 ticks.

[mwilt-rapidmicrobio]

I added some tracing so now I know it is hanging in the call to ulTaskNotifyTake after calling SendQuery in the master thread. It should have gotten a notification when either a packet came in or the timeout timer started in SendQuery expired. I will try the previous suggestion and pass a timeout value to ulTaskNotifyTake. This should not be necessary since the timeout timer is supposed to handle this situation.

[alejoseb]

You are right it should not be necessary, unless we have an inverted priority issue or we are running out of freeRTOS heap.

[mwilt-rapidmicrobio]

I changed the call to ulTaskNotifyTake to use the timeout specified when the bus was set up. Here is what it looks like with a bit of context:

 else // send a query for USART and USB_CDC
 {
   SendQuery(modH, telegram);
   /* Block until a Modbus Frame arrives or query timeouts*/
   ulNotificationValue = ulTaskNotifyTake(pdTRUE, modH->u16timeOut);

This works much better- it got through several days of constant abuse.

When it did fail, I got a new failure mode. It appeared that the uart interrupt was going off constantly, not allowing anything else to happen. Since it takes several days for this failure mode to occur, I am hoping that some analysis will lead to a solution.

Stepping through the interrupt code, I see it test and clear several conditions. There are a whole bunch of bits set in isrflags. When the interrupt handler exits, it immediately gets called again. Could this be as simple as clearing a flag that wasn't anticipated?

[mwilt-rapidmicrobio]

Using STMCubeIDE, project manager tab, I turned on "Register CallBack" for "UART". I can see in the code that it does a lot of stuff differently when this is turned on. There is a comment about this mechanism "used to override the weak predefined callback". The Modbus code that interacts with this is unchanged. This appears to fix the issue with the interrupt going off constantly. I can't imagine why.
So, the only change I made to the library is to pass the timeout to ulTaskNotifyTake as described above rather than relying on the timeout timer. I would like to do some further testing to see what exactly is going wrong with the timeout timer.

[alejoseb]

Using the timeout value instead of the timeout timer is a valid option, but it is diverging a bit from the standard MODBUS specification. In any case, if you see that the USART interrupt is going off continuously and you are not using DMA, I think you are observing a USART error interrupt. After the USART error interrupt is triggered I have observed that USART does not recover again if the interrupt is not properly serviced. So the Modbus library will hang, because the actual USART peripheral stays in an error condition.

I guess, that is the reason when you enable the callback it is sort of working, because cubeIDE should add the code to handle the error interrupt graciously.

Being said that, you can see that the USART callback.c file contains an error callback handler but only for the DMA mode. I think it is worth it to implement an error handler callback for the standard mode (no DMA). In this error handler, the USART should be totally reset and configured again according to the initial configuration process. Maybe cleaning the ring buffer as well can help.