Serial Port Callback Stops Responding

[DieselPower]

I have a PXI system running Pharlap.  I have an 16 port serial card PXI-8430.  This is one of 12 systems with very similar configuration.

One of those systems randomly loses connection to one particular serial device, but I am communicating with three of these devices on three different ports.  I am also communicating to two other different devices on different ports and those devices do not lose connection.  The ports that disconnect are no adjacent.  Why would those ports stop responding to the callback? 

I have installed a serial port sniffer and I can see the request and response in both directions so I know the traffic is there.  The callback has simply stopped calling back.

The only way to fix it is to cycle PXI power.  Disconnecting from it and killing the callback is not sufficient.

Is it necessary to destroy a ComCallback when one is Installed? 

int Timer_Port_Open(int CurrentTimer)
{
	// Open and configure the port
	HWTimers[CurrentTimer].gBaud		= 38400;
	HWTimers[CurrentTimer].gParity	= 0;	// no parity checking
	HWTimers[CurrentTimer].gDataBits	= 8;	// 8-bit characters
	HWTimers[CurrentTimer].gStopBits	= 1;	// 1 stop bit
	HWTimers[CurrentTimer].gInQSize	= 512;	// 512-byte input queue
	HWTimers[CurrentTimer].gOutQSize	= -1;	// disable the output queue (synchronous writes)	
	
	// Initialize members
	HWTimers[CurrentTimer].TimerValue	= 0;	//

	if (OpenComConfig (HWTimers[CurrentTimer].gPortNum, NULL, HWTimers[CurrentTimer].gBaud, HWTimers[CurrentTimer].gParity, HWTimers[CurrentTimer].gDataBits, HWTimers[CurrentTimer].gStopBits, HWTimers[CurrentTimer].gInQSize, HWTimers[CurrentTimer].gOutQSize) < 0)
		return -1;
	
	InstallComCallback (HWTimers[CurrentTimer].gPortNum, LWRS_RXFLAG, -1, TERM_CHAR, Timer_Message_Received, NULL);
	
	Timer_Port_SendMessage(CurrentTimer);
	
	return 0;
}

extern void CVICALLBACK Timer_Message_Received (int COMPort, int eventMask, void *callbackData)
{
	static char receivedMsg[64];
	float	TimerValue = 0;
	int TempPort = -1;
	
	do
	{
		TempPort++;
	} while (HWTimers[TempPort].gPortNum != COMPort);
	
	if (ComRdTerm (HWTimers[TempPort].gPortNum, receivedMsg, sizeof(receivedMsg) - 1, TERM_CHAR) > 0)
	{
		if (strlen(receivedMsg) < 20)
			strncpy(HWTimers[TempPort].msg, receivedMsg+10,39);

		//DebugPrintf("%s\n\n", receivedMsg);  
	}
		
	HWTimers[TempPort].TimerValue = atof(HWTimers[TempPort].msg);
	
	return;
}

[gdargaud]

I'm just going around the messages with no answers in the forum, and several things strike me in your code:

- neither ComReadTerm() nor strncpy() add a terminating ASCII NUL at the end of the buffer, so I'm pretty sure this code will crash randomly.

- you need to retrieve the value returned by ComReadTerm() for that.

