QThread richtig einsetzen

[Schlangenmensch]

Skylac06, kann es sein, dass du vergisst, deine Worker Instance zu löschen? Ich würde im Destruktor noch ein:

delete worker;

erwarten.

Es gibt tatsächlich auch ein kleines Beispiel von QT https://wiki.qt.io/QThreads_general_usage

[Skylac06]

Schlangenmensch schrieb:

Skylac06, kann es sein, dass du vergisst, deine Worker Instance zu löschen? Ich würde im Destruktor noch ein:

delete worker;

erwarten.

Das ist vollkommen korrekt. Entschuldigung, habe ich auf die Schnelle dann doch einen Fehler eingebaut.
Ich habe es korrigiert. Vielen Dank, Schlangenmensch.

[Saheb]

@Skylac06

vielen dank.
Eine super Einführung.
Danke nochmal

[Saheb]

Ich habe es noch einen Frage:

Warum ist nicht zu empfehlen beim Threadanwendung von QThread zu erben und somit wird die run() Methode überschrieben?

Danke

[Schlangenmensch]

Saheb schrieb:

Warum ist nicht zu empfehlen beim Threadanwendung von QThread zu erben und somit wird die run() Methode überschrieben?

Um aus dem, von partsoft angegebenen Blog zu zitieren:

QThread was designed and is intended to be used as an interface or a control point to an operating system thread, not as a place to put code that you want to run in a thread. We object-oriented programmers subclass because we want to extend or specialize the base class functionality. [...]

tl;dr: Weil es dafür nicht entwickelt worden ist.

[Saheb]

Hallo,

wenn ich folgende habe:
Header File:

class QTcpSocket;

class CAgilentLan : public QThread
{

public:

	typedef enum 
	{
		enRspTimeout = -2, 
		enRspOffline,
		enRspOK,
		enRspError
	} t_enRspType ;

	CAgilentLan(QString strIpAdress_p, int iTimeout_p, bool boSimulate_p=false, bool boDebug_p=false);
	~CAgilentLan();

	// High-Level Funktionen
	QByteArray ExecuteGetScreenshot(void);
	void ExecuteSaveScreenshot(QString &strPathFileName_p, const char *strFormat_p);
	void ExecuteSettingsSave(unsigned char ucRegisterNo_p);
	void ExecuteSettingsRecall(unsigned char ucRegisterNo_p);
	double ExecuteQuickMeasure(unsigned char ucChannel_p, char *pchType_p);
	double ExecuteStatMeasure(char *pchType_p, bool boTrigger_p);
	void ExecuteResetMeasurementStatistics(void);

	bool Open(const QString &strIpAdress_p, const int iTimeout_p);
	bool Close(const int iTimeout_p);
	t_enRspType Execute(const char *strCmd_p, const int iWaitResponse_p, const int iTimeout_p, const int iRspDelayed_p);
	QByteArray GetLastResponse(void);

	t_enRspType ExecuteSimulated(const char *strCmd_p);
	QString GetAgilentIdentifikation();

	...

protected:
	void run();

private:
	bool m_boDebug;
	bool m_boSimulate;

	QMutex m_MtxProcess;
	QMutex m_MtxResponse;

	QWaitCondition m_WaitCondProcess;
	QByteArray m_ByteArrayWrite;
	QByteArray m_ByteArrayRead;

	QByteArray m_IDN_AgilentIdentification;

	//Hilfsvariable
	double m_SDDevPeriodeA ;
	double m_SDDevDCA;
	double m_SDDevDCB;
	double m_SDDevWidthZ;

	double m_OZiSDDevPeriodeA;
	double m_OZiSDDevDCA;
	double m_OZiSDDevDCB;
	double m_OZiSDDevWidthZ;

	double m_OZiMeanPeriodeA;
	double m_OZiMeanDCA;
	double m_OZiMeanDCB;
	double m_OZiMeanWidthZ;

	QByteArray m_ByteSTDD;
	QByteArray m_ByteMean;
	QStringList m_ListSTDDev;
	QStringList m_ListMean;

	QTcpSocket *m_pTcpSctCtrl;

	QString m_strCtrlIP;
	int m_iTimeoutClose;
	int m_iTimeout;
	int m_iResponseDelayed;
	int m_iWaitResponse;

