Confused about wxThreadHelper and how to use it

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
User avatar
Parduz
I live to help wx-kind
I live to help wx-kind
Posts: 188
Joined: Fri Jan 30, 2015 1:48 pm
Location: Bologna, Italy

Confused about wxThreadHelper and how to use it

Post by Parduz »

I'm trying to grasp the wxThreadHelper but i'm a bit confused, and i would ask a direction before starting banging my head.

What I'm trying to do is to create a custom component that act as a non blocking "communication device", wrapping a socket and a serial port and exposing a pair of methods to write and read on whatever device the user choose. I'm starting with the Serial port, right now. I already have my Serial class working (blocking).

This is where i am right now:
CommManager.h

Code: Select all

#ifndef COMMMANAGER_H
#define COMMMANAGER_H

#include <wx/wx.h>
#include <wx/event.h>

#include <string>
#include "Comm/BBBSerial.h"

#define		ph_rxBufferSize		2048
#define		ph_txBufferSize		512

typedef std::string String;

BEGIN_DECLARE_EVENT_TYPES()
    DECLARE_EVENT_TYPE(wxEVT_SERIAL, -1)
END_DECLARE_EVENT_TYPES()

class CommManager : public wxEvtHandler, public wxThreadHelper
{
	public:
		/** Default constructor */
		CommManager();
		/** Default destructor */
		virtual ~CommManager();

		// worker thread entry point
		wxThread::ExitCode Entry();

		// Seriale
		BBBSerial Serial;


		bool SendAndWait(const String& tx_Message, String& rx_Answer);
		bool SendMessage(const String& tx_Message);


		// Buffer TX & RX
		char*	rxBuffer;
		char*	txBuffer;

		String	Answer; // TODO: Make private and expose String GetAnswer() method;
	protected:

	private:

		unsigned int mCounter;	//
		wxCriticalSection mCS;
		void OnSerialRxCompleted(wxCommandEvent& SerialEvent);
};

#endif // COMMMANAGER_H
CommManager.cpp

Code: Select all

#include "CommManager.h"
//---------------------------------------------------------------------------------------------------------------------
DEFINE_EVENT_TYPE(wxEVT_MYTHREAD)
//---------------------------------------------------------------------------------------------------------------------



