Click here to Skip to main content
15,888,803 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
I am developing an application that reads a fixed packet of data from a com port at regular intervals of 50ms. I am using CreateFile and ReadFile to open and read from the port respectively. There is a tiny amount of processing that happens with the data that is read.

Because timing is vital in such an application, I decided to check how long each of the separate tasks was taking. The ReadFile function was called at every 50ms or whenever the EV_RXCHAR event happened. The processing takes hardly a millisecond. This is all good. But the ReadFile function takes on an average 15ms to return. This seems like an awful lot of time to read data from a port. I am sending data to the port through a third-party application that writes data as packets, so it can't (I think) be because of its write speed.

This becomes an issue because of the following: I did the above tests with virtual com ports. When I test it with some physical hardware, the time taken can vary from 47ms to 127ms. The hardware is a card designed to package data and send them as packets. This amount of delay causes my application to go out of sync pretty quickly.

So any suggestions as to how I can speed this up, or maybe try a different strategy would be highly appreciated.

PS: I am doing non-overlapped read and write on purpose.

What I have tried:

I don't know what to add here, so I am adding my read function code. It is in an infinite loop until instructed otherwise.

C++
void CGCUGUIDlg::fnRead()
{
	
	while(m_bCount==true)
	{
		
		{
			
			WaitCommEvent(m_hComm,&dwEventMask,NULL);
			if(dwEventMask==EV_RXCHAR)
			{
				DWORD NoBytesRead;
				BYTE  abBuffer[100];
				if(ReadFile((m_hComm),&abBuffer,sizeof(abBuffer),&NoBytesRead,0)) //This takes over 47ms with physical hardware
				{
					midTime1 = GetTickCount();
					if(NoBytesRead==45)
					{
						
if(abBuffer[0]==0x10&&abBuffer[1]==0x10||abBuffer[0]==0x80&&abBuffer[1]==0x80)
						{
							this->fnSetData(abBuffer);
						}
						else
						{
							CString value;							
							value.Append("Header match failed");
							this->SetDlgItemText(IDC_RXRAW,value);	
						}
					}
					else if (NoBytesRead<45 && NoBytesRead>0)
					{
						CString value;
						value.Append(LPCTSTR(abBuffer),NoBytesRead);
						value.Append("\r\nInvalid Packet Size");
						this->SetDlgItemText(IDC_RXRAW,value);
					}
					
				}
				else
				{
					DWORD dwError2 = GetLastError();
					CString error2;
					error2.Format(_T("%d"),dwError2);
					this->SetDlgItemText(IDC_RXRAW,error2);
				}
				
			}
		}
		
	}
	CloseHandle(m_hComm);
		
	
}


PS: I apologise for the horrible formatting and random brackets.
Posted
Updated 13-Sep-18 1:05am
v3
Comments
PIEBALDconsult 14-Aug-17 1:01am    
This won't help much, but I think you should instantiate the array -- BYTE abBuffer[100] -- once, before entering the loop rather than instantiating it on each cycle of the loop.
Other than that, what I have done in the past is to have the reading and processing performed in separate threads.
I.e. separate the concerns.
I always hear my junior high school football coach's mantra, "get it, get rid of it; get it, get rid of it..."
TheLostJedi 14-Aug-17 1:50am    
Thank you for that reply!! The reason I create a new instance of the buffer each time is to ensure I have an empty buffer every time, and also it's kind of neat that the previous buffer gets cleaned up automatically once the scope ends. Also, this read is happening in a separate worker thread and because processing can happen only after the read is done I didn't think too much about allocating separate resources for the processing. But the main issue is that I can time and see that the ReadFile is taking a massive amount of time. Just that one line. The total time of each loop is actually the ReadFile time plus 1 or 2ms. I realise that these are inbuilt API calls, but I was hoping there was a way to speed it up.
Kornfeld Eliyahu Peter 14-Aug-17 1:41am    
If synchronization is important, you can not set a fixed time interval, only use events to communicate...
Com ports are built on UART unit, that can work with different speed (baudrate) to fit to the device connected...
You didn't show how you opened the port, but you may try a different baudrate setting...
TheLostJedi 14-Aug-17 1:50am    
Thank you for the reply!! I am using an event based read precisely for the reason you mentioned; synchronisation is near impossible with timed reads not in the least because of inaccurate timers.
I apologise for not showing how the port was opened. It was set to a baud-rate of 115200 bps and, unfortunately, that cannot be changed and is the same for the device sending the data. But I assume this would put a finite lower limit on the time each read takes right?
Kornfeld Eliyahu Peter 14-Aug-17 2:09am    
You wrote: "reads a fixed packet of data from a com port at regular intervals of 50ms"... It does not sound like even based :-)
115200 is a good speed generally. Passing 100 bytes at that speed should take about 10ms, so it sounds good...
Two things:
1. Why do you close the com port at the end of the function?
2. Do you have a port monitoring tool to see what actually goes and comes?

1 solution

You are calling ReadFile() with nNumberOfBytesToRead = sizeof(abBuffer). So the function will return after 100 bytes has been received which took some time.

You might get the number of available bytes instead and pass that (when EV_RXCHAR has been signaled):
DWORD dwComErrors;
COMSTAT tComStat;
if (::ClearCommError(m_hComm, &dwComErrors, &tComStat))
{
    // Call ReadFile() with nNumberOfBytesToRead = tComStat.cbInQue
}

For further optimisation you should also perform the reading within a worker thread using overlapped IO (with two OV structures for WaitCommEvent and ReadFile) call WaitForMultipleObjects to wait for COMM events and thread termination. Then reading from the serial port would not block your application. However, then you can not set GUI objects directly but must implement some kind of data passing (e.g. by posting user defined messages).

I would always use a worker thread. Then the GUI part of your application is not blocked even when ReadFile blocks due to waiting for more data than actually available. It would also avoid blocking your application if the device is disconnected.

Some useful links (quite old but still valid:
Serial Communications[^]
Serial Communication in Windows[^]
Serial Port IO[^]
 
Share this answer
 
Comments
TheLostJedi 14-Aug-17 3:52am    
Thank you for the reply!! I actually tried substituting the 100 bytes for exactly the size of the data packet being sent (fixed and constant at 45 bytes) but the time taken for the readfile function to return remained the same.
Also I am doing non-overlapped read, because I find it easy to organise read and write commands. And what you said about the worker thread is true, and that is why I put the read call in a thread.
I am beginning to think that maybe the delay is just inherent, but it's the fact that this delay doesn't occur on every read that makes me doubt that.
Jochen Arndt 14-Aug-17 4:32am    
When having a fixed packet size of 45 and performing the read within a worker thread there might no need to call WaitCommEvent. Just try to read the 45 bytes. To avoid sync problems increase the read buffer size by calling SetupComm() and optionally increase the thread priority to above normal (above the priority of normal applications). That should prevent out of sync (at least if there is no very high system load from low level IO like disk and network).

Your application might crash when calling SetDlgItemText() from threads that did not own the control. You might not have run into this so far because you are using that only upon errors.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900