	t_enRspType m_enRetVal;
};

Cpp File:

CAgilentLan::CAgilentLan(QString strIpAdress_p, int iTimeout_p, bool boSimulate_p, bool boDebug_p):m_SDDevPeriodeA(0), m_SDDevDCA(0),m_SDDevDCB(0),m_SDDevWidthZ(0)
{
//	boSimulate_p = false;

	m_boDebug = boDebug_p;
	m_boSimulate = boSimulate_p;

	m_enRetVal = enRspOffline;
	m_strCtrlIP = QString("");
	m_iTimeout = 0;
	m_iTimeoutClose = iTimeout_p;
	if (this->Execute("CreateObject", 0, 0, 0) != enRspOK)
	{
		throw(QString("Error AgilentLan.CPP/Constructor TcpSocket Create memory error"));
	}

	this->Open(strIpAdress_p, iTimeout_p);
}

CAgilentLan::~CAgilentLan()
{
	while(isRunning());

	this->Close(m_iTimeoutClose);

	while(isRunning());

	if (this->Execute("DeleteObject", 0, 0, 0) != enRspOK)
	{
		throw(QString("Error AgilentLan.CPP/Destructor TcpSocket Delete memory error"));
	}

	while(isRunning());
}
bool CAgilentLan::Open(const QString &strCtrlIP_p, const int iTimeout_p)
{
	bool boRet_l = true;

	m_strCtrlIP = strCtrlIP_p;

	if (this->Execute("OpenConnection", 0, iTimeout_p, 0) != enRspOK)
	{
		throw(QString("Error AgilentLan.CPP/Open TcpSocket Open Connection error"));
	}

	if (m_boDebug)
	{
		throw(QString("Tcp Socket is open"));
	}

	return boRet_l;
}

bool CAgilentLan::Close(const int iTimeout_p)
{
	bool boRet_l = true;

	while(isRunning());

	if (this->Execute("CloseConnection", 0, iTimeout_p, 0) != enRspOK)
	{
		throw(QString("Error AgilentLan.CPP/Close Connection error"));
	}

	if (m_boDebug)
	{
		throw(QString("Tcp Socket is closed"));
	}

	while(isRunning());

	return boRet_l;
}

CAgilentLan::t_enRspType CAgilentLan::Execute(const char *strCmd_p, const int iWaitResponse_p, const int iTimeout_p, const int iRspDelayed_p)
{
	CAgilentLan::t_enRspType enRetVal_l = enRspOffline;

	if (!isRunning())
	{
		m_ByteArrayRead.clear();

		m_ByteArrayWrite.clear();
		m_ByteArrayWrite.append(strCmd_p);

		m_iWaitResponse = iWaitResponse_p;
		m_iTimeout = iTimeout_p;
		m_iResponseDelayed = iRspDelayed_p;

		start();

		m_MtxProcess.lock();
		m_WaitCondProcess.wait(&m_MtxProcess,-1);
		m_MtxProcess.unlock();

		enRetVal_l = this->m_enRetVal;
	}
	else
	{
		enRetVal_l = enRspError;
	}

	while(isRunning());

	return enRetVal_l;
}