- why do you skip the 1st 10 bytes of the reply (maybe it's useless wrapper) ?

- why do you have an infinite loop at the start of your callback ?

[DieselPower]

- neither ComReadTerm() nor strncpy() add a terminating ASCII NUL at the end of the buffer, so I'm pretty sure this code will crash randomly.

Generally I initialize strings.  In this case I did not, but I will add it.

- you need to retrieve the value returned by ComReadTerm() for that.

Are you saying I should retrieve the number of bytes received as a test case on that call rather than just making the call?

- why do you skip the 1st 10 bytes of the reply (maybe it's useless wrapper) ?

The message has a header and then a timer value.  I only need the timer value.

- why do you have an infinite loop at the start of your callback ?

It is not infinite.  It could be a for loop iterating through the number of timers I'm talking to, but this one works as well.  I agree it is likely bad programming and I'm not sure why I did it this way.

I ended up solving the problem by remaking the serial port whenever it would time out.  It is only one cell that this happens in, and only one type of serial device vs. the many different kinds I am using.

Thank you.

[gdargaud]

I should look more like:

if (HWTimers[TempPort].gPortNum != COMPort) return;
	
int Nb=ComRdTerm (HWTimers[TempPort].gPortNum, receivedMsg, sizeof(receivedMsg) - 1, TERM_CHAR)
if (Nb<=0) return; // And possibly error message
receivedMsg[Nb]='\0'; // Now it's a proper string !
if (Nb < 20 and Nb>10) {
 strncpy(HWTimers[TempPort].msg, receivedMsg+10,39);
 HWTimers[TempPort].msg[39-10]='\0'; // Must be terminated manually
}

[DieselPower]

I am going to revisit this because it is becoming an issue again.  Not sure why it laid low for a while.

I have since changed the code to what you see below.

Both strings are initialized so that I'll always have a terminating character.

I am using one callback for multiple serial ports since the callback returns the COM port ID.  The problem is that when one timer quits they all quit.  I have added some debugging code in here, but I haven't been able to catch the issue yet to debug it.  I have some code that will close and reopen the serial port if I don't get a response in 5 seconds, but that isn't enough to make the CALLBACK function start to sense messages again.  I added the FlushInQ to the code as well, but again, I haven't had the issue happen.

My break and make code closes the COM port and reopens it.  Why would it still fail to sense incoming data?  I have verified two-way traffic with a serial sniffer (EZ Tap).

extern void CVICALLBACK Timer_Message_Received (int COMPort, int eventMask, void *callbackData)
{
	static char receivedMsg[64] = {'\0'};
	static char TerminalMessage[64] = {'\0'};
	float	TimerValue = 0;
	int TempPort = -1;
	int response = 0;
	
	do
	{
		TempPort++;
	} while (HWTimers[TempPort].gPortNum != COMPort);
	
	response = ComRdTerm (HWTimers[TempPort].gPortNum, receivedMsg, sizeof(receivedMsg) - 1, TERM_CHAR);
	
	if (response > 0)
	{
		if (strlen(receivedMsg) < 20)
			strncpy(HWTimers[TempPort].msg, receivedMsg+10,39);
		else
		{
			snprintf(TerminalMessage, 64, "%s message: %s.  Flushing receive buffer.", HWTimers[TempPort].Name, receivedMsg);
			Terminal_Writeline(TerminalMessage);
			FlushInQ(HWTimers[TempPort].gPortNum);
		}
	}
		
	if ((debugIndex - 100) == HWTimers[TempPort].gPortNum)
	{
		snprintf(TerminalMessage, 64, "%s|%d|%d|%s", HWTimers[TempPort].Name, HWTimers[TempPort].TimeSinceResponse, response, receivedMsg);
		Terminal_Writeline(TerminalMessage);
	}
	
	HWTimers[TempPort].TimerValue = atof(HWTimers[TempPort].msg);
	HWTimers[TempPort].TimeSinceResponse = 0;
	
	return;
}

[RobertoBozzolo]

1. Is there a typo that you react on the string being LESS than 20 characters and strncpy up to 49?

2. It's not clear if you already do it, but since CloseCom automatically discards the callback when you open the port again you must install the callback as well

[DieselPower]

I'm not sure why I made the array hold 40 characters when the message is shorter, unless it was fear if concatenated messages overrunning it.

Here is code to catch a timer that isn't responding and reconnect:

				for (int count = 0; count < numTimers; count++)	   
				{
				   	if (HWTimers[count].TimeSinceResponse > 5)
					{
						char TerminalMessage[100] = {'\0'};
						sprintf(TerminalMessage,"%s Hardware Timer Frozen.  Reconnecting.", HWTimers[count].Name);
						Terminal_Writeline(TerminalMessage);
						FlushInQ(HWTimers[count].gPortNum);
						Timer_Port_Close(count);
						SleepUS(10000);
						Timer_Port_Open(count);
					}
				}	

Here is the port open code:

int Timer_Port_Open(int CurrentTimer)
{
	// Open and configure the port
	HWTimers[CurrentTimer].gBaud		= 38400;
	HWTimers[CurrentTimer].gParity	= 0;	// no parity checking
	HWTimers[CurrentTimer].gDataBits	= 8;	// 8-bit characters
	HWTimers[CurrentTimer].gStopBits	= 1;	// 1 stop bit
	HWTimers[CurrentTimer].gInQSize	= 512;	// 512-byte input queue
	HWTimers[CurrentTimer].gOutQSize	= -1;	// disable the output queue (synchronous writes)	
	HWTimers[CurrentTimer].TimeSinceResponse = 0;
	
	// Initialize members
	HWTimers[CurrentTimer].TimerValue	= 0;	//

	if (OpenComConfig (HWTimers[CurrentTimer].gPortNum, NULL, HWTimers[CurrentTimer].gBaud, HWTimers[CurrentTimer].gParity, HWTimers[CurrentTimer].gDataBits, HWTimers[CurrentTimer].gStopBits, HWTimers[CurrentTimer].gInQSize, HWTimers[CurrentTimer].gOutQSize) < 0)
		return -1;
	
	InstallComCallback (HWTimers[CurrentTimer].gPortNum, LWRS_RXFLAG, -1, TERM_CHAR, Timer_Message_Received, NULL);
	
	Timer_Port_SendMessage(CurrentTimer);
	
	return 0;
}

All of these timers (up to 4) call the same function to read the data.  The COM port ID on the callback helps me identify them.  One thing I just realized is that, when this happened yesterday, I was not getting a message that it needed to reconnect.  That means the callback was firing, but it wasn't getting to the section that extracts the time from the message.  I'm wondering of the messages overran and then it couldn't make sense of them.  I have since added the diagnostic message and the flush if the message is > 20 in length.  Not sure whats going to happen if the message is 19, but the debug message will allow me to see what the message looks like in the event that the timer value stops updating.

[RobertoBozzolo]

I suppose I wasn't clear enough, but here there is an evident problem in the handling of the received message, since you read 40 characters only if you have less than 20 in the buffer:

if (strlen(receivedMsg) < 20)
	strncpy(HWTimers[TempPort].msg, receivedMsg+10,39);
else

But since your code is working I suppose this is a simple typo.

I personally would add some error checking in the com callback, trying to trap timeout errors (rs232err variable) that could result in incomplete readings from the queue and to see if there are parts of messages not downloaded in the first read.

Additionally, have you considered setting a proper timeout value instead of the default 5-sec time? Can there be a situation in which some device responds lately or does not respond at all that could result in a problem in code?

[DieselPower]

 I have changed 39 to 20 just to be consistent.  

I also added the following:

	else if (response < 0)
	{
		snprintf(TerminalMessage, 1024, "%s error[%d]: %s", HWTimers[TempPort].Name, response, GetRS232ErrorString(response));
		Terminal_Writeline(TerminalMessage);	
	}

Also changed the timeout:

	SetComTime(HWTimers[CurrentTimer].gPortNum, 0.5);