Click here to Skip to main content
15,937,127 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
Dear fellow coders,

I am facing this problem with my program. The program is interfacing with another machine which is outputting serial data at 1Hz. My program is MFC based (a cpropertypage) and to handle the data, i have defined a thread (pRecvDataThread) to handle the reading of data from the serial port's buffer, and display it on my list control in the main thread.

I have separated GUI stuff like getting list control handles etc out of the thread as suggested in some tutorial. However my GUI still is hardly responsive. Heres the section of the code, does anyone know whats wrong?

C++
// on serial data arrival
LRESULT MyApp::OnDataArrived(WPARAM wParam,LPARAM lParam) 
{
	Sleep(40);
	const CSerialMFC::EEvent eEvent = CSerialMFC::EEvent(LOWORD(wParam));
	const CSerialMFC::EEvent eError = CSerialMFC::EEvent(HIWORD(wParam));

	switch(eEvent)
	{
	case CSerialMFC::EEventRecv:

		pRecvDataThread = AfxBeginThread(MyRecvThread, LPVOID(this), THREAD_PRIORITY_NORMAL);
		break;
	}

	return 0;
}

// my thread proc
UINT MyRecvThread(LPVOID pParam)
{

	MyApp *pTab = (MyApp*)pParam;
	WaitForSingleObject(pTab ->MutexRecv, INFINITE);
	DWORD dwBytesRead = 0;
	byte abBuffer[512];

	int errorInt = pTab->serialMFC.Read(abBuffer, sizeof(abBuffer), &dwBytesRead, 0, INFINITE);

	if(errorInt != ERROR_SUCCESS)
	{
		AfxMessageBox(_T("ERROR in reading serial port"));
		return 0;
	}

	char szTimestamp[128];
	SYSTEMTIME st;
	//GetSystemTime(&st);
	GetLocalTime(&st);
	sprintf_s(szTimestamp, 128, "%02d:%02d:%02d:%03d", st.wHour, st.wMinute, st.wSecond, st.wMilliseconds);
	

	pTab->ReadData(abBuffer, szTimestamp, numBytesRead);

	ReleaseMutex(MyApp->MutexRecv);
	return 0;
}
void MyApp::ReadData(byte bytesRead [], char* timestamp, int bytesread)
{
    // display the bytes in list control
}
Posted
Updated 23-Oct-11 22:50pm
v3

You should handle all the communication details in one thread (say the 'communicating' one) while, now, you're handling events in your main (GUI) thread.
Moreover (and probably the whole point here) you shouldn't lock the mutex and then perform a blocking read. You should instead, lock the mutex just to update the shared data once the blocking (or long timeout) read has already been performed.
 
Share this answer
 
Comments
Zsefv2 24-Oct-11 8:20am    
yeah the mutex was actually it was not there at the beginning... the shared data being the list control?
CPallini 24-Oct-11 8:23am    
The shared data should (probably) be just a buffer. The shared buffer may be used to update the List Control.
you're spinning up and dropping a thread per reception; that's very expensive, and your message pump is heavily involved in the reception

Start the receive thread up, and have it sleep on a mutex, which gets signaled by your OnDataArrived message handler

Or even better, have the receive thread use WaitCommEvent, which will divorce it completely from the message pump
 
Share this answer
 
Comments
Zsefv2 24-Oct-11 8:22am    
hmmm, let me get you, so in the initialization phase i startup the receive thread then suspend it? so on data arrived i resume it?

what do u mean by letting the thread use WaitCommEvent? are there examples i can follow?
barneyman 24-Oct-11 18:55pm    
fundamentally, yes, spin up a thread, have it be responsible for the COM port (opening it etc during its initialisation) then use WaitCommEvent to sleep it until data comes in (http://www.codeproject.com/KB/system/serial_com.aspx) ... as others have said, the 'real' tricky bit is synching up the pumping of data back into your main thread, I'd be inclined to do it as a locked queue of pointers, so you're race exposure is limited to adding/removing *pointers* to memory
Zsefv2 26-Oct-11 22:11pm    
ok, i spin up a thread then suspend it, then when the event of data arrives on the com port, i resume the thread. is that it? then what if another set of data arrives on the com port? the thread would still be running?
Chuck O'Toole 26-Oct-11 22:49pm    
Using suspend and resume in this way does not guarantee proper synchronization. You have no idea how far the started thread gets before the parent thread suspends is, it could be far into reading data. Better to create an event and have the reader thread wait on that event for permission to advance to the 'read data' state.
Zsefv2 26-Oct-11 23:45pm    
can some kind soul put some code here so i can refer etc?
Something like this?

C++
unsigned long ReceiveThread(CVnTabs* pvnsTab)
{
    while(true)
    {
	if(!initialisePort)
	{
	    // Setup the serial port (9600,N81) using hardware handshaking
            initialisePort = true;
        }
        
        do
	{
		// do polling for data
                serial.Read(abBuffer,sizeof(abBuffer),&dwBytesRead);
		// if bytes more than 0
		if(dwBytesRead> 0)
		{
    		    // process data
		}

	}while(dwBytesRead == sizeof(abBuffer));

//::PostMessage ya-da ya-da
 
Share this answer
 
v2
By doing this inside of your independent thread:
pTab->ReadData(abBuffer, szTimestamp, numBytesRead);

and this is what you said you're doing in there:
C#
void MyApp::ReadData(byte bytesRead [], char* timestamp, int bytesread)
{
    // display the bytes in list control
}

You're still tying in your worker thread and your GUI (not independent). There's no separation, that's why your GUI gets hung up. Your worker thread should never access GUI components directly within it.
 
Share this answer
 
Comments
Zsefv2 27-Oct-11 22:31pm    
thanks for the reply, thats what i did to fix the problem. Totally removed the serial com operations from the GUI thread and put them on the thread I spun.

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