QByteArray CAgilentLan::GetLastResponse(void)
{
	QByteArray ay_l;

	m_MtxResponse.lock();
	ay_l = m_ByteArrayRead;
	m_MtxResponse.unlock();

	return ay_l;
}
void CAgilentLan::run()
{
	bool boRun_l = true;
	bool boSent_l = false;

	const char *pStrCmd_l = NULL;
	const char *pStrRsp_l = NULL;

	m_enRetVal = enRspError;

	if (strcmp(m_ByteArrayWrite.data(),"CreateObject")==0)
	{
		if (this->m_boSimulate == true)
		{
			m_enRetVal = enRspOK;
		}
		else
		{
			m_pTcpSctCtrl = (QTcpSocket *)new QTcpSocket;
			if (m_pTcpSctCtrl != NULL)
			{
				m_enRetVal = enRspOK;
			}
		}

		boRun_l = false;
	}

	if (strcmp(m_ByteArrayWrite.data(),"DeleteObject")==0)
	{
		if (this->m_boSimulate == true)
		{
		}
		else
		{
			if (m_pTcpSctCtrl != NULL)
			{
				delete m_pTcpSctCtrl;
			}
		}

		m_enRetVal = enRspOK;
		boRun_l = false;
	}

	if (strcmp(m_ByteArrayWrite.data(),"OpenConnection")==0)
	{
		if (this->m_boSimulate == true)
		{
			m_enRetVal = enRspOK;
		}
		else
		{
			m_pTcpSctCtrl->connectToHost(m_strCtrlIP,5025);
			if (m_pTcpSctCtrl->waitForConnected(m_iTimeout) == true)
			{
				m_enRetVal = enRspOK;
			}
		}

		boRun_l = false;
	}

	if (strcmp(m_ByteArrayWrite.data(),"CloseConnection")==0)
	{
		if (this->m_boSimulate == true)
		{
			m_enRetVal = enRspOK;
		}
		else
		{
			m_pTcpSctCtrl->disconnectFromHost();
			if ((m_pTcpSctCtrl->state() == QAbstractSocket::UnconnectedState) || (m_pTcpSctCtrl->waitForDisconnected(m_iTimeout)))
			{
				m_enRetVal = enRspOK;
			}
		}

		boRun_l = false;
	}

	int iTimeout_l = m_iTimeout;

	while (boRun_l && (!this->m_boSimulate))
	{
 		if (boSent_l == false)
		{
			boSent_l = true;
			// Kommando absetzen
			m_pTcpSctCtrl->write(m_ByteArrayWrite.data(),m_ByteArrayWrite.size());
			// Auf Antwort warten
			if (m_iWaitResponse >= 0)
			{
				QThread::msleep(m_iWaitResponse);
			}
			else
			{
				boRun_l = false;
			}
		}

		// Anwort auf Kommando abwarten	
		if ((m_pTcpSctCtrl->bytesAvailable() > 0) && (boRun_l == true))
		{
			m_MtxResponse.lock();

			m_ByteArrayRead.append(m_pTcpSctCtrl->readAll());

			if (m_boDebug)
			{
				throw(QString(m_ByteArrayRead.data()));
			}

			m_MtxResponse.unlock();

			iTimeout_l = m_iTimeout;
//			iTimeout_l = m_iWaitResponse;
		}
		else
		{
			if (m_pTcpSctCtrl->waitForReadyRead(iTimeout_l) == false)
			{
				m_MtxResponse.lock();

				if (m_ByteArrayRead.size() == 0)
				{
					if (m_boDebug)
					{
						throw(QString("AgilentLan Timeout"));
					}
					m_enRetVal = enRspTimeout;
				}
				else
				{
					m_enRetVal = enRspOK;
				}

				m_MtxResponse.unlock();

				boRun_l = false;					
			}
		}
	}

	QThread::msleep(m_iResponseDelayed);

	QMutexLocker locker(&m_MtxProcess);

	m_WaitCondProcess.wakeOne();

}

Diese Klasse möchte ich gerne umschreiben und zwar nach der vorgeschlagen "Qthread richtig einsetzen".
Das heisst ich muss eine neue Klasse z.B Work erstellen richtig?

[Schlangenmensch]

Du musst nicht unbedingt eine neue Klasse erstellen.

Du könntest zum Beispiel deine Klasse nicht von QThread erben lassen, sondern von QObject.
Die run() Methode raus nehmen, bzw. umbenennen und alle Funktionsaufrufe, bei denen du QThread Funktionen aufrufst raus nehmen (bzw. umschreiben).

Aber: Bevor du damit anfängst, überleg dir erst mal, welche Aufgabe du genau parallelisieren möchtest, warum diese Aufgabe parallel laufen soll, wie viele parallele Tasks du zulassen möchtest usw.