CommManager::CommManager()
{
	rxBuffer	= new char[2048];
	txBuffer	= new char[256];

	// Create thread
	CreateThread();

	Bind(wxEVT_MYTHREAD, &OnSerialRxCompleted, this);
}
//---------------------------------------------------------------------------------------------------------------------
CommManager::~CommManager()
{
}
//---------------------------------------------------------------------------------------------------------------------
bool CommManager::SendAndWait(const String& tx_Message, String& rx_Answer)
{
	// Non threaded Serial comunication
	unsigned int	txlen;
	unsigned int	rxlen;
	bool Result=false;
	int Ret;

	// String to char array
	txlen  = tx_Message.length()+1;
	strncpy(txBuffer, (const char*)tx_Message.c_str(), txlen);

	if (Serial.WriteString(txBuffer)) {
		// Send OK, wait for the answer
		rxlen = 0;
		Ret = Serial.ReadString(rxBuffer);
		if (Ret>0) {
			rxlen = Ret;
			// La serialib termina il buffer con lo 0, quindi non devo farlo io
			if (rxlen) {
				rx_Answer = rxBuffer;
				Result = true;
			}
		}else if (Ret==0) {
			// Timeout
			// TODO
		}else{
			// RX Error
			// TODO
		}
	}else{
		// some kind of error
		// TODO
	}
	return Result;
}
//---------------------------------------------------------------------------------------------------------------------
bool CommManager::SendMessage(const String& tx_Message)
{
	// Non threaded Serial comunication
	unsigned int	txlen;
	unsigned int	rxlen;
	bool Result=false;

	// String to char array
	txlen  = tx_Message.length()+1;
	strncpy(txBuffer, (const char*)tx_Message.c_str(), txlen);

	if (Serial.WriteString(txBuffer)) {
		// Send OK, start the thread for the answer
		rxlen = 0;
		GetThread()->Run();
		Result = true;
	}else{
		// some kind of error
		// TODO
	}
	return Result;
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wxThread::ExitCode CommManager::Entry()
{
	int Ret;
    wxCommandEvent evt(wxEVT_MYTHREAD);
	// loop, so long as we haven't been asked to quit
    while ( ! GetThread()->TestDestroy()) {
        {
			Ret = Serial.ReadString(rxBuffer);
			evt.SetInt(Ret);
			//evt.SetClientData(rxBuffer);
			wxPostEvent(this, evt);
        }
        //Sleep(100);
    }
    // we have been asked to stop
    return (wxThread::ExitCode)0;
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
void CommManager::OnSerialRxCompleted(wxCommandEvent& SerialEvent)
{

	int Ret = SerialEvent.GetInt();
	unsigned int	rxlen;

	if (Ret>0) {
		rxlen = Ret;
		// La serialib termina il buffer con lo 0, quindi non devo farlo io
		if (rxlen) {
			Answer = rxBuffer;
		}
	}else if (Ret==0) {
		// Timeout
		// TODO
	}else{
		// RX Error
		// TODO
	}

	// Queue the event on the main frame, to let the thread event call finish
	GetThread()->Pause();
    wxCommandEvent evt(wxEVT_SERIAL);
	evt.SetInt(rxlen);
	wxTheApp->GetTopWindow()->GetEventHandler()->QueueEvent(&evt);
}
My first problem is that the "Bind" line does'nt compile, and i don't understand why.

Then i have a lot of doubts:
- should i pause the thread in his own event?
- should i pause it or should i let it run using a flag to avoid to listen to the serial when not needed? my app is "master", the answering machine is "slave" and never send data if not asked for, and my app will COSTANTLY polls the serial device, as fast as it can.
- should i use a wxCriticalSection when copying the data from the buffer to the string?
- should i use a wxThread instead of a wxThreadHelper ?


- EDIT -
If this matters, my app should run on a Beaglebone Black with Debian, even if i'm developing on a Win7 machine.
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Confused about wxThreadHelper and how to use it

Post by doublemax »

My first problem is that the "Bind" line does'nt compile, and i don't understand why.
Try:

Code: Select all

 Bind(wxEVT_MYTHREAD, &CommManager::OnSerialRxCompleted, this);
should i pause the thread in his own event?
I think that would defy the purpose of using a thread.

But as you already suspected, the use of rxBuffer is not good right now, because you're already trying to read into the same buffer not knowing if the old content has already been processed in the main thread. You can either protect the buffer with a mutex or critical section or use a custom event and send the whole data inside the event.

You should also check how many bytes you actually receive each time. If you poll the serial device at maximum speed, i would suspect that you only very few bytes each time. And it would be quite an overhead to send an event for one byte. So maybe you should let the thread sleep for 100ms on each iteration.

Also it's not quite clear why you receive data in a thread, process the data in a main thread and then send another thread to the main window. Why don't you process the serial data directly inside the thread?
should i use a wxThread instead of a wxThreadHelper
wxThreadHelper doesn't really do that much (check its code in wx/include/thread.h). It just helps with creating and destroying the thread.
Use the source, Luke!
User avatar
Parduz
I live to help wx-kind
I live to help wx-kind
Posts: 188
Joined: Fri Jan 30, 2015 1:48 pm
Location: Bologna, Italy

Re: Confused about wxThreadHelper and how to use it

Post by Parduz »

doublemax wrote:
My first problem is that the "Bind" line does'nt compile, and i don't understand why.
Try:

Code: Select all

 Bind(wxEVT_MYTHREAD, &CommManager::OnSerialRxCompleted, this);
Nope.
This is the error:
C:\Source\Codeblocks\Progetti\DataGraphics\src\CommManager.cpp:18:62: required from here
C:\SVILUPPO\Toolchains\wxWidgets-3.0.3\include/wx/event.h:388:29: error: invalid conversion from 'wxEventFunctorMethod<int, CommManager, wxCommandEvent, CommManager>::EventClass* {aka wxEvent*}' to 'wxCommandEvent*' [-fpermissive]
CheckHandlerArgument(static_cast<EventClass *>(NULL));
~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\SVILUPPO\Toolchains\wxWidgets-3.0.3\include/wx/event.h:371:17: note: initializing argument 1 of 'static void wxEventFunctorMethod<EventTag, Class, EventArg, EventHandler>::CheckHandlerArgument(EventArg*) [with EventTag = int; Class = CommManager; EventArg = wxCommandEvent; EventHandler = CommManager]'
static void CheckHandlerArgument(EventArg *) { }
^~~~~~~~~~~~~~~~~~~~
doublemax wrote:
should i pause the thread in his own event?
I think that would defy the purpose of using a thread.
You should also check how many bytes you actually receive each time. If you poll the serial device at maximum speed, i would suspect that you only very few bytes each time. And it would be quite an overhead to send an event for one byte. So maybe you should let the thread sleep for 100ms on each iteration.
Also it's not quite clear why you receive data in a thread, process the data in a main thread and then send another thread to the main window. Why don't you process the serial data directly inside the thread?
Ok, i had to be more clear, sorry :)
  1. My app (the main thread) sends a message with a data request. The device at the other side of the serial cable answers with the data.
  2. The Serial.ReadString is a "high level" function which returns only when a carriage return (0x0D) is received (or for a timeout waiting for it), passing the received bytes as a 0-terminated string in the char array. So the listening thread is needed as the ReadString is calling the serial i/o functions in a loop. (perhaps i should put a Sleep there....) :-k
  3. When Serial.ReadString returns i have to do some protocol and checksum checks (not present in the code right now) and then notify the main thread that the answer is ready (so i used the QueueEvent to allow the thread event to finish and "empty that stack").
  4. In the main thread the App parse the received string, and based on the data received ask for new data (so, going to 1. again) (this is where the listening thread have nothing to do, hence i thought a Pause() could have been useful)
  5. While waiting for the new answer, the main thread draws the received ones (as a line graph or as a bitmap, either is a relative long process which should be a bit shorter than the time the device use to send the answer back)
This whole cycle should happens as fast as possible, meaning that the time between the end of the received answer and the sending of the next query should be the minimum possible; the bottleneck SHOULD be the time the serial device uses to build and send his answer (from 200 to 1024 bytes at baudrate=115200). The queries are short messages (about 20~30 bytes) so i did'nt put the sending task in the listening thread.
doublemax wrote:But as you already suspected, the use of rxBuffer is not good right now, because you're already trying to read into the same buffer not knowing if the old content has already been processed in the main thread. You can either protect the buffer with a mutex or critical section or use a custom event and send the whole data inside the event.
As the program works, i should never read the rxBuffer when i'm still waiting for the answer (the previous one was already save on a string).
Anyway being safe is good: I'd like to see how to do it with a custom event... seems the logical way to do it.

Thanks!
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Confused about wxThreadHelper and how to use it

Post by doublemax »

For the Bind() error:

Code: Select all

//DEFINE_EVENT_TYPE(wxEVT_MYTHREAD)
//DEFINE_EVENT_TYPE(wxEVT_SERIAL)

wxDEFINE_EVENT(wxEVT_MYTHREAD, wxCommandEvent);
wxDEFINE_EVENT(wxEVT_SERIAL, wxCommandEvent);

Code: Select all

/*
BEGIN_DECLARE_EVENT_TYPES()
    DECLARE_EVENT_TYPE(wxEVT_MYTHREAD, -1)
    DECLARE_EVENT_TYPE(wxEVT_SERIAL, -1)
END_DECLARE_EVENT_TYPES()
*/

wxDECLARE_EVENT(wxEVT_MYTHREAD, wxCommandEvent);
wxDECLARE_EVENT(wxEVT_SERIAL, wxCommandEvent);
You were using wxEVT_MYTHREAD and wxEVT_SERIAL, one of them was defined and the other declared.
Use the source, Luke!
User avatar
Parduz
I live to help wx-kind
I live to help wx-kind
Posts: 188
Joined: Fri Jan 30, 2015 1:48 pm
Location: Bologna, Italy

Re: Confused about wxThreadHelper and how to use it

Post by Parduz »

Ok, I tried to go further, and now i have some segmentation fault / invalid pointer / free() errors if i use QueueEvent and also if i call the thread Pause/Resume methods.

(my program runs on Debian, if this matters)

Here the relevant part of my code:

CommManager.cpp

Code: Select all

// class CommManager : public wxEvtHandler, public wxThreadHelper

CommManager::CommManager()
{
	// [...]

	// Create thread
	CreateThread();
	GetThread()->Run();
	Bind(wxEVT_MYTHREAD, &CommManager::OnSerialRxCompleted, this);
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
bool CommManager::SendMessage(const String& tx_Message)
{
CommLogFuncSTART();

	// Threaded Serial comunication
	unsigned int	txlen;
	unsigned int	rxlen;
	bool Result=false;

	// String to char array
	txlen  = tx_Message.length()+1;
	strncpy(txBuffer, (const char*)tx_Message.c_str(), txlen);

	m_waiting = true;
	if (Serial.WriteString(txBuffer)) {
		// Send OK, start the thread for the answer
		rxlen = 0;
/*
		GetThread()->Resume();
*/
		Result = true;
	}else{
		// some kind of error
		// TODO
	}
	return Result;
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wxThread::ExitCode CommManager::Entry()
{
	int Ret;
	wxCommandEvent evt(wxEVT_MYTHREAD);
	
	// loop, so long as we haven't been asked to quit
	while ( ! GetThread()->TestDestroy()) {
        	{
			if (m_busy==false && m_waiting) {
				Ret = Serial.ReadString(rxBuffer);
				evt.SetInt(Ret);
			}else{
				// This branch is needed only 'cause i can't pause the thread
				wxMilliSleep(1000);
				evt.SetInt(-55);
			}
			wxPostEvent(this, evt);	// This call works
			//wxQueueEvent(this, &evt);	// This call crashes the program
		}
	}
	// we have been asked to stop
	return (wxThread::ExitCode)0;
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
void CommManager::OnSerialRxCompleted(wxCommandEvent& SerialEvent)
{

	// Event fired from the thread

	int Ret = SerialEvent.GetInt();
	unsigned int	rxlen;

	if (Ret>0) {
		rxlen = Ret;
		Answer = rxBuffer;
	}else if (Ret==0) {
		// Timeout
		// TODO
	}else if (Ret==-55) {
		// the thread was "Sleeping"
		// TODO
	}else{
		// RX Error
		// TODO
	}
/*
	GetThread()->Pause();	// This call will crashes the program
*/

	// Queue the event on the main frame, to let the thread event call finish
	wxCommandEvent evt(wxEVT_SERIAL);
	evt.SetInt(Ret);

	// This call makes GUI_MainForm::OnCommRx works, but then the program crashes when it returns.
	//wxTheApp->GetTopWindow()->GetEventHandler()->QueueEvent(&evt);
	wxPostEvent(wxTheApp->GetTopWindow()->GetEventHandler(), evt);	// This call works

	m_waiting = false;
}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
GUI_MainForm.cpp

Code: Select all

GUI_MainForm::GUI_MainForm(wxWindow* parent) : fMainForm(parent)
{
	//[...]
	Comm = new(CommManager);
	LastMsg = msg_Unknown;
	Bind(wxEVT_SERIAL, &GUI_MainForm::OnCommRx, this);
	//[...]
}
//+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
void GUI_MainForm::OnCommRx(wxCommandEvent& SerialEvent)
{
	wxString mf_dbgstr;
	mf_dbgstr.Printf("--- OnCommRx: evt ID  = %d,\n                  int = %d", SerialEvent.GetId(), SerialEvent.GetInt());
	wxLogDebug(mf_dbgstr);
}
//+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

What am I doing wrong?
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Confused about wxThreadHelper and how to use it

Post by doublemax »

Code: Select all

wxCommandEvent evt(wxEVT_SERIAL);
   evt.SetInt(rxlen);
   wxTheApp->GetTopWindow()->GetEventHandler()->QueueEvent(&evt);
The event passed to QueueEvent must be created on the heap. It will be deleted by wxWidgets when it has been processed.
Use the source, Luke!
User avatar
Parduz
I live to help wx-kind
I live to help wx-kind
Posts: 188
Joined: Fri Jan 30, 2015 1:48 pm
Location: Bologna, Italy

Re: Confused about wxThreadHelper and how to use it

Post by Parduz »

Oh.... Thanks a lot!
Everything works without crashes, now. :oops:
I have a question: the event fired from the listening thread (in the Entry() loop) will be executed in that thread or in the main one?
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Confused about wxThreadHelper and how to use it

Post by doublemax »

the event fired from the listening thread (in the Entry() loop) will be executed in that thread or in the main one?
Events are always handled in the main thread.No exceptions.
Use the source, Luke!
Post Reply