Außerdem ist es dann vlt eine Überlegung wert, ob du das nicht mit Futures und Promises realisieren kannst (Ein paar Erklärungen dazu: QT: http://blog.qt.io/blog/2017/04/18/multithreaded-programming-future-promise/ Boost: https://theboostcpplibraries.com/boost.thread-futures-and-promises) Das würde dir die low level Thread Erstellung abnehmen.

[Saheb]

Schlangenmensch schrieb:

Du könntest zum Beispiel deine Klasse nicht von QThread erben lassen, sondern von QObject.

Okay
Das ist schon klar.

Schlangenmensch schrieb:

Die run() Methode raus nehmen, bzw. umbenennen und alle Funktionsaufrufe, bei denen du QThread Funktionen aufrufst raus nehmen (bzw. umschreiben).

Die run() Methode nenne ich um zu einen SLOT Methode richtig?

[Schlangenmensch]

Ja, ich glaube du meinst das richtige. Du benennst die Methode um und deklarierst die als SLOT.

public slots:
   void doAnything();

[Saheb]

Kannst du bitte mich auf die Sprünge helfen?
Ich habe es so angefangen:

class QTcpSocket;

class CAgilentLan : public QObject
{
	Q_OBJECT

public:
 ...
    CAgilentLan(QString strIpAdress_p, int iTimeout_p, bool boSimulate_p=false, bool boDebug_p=false);
    ~CAgilentLan();

...
    t_enRspType Execute(const char *strCmd_p, const int iWaitResponse_p, const int iTimeout_p, const int iRspDelayed_p);
 ..

public slots:
    void AgilentStart();

};

Die run() Methode umbenannt und zu einen Slot deklariert.
Mir ist dann nicht ganz klar wie soll ich die restlichen Methoden zu SLOT und SIGNAL einbinden kann?

In der alte Version was diese Methode :

t_enRspType Execute(const char *strCmd_p, const int iWaitResponse_p, const int iTimeout_p, const int iRspDelayed_p);

Zentral.

Das heisst jede aufruf diese Methode verusacht eine automatische Aufruf der

run()

Methode.
Ich will weiterhin dieses Ablauf beibehalten damit die Änderung minimal bleibt.

[Schlangenmensch]

Hi,

vielleicht hilft dir die Seite: https://wiki.qt.io/QThreads_general_usage weiter.

Kurz zusammengefasst, an der Stelle, an der du CAgilentLan erzeugst und aufrufst muss du einen QThread erzeugen, mit moveToThread das Objekt in den Thread "schieben" und dann eben die Verbindung zwischen Signal und Slot herstellen.

Also ungetestet hast du dann irgendwo sowas

QThread* thread = new QThread;
CAgilentLan* worker = new CAgilentLan(/*Argumente nicht vergessen */);
worker->moveToThread(thread);
connect(thread, SIGNAL (started()), worker, SLOT (AgilentStart()));
thread->start();

Was hier noch fehlt ist das warten auf Beendigung des Threads (z.B. über ein Signal) und das löschen des Threads und CAgilentLan. Hierfür stellt QT für QObjects die Funktion QObject::deleteLater() bereit.

[Saheb]

Hallo,

vielen dank für die Antwort.

Ich habe es noch eine Frage:

Schlangenmensch schrieb:

an der Stelle, an der du CAgilentLan erzeugst und aufrufst muss du einen QThread erzeugen....

Diesem satz habe ich nicht genau verstanden.

Bei jeden aufruf dieses Object CAgilentLan muss ich bei der betroffene Class in der Konstruktor sowas schreiben:

QThread* thread = new QThread;
CAgilentLan* worker = new CAgilentLan(/*Argumente nicht vergessen */);
worker->moveToThread(thread);
connect(thread, SIGNAL (started()), worker, SLOT (AgilentStart()));
thread->start();

oder verstehe ich das falsch?

Sorry noch mal

[Schlangenmensch]

An wie vielen Stellen hast du das denn?

Jedes mal, wenn du ein Objekt der Klasse CAgilentLan in einem extra Thread ausführen möchtest, benötigst du sowas in der Richtung.

Das muss nicht im Konstruktor sein.

Die Frage, die sich mir nach wie vor stellt ist, was genau willst du parallel ausführen.

Willst du mehrmals CAgilentLan::AgilentStart() parallel ausführen (wenn ich mich richtig erinnere gabs da Netzwerkverbungen)? Oder was ist deine Idee hinter der Parallelisierung.

Edit: Zu früh um richtig zu schreiben.

[Saheb]

Schlangenmensch schrieb:

An wie vielen Stellen hast du das denn?

Bei eine Klasse habe ich das in mehreren Stellen (15 mal).

Jedes mal, wenn du ein Objekt der Klasse CAgilentLan in einem extra Thread ausführen möchtest, benötigst du sowas in der Richtung.

Das muss nicht im Konstruktor sein.
[quote="Schlangenmensch"
Die Frage, die sich mir nach wie vor stellt ist, was genau willst du parallel ausführen.
[/quote]
Während es mit der Agilent kommuniziert ist, müssen die anderen Thread warten.

Schlangenmensch schrieb:

Willst du mehrmals CAgilentLan::AgilentStart() parallel ausführen (wenn ich mich richtig erinnere gabs da Netzwerkverbungen)? Oder was ist deine Idee hinter der Parallelisierung.

Nein Parallesieren möchte ich das nicht.
Mir ist wichtig, dass die Threads synchronisiert sind.

Was ich eigentlich für die Zukunft habe, ist der jetzigen Stand der CAgilentLan class zu erweitern in dem ich die VISA "https://en.wikipedia.org/wiki/Virtual_Instrument_Software_Architecture" Standard zu benutzen.
Grund für diese Umstieg: Der jetzigen Stand dauert sehr lange, wenn es stets eine neue Verbindug aufgebaut ist.

[Schlangenmensch]

Saheb schrieb:

Nein Parallesieren möchte ich das nicht.
Mir ist wichtig, dass die Threads synchronisiert sind.

Das Widerspricht sich. Natürlich kannst du mehrere Threads synchronisieren. Aber trotzdem laufen die parallel. Wenn du keine Parallelisierung haben möchtest, benötigst du keine Threads.
Es bringt keinen Vorteil, wenn du aus deinem Hauptprogramm einen Thread startest um dann direkt im Hauptprogramm darauf zu warten, dass der Thread fertig wird.

VISA scheint mir eine Kommunikations API zu sein. Ich habe damit noch nichts zu tun gehabt und habe auch nicht vor jetzt die Spezifikation zu lesen. Ich kann mir aber nicht vorstellen, dass die mehrere Threads erwartet.

[Saheb]

Sorry
natürlich sind mehrere Thread zusammen gestartet abgesehen von der Hauptthread "GUI".
Ja VISA ist eine Kommunikation API.

[Schlangenmensch]

An jeder Stelle wo du die Threads mit dem CAgilentLan in mehreren Threads ausführen willst, wirst du dann die Threads erstellen müssen und mit moveToThread eben den Thread an das Objekt übergeben.

Wenn du potentiell 15 Stellen hast an denen das vokommt, dann eben an 15 verschiedenen Stellen. Eventuell musst du auch nicht immer neue Threads erzeugen, sondern kannst Threads wieder verwenden.

Aber wenn du 15 Stellen im Code hast, wo du die Sachen ausführst, klingt das für mich so, als ob man das geschickter Designen kann. Aber das ist eine andere Frage und hat mit QThreads nichts zu tun

[Saheb]

Hi,

vielen vielen dank für deine Unterstützung.

Schlangenmensch schrieb:

Aber wenn du 15 Stellen im Code hast, wo du die Sachen ausführst, klingt das für mich so, als ob man das geschickter Designen kann.

Welce Design Pattern hätte man nehmen soll (nur zu Info).

[Schlangenmensch]

Saheb schrieb:

Welce Design Pattern hätte man nehmen soll (nur zu Info).

Das kann ich nicht beantworten, ohne genauere Informationen über das gesamte Projekt zu haben. Das würde hier deutlich zu weit gehen.
Aber an 15 verschiedenen Stellen unterschiedliche Instanzen einer recht Zentralen Klasse zu haben ist doch eher ungewöhnlich.

Hast du viele Klassen, die einen CAgilentLan als Member haben? Dann würde sich vlt. Vererbung anbieten.
Möglicherweise auch ein strikteres objektorientiertes Design (SOLID)... es gibt viele Möglichkeiten und die können alle richtig oder total falsch sein.

[Saheb]

Schlangenmensch schrieb:

Hast du viele Klassen, die einen CAgilentLan als Member haben?

In noch eine Klasse wird die CAgilentLan verwendet.