QThread with multiple methods

JohnCu

@Christian-Ehrlicher
Here you have a maximally simplified code, which does not contain any sophisticated algorithm, just the problematic part:

App.h code"

#pragma once

#include <QtWidgets/QMainWindow>
#include "ui_App.h"
#include "worker.h"
#include <vector>
#include <QMessageBox>
#include <qfiledialog.h>
#include <qlistwidget.h>
#include <qfuture.h>
#include <QCoreApplication>
#include <QtConcurrent/qtconcurrentrun.h>
#include <QThread>
#include <qwt_plot.h>
#include <qwt.h>
#include <Qwt_Legend.h>
#include <Qwt_Plot_Grid.h>
#include <Qwt_Plot_Curve.h>
#include <Qwt_Symbol.h>
#include <direct.h>
#include <algorithm>
#include <qmainwindow.h>
#include <qtoolbar.h>
#include <qtoolbutton.h>
#include <qcombobox.h>
#include <qslider.h>
#include <qlabel.h>
#include <qcheckbox.h>
#include "plot.h"
#include "qwt_color_map.h"
#include <string>
#include <cctype>
#include <fstream>
#include <iostream>
#include <QDebug>

class App : public QMainWindow
{
	Q_OBJECT
	QThread workerThread;
public:
	// methods
	App(QWidget *parent = Q_NULLPTR);	
	~App();
private:
	// methods
	void disable_all_buttons();
	
	// variables
	Worker worker_obj;
	Plot* plot = new Plot("plot");
	Ui::App_GUIClass ui;

signals:
	void onConnecttoHardware();
	void onRun();

public slots:
	void enable_all_buttons();
	void plot_results();

private slots: 
	void on_Run_triggered();
};

App. cpp code:

#include "App.h"

App::App(QWidget *parent)
	: QMainWindow(parent)
{
	ui.setupUi(this);
	
	// <- prepare gui

	worker_obj.moveToThread(&workerThread);
	connect(&workerThread, &QThread::finished, &worker_obj, &QObject::deleteLater);
	// signals from worker_obj and slots in App
	connect(&worker_obj, &Worker::on_enable_buttons, this, &App::enable_all_buttons);
	connect(&worker_obj, &Worker::update_plot, this, &App::plot_results, Qt::DirectConnection);
	// signals from App and slots in worker_obj
	connect(this, &App::onRun, &worker_obj, &Worker::acquire_data, Qt::BlockingQueuedConnection);
	connect(this, &App::onConnecttoHardware, &worker_obj, &Worker::connect_to_hardware, Qt::BlockingQueuedConnection);
	workerThread.start();
}

void App::on_Run_triggered()
{
	disable_all_buttons();
	QMessageBox::information(this, "Message", "Prepare hardware");

	emit onConnecttoHardware();

	// if connected to hardware
	QMessageBox::information(this, "Message", "Hardware connected");

	emit onRun();
			
	QMessageBox::information(this, "Message", "Operation succesful");
}

App::~App()
{
	delete plot;
	workerThread.quit();
	workerThread.wait();
}

void App::enable_all_buttons() {
	// enable all gui buttons
}
void App::disable_all_buttons() {
	// disable all gui buttons
}

void App::plot_results()
{
	//QMutex mutex;
	//mutex.lock();

	QHBoxLayout* myLayoutTrans = new QHBoxLayout(ui.tabWidgetResults->widget(3)); // modify 4th tab in gui
	myLayoutTrans->addWidget(plot);
	int current_tab_index;
	current_tab_index = ui.tabWidgetResults->currentIndex();

	ui.tabWidgetResults->widget(3)->setLayout(myLayoutTrans);
	plot->Update_plot("y axis", worker_obj.hard_obj->result_x1,
		worker_obj.hard_obj->result_y1, worker_obj.hard_obj->result_x2,
		worker_obj.hard_obj->result_y2,	1000);

	delete myLayoutTrans;
	//mutex.unlock();
}

worker.h code:

#include "hardware_control.h"
#include <iostream>
#include <iomanip>
#include <sstream>
#include <armadillo>
#include <QThread>
#include <QObject>
#include <QtCore>
#include <cstdio>
#include <memory>
#include <stdexcept>
#include <string>
#include <array>
#include <chrono>
#include <thread>
#include <stdlib.h>

class Worker: public QObject
{
	Q_OBJECT
public:
	// methods
	explicit Worker(QObject *parent=0);
	~Worker();

	Hardware_control* hard_obj = new Hardware_control();

signals:
	void on_enable_buttons();
	void update_plot();

public slots:
	void acquire_data();
	void connect_to_hardware();

private:
	QMutex mutex;
	QWaitCondition condition;
	bool quit;
};

worker.cpp code:

#include "worker.h"

Worker::Worker(QObject *parent){
}

Worker::~Worker()
{
	delete hard_obj;
}

void Worker::connect_to_hardware()
{
	// connects to hardware via serial port
}

void Worker::acquire_data() {
	// getting data from hardware

void Worker::acquire_data() {
	// getting data from hardware

	while (!end) {
		hard_obj->tune_hardware(); // takes from 10ms to 5s
		while (more_data_required) {
			hard_obj->get_data_from_hardware(); // takes about 10ms
			hard_obj->get_results();

			emit update_plot();

			// do some calculations

			hard_obj->get_data_from_hardware(); // takes about 10ms
			hard_obj->get_results();

			emit update_plot();
		}
	}
	emit on_enable_buttons();
}

hardware_control.h code:

#pragma once
#include "hardware_control.h"
#include <QFile>
#include <QDir>
#include <armadillo>

class Hardware_control
{
public:
	Hardware_control();
	~Hardware_control();
	
	void get_data_from_hardware();
	void get_results();
	void postprocess_data();
	void download_results();
	void tune_hardware();

	double* result_x1, * result_y1;
	double* result_x2, * result_y2;


signals:
	void update_plot();

private:
	int port_number;
	arma::vec hardware_output_x, hardware_output_y;
};

hardware_control.cpp code:

#include "hardware_control.h"

Hardware_control::Hardware_control() {
	hardware_output_x = arma::zeros<arma::vec>(1000);
	hardware_output_y = arma::zeros<arma::vec>(1000);
	result_y1 = new double[1000];
	result_x1 = new double[1000];
	result_y2 = new double[1000];
	result_x2 = new double[1000];
}

Hardware_control::~Hardware_control() {
	delete[] result_y1;
	delete[] result_x1;
	delete[] result_y2;
	delete[] result_x2;
}

void Hardware_control::tune_hardware()
{
	// tune hardware before getting result data
}

void Hardware_control::get_data_from_hardware() {
	// gets hardware_output_x & hardware_output_y from hardware
	for (int i = 0; i < 1000; i++) {
		hardware_output_x[i] = get_hardware_response_x(i);
		hardware_output_y[i] = get_hardware_response_y(i);
	}
}

void Hardware_control::download_results() {
	for (int i = 0; i < 1000; i++) {
		result_x1[i] = hardware_output_x[i];
		result_y1[i] = hardware_output_y[i];
	}
}

void Hardware_control::get_results() {
	// data postrpocess based on the data previously obtained from hardware
	postprocess_data();

	download_results();
}

void Hardware_control::postprocess_data() {
	// transforms hardware_output_x & hardware_output_y into result_x2 & result_y2
	for (int i = 0; i < 1000; i++) {
		result_x2[i] = postprocess_x(hardware_output_x[i]);
		result_y2[i] = postprocess_y(hardware_output_y[i]);
	}
}

Plot.h code:

#include <qwt_plot.h>
#include <qwt_plot_zoomer.h>
#include <qwt_plot_panner.h>
#include <qwt_plot_grid.h>
#include <qwt_plot_curve.h>
#include <qwt_symbol.h>
#include <qwt_plot_canvas.h>


class Plot : public QwtPlot
{
	Q_OBJECT

public:

	Plot(std::string type);//sets up plot 
	~Plot();
	void Update_plot(const QString& title, const double X_val[1601 * 16], const double Y_val[1601 * 16], const double X2_val[1601 * 16], const double Y2_val[1601 * 16], int number);
	void detach_curve();

protected:
	//QwtPlotCurve * curve_;

private:
	double max_Y_val, max_Y2_val;
	double min_Y_val, min_Y2_val;
	QwtSymbol* symbol;
	QwtPlotGrid* grid;
	QwtPlotCurve curve1, curve2;
	QwtPointSeriesData* myData, *myData2;
	QVector<QPointF> samples;
	QVector<QPointF> samples2;
	QwtPlotCanvas* canvas;
	QwtPlotZoomer* zoomer;
	QwtPlotPanner* panner;
	bool if_update_plot;
};

Plot.cpp code:

#include "plot.h"

class MyZoomer : public QwtPlotZoomer
{
public:
	MyZoomer(QWidget *canvas) :
		QwtPlotZoomer(canvas)
	{
		setTrackerMode(AlwaysOn);
	}

	virtual QwtText trackerTextF(const QPointF &pos) const
	{
		QColor bg(Qt::white);
		bg.setAlpha(200);

		QwtText text = QwtPlotZoomer::trackerTextF(pos);
		text.setBackgroundBrush(QBrush(bg));
		return text;
	}
};

Plot::Plot(std::string type)
{
	if_update_plot = false;
	setObjectName("plot");
	setAxisTitle(xBottom, "x axis");
	setAxisScale(yLeft, -150, 0);


	QwtPlotCanvas *canvas = new QwtPlotCanvas();
	canvas->setPalette(Qt::white);
	canvas->setBorderRadius(10);

	setCanvas(canvas);

	// grid
	grid = new QwtPlotGrid();
	grid->attach(this);

	zoomer = new QwtPlotZoomer(canvas);
	zoomer->setRubberBandPen(QColor(Qt::black));
	zoomer->setTrackerPen(QColor(Qt::black));
	zoomer->setMousePattern(QwtEventPattern::MouseSelect2,
		Qt::RightButton, Qt::ControlModifier);
	zoomer->setMousePattern(QwtEventPattern::MouseSelect3,
		Qt::RightButton);

	panner = new QwtPlotPanner(canvas);
	panner->setMouseButton(Qt::MidButton);
	this->setAutoReplot(true);
	replot();
}

Plot::~Plot()
{
	delete zoomer;
	delete panner;
	if (if_update_plot == true) {
		delete grid;
	}
}

void Plot::Update_plot(const QString& title, const double X_val[1601 * 16], const double Y_val[1601 * 16], const double X2_val[1601 * 16], const double Y2_val[1601 * 16], int number)
{
	if_update_plot = true;
	max_Y_val = -200;
	min_Y_val = 0;
	max_Y2_val = -200;
	min_Y2_val = 0;
	double min_f = 0, max_f = 0;
	myData = new QwtPointSeriesData;
	myData2 = new QwtPointSeriesData;

	// clear samples
	for (int i = 0; i < samples.length(); i++) {
		samples.pop_back();
	}
	while (samples.length() > 0) {
		samples.pop_back();
	}
	for (int i = 5; i < number; i++) {
		if (Y_val[i] != 0) {
			samples.push_back(QPointF(X_val[i]/1e9, Y_val[i]));
			if (max_Y_val < Y_val[i]) {
				max_Y_val = Y_val[i];
			}
			if (min_Y_val > Y_val[i]) {
				min_Y_val = Y_val[i];
			}
			max_f = std::ceil(X_val[i]);
			if (min_f == 0) {
				min_f = std::floor(X_val[i]);
			}
		}
	}
	// clear samples2
	for (int i = 0; i < samples2.length(); i++) {
		samples2.pop_back();
	}
	while (samples2.length() > 0) {
		samples2.pop_back();
	}
	for (int i = 5; i < number; i++) {
		if (Y2_val[i] != 0) {
			samples2.push_back(QPointF(X2_val[i] / 1e9, Y2_val[i]));
			if (max_Y2_val < Y2_val[i]) {
				max_Y2_val = Y2_val[i];
			}
			if (min_Y2_val > Y2_val[i]) {
				min_Y2_val = Y2_val[i];
			}
		}
	}

	curve1.detach();
	curve2.detach();
	grid = new QwtPlotGrid();
	grid->attach(this);

	myData->setSamples(samples);
	curve1.setStyle(QwtPlotCurve::Lines);
	curve1.setPen(Qt::blue, (2.0), Qt::SolidLine);

	myData2->setSamples(samples2);
	curve2.setStyle(QwtPlotCurve::Lines);
	curve2.setPen(Qt::red, (1), Qt::SolidLine);

	setAxisScale(yLeft, std::floor(min_Y_val), std::ceil(max_Y_val));
	setAxisScale(xBottom, min_f/1e9 - 0.000001, max_f/1e9 + 0.000001);
	setAxisTitle(yLeft, title);

	curve1.setData(myData);
	curve1.attach(this);
	curve2.setData(myData2);
	curve2.attach(this);
	this->replot();
}

void Plot::detach_curve()
{
	curve1.detach();
}

Christian Ehrlicher

@JohnCu said in QThread with multiple methods:

connect(&worker_obj, &Worker::update_plot, this, &App::plot_results, Qt::DirectConnection);

This is wrong and will lead to crashes. Also the blockingQueued stuff is not needed - at least I don't see a need for it. Apart from that your code does not compile.

JohnCu

@Christian-Ehrlicher Please take look one more time at Worker::acquire_data(), because i have forgotten to add something significant. Plot needs to be updated before stepping into subsequent line, because then, some calculations are taking place, which modify vectors used to visualize results. I have also added plot class
When i dont set Qt::DirectConnection the algorithm goes well, but as a result i have multiple plot of the same vector, and the data is the one acquired just before showing gui information message. So to sum up, i click run in gui, then some data is acquired multiple times and after a while, both result and gui msgbox appear.
It would be hard for you to compile it, because you dont have the hardware necessary to generate the result vectors. Moreover, a algorithm (in worker class) is sophisticated, and would make whole code hard to be analyzed. Thats why i isolated the problem and passed it like this.

Christian Ehrlicher

@JohnCu said in QThread with multiple methods:

Please take look one more time

You still not fixed my comments. DirectConnection is wrong!

JohnCu

@Christian-Ehrlicher said in QThread with multiple methods:

DirectConnection

I understand, at first there was no connection specifier in this connect call. In previous post I described what happened then. So do you see any solution for mentioned problem?

Christian Ehrlicher

@JohnCu said in QThread with multiple methods:

So do you see any solution for mentioned problem?

Don't use blocking and direct connections when working with threads. I don't see a problem calling a slot when a push button is pressed inside the thread which then triggers the execution.

JohnCu

@Christian-Ehrlicher The situation is as follows, I want to have my gui responsible all the time, but workers thread needs to emit signal to gui in order to plot the results. And the results needs to be ploted before subsequent step in workers thread. Now, data is plotted while app gets stucked on gui msgbox. I mean, when I dont use blocking and direct connections for update plot.

Christian Ehrlicher

@JohnCu said in QThread with multiple methods:

And the results needs to be ploted before subsequent step in workers thread.

But why? But I don't care - simply emit another signal once the plotting is done so your thread can continue to calculate.

JohnCu

@Christian-Ehrlicher Ok, I will try. Thank you anyway, for all of your replies :)

JohnCu

@Christian-Ehrlicher One more thing. I have menaged to handle stuff in workers thread (after emitting signal which initiates plotting, workers thread waits for succesful visualization), but there is still a problem in on_Run_triggered() method in App.cpp. After initiating result acquisition (emit onRun();) the main thread also needs to wait till operation is finished. Moreover, gui needs to be responsible all the time. When i create here a loop, which waits for signal from
workers class, the gui thread is stucked in this loop and visualization in gui is impossible. Are there any methods which, will enable me to have responsible thread (in order to plot according to signal from worker), but paused on emit onRun() line?

Christian Ehrlicher

Show a modal wait dialog.

JohnCu

@Christian-Ehrlicher But does not a modal wait dialog assume, that everything, except this dialog window is unable to be clicked? I want to achieve both gui responsibility and live data visualization.
For now I have a following loop:

{
	QEventLoop loop;
	loop.connect(&workers_obj, SIGNAL(on_ready()), SLOT(quit()));
	QFuture<void> future;
	future = QtConcurrent::run(&this->workers_obj, &Worker::acquire_data);
	loop.exec();
}

Here I use on_ready() signal generated at the end of acquire_data() method. Everything works fine, until I want to cancel computation in worker thread. It leads to null-pointer access etc.

Christian Ehrlicher

You wanted to wait in the main dialog (for whatever reason) - I told you a solution.
Now you tell me you won't block the ui - so don't block it and connect a slot to the finished signal of the thread.

JohnCu

@Christian-Ehrlicher Ok, let me explain the situation one more time. I launch a function from a gui thread (gui class code). Then i need to wait for the function to finish its job, before i go to a subsequent line. However, mu gui thread event loop needs to be active all the time, in order to enable data plotting. I can not freeze gui thread, I need to have it responsible, but it needs to be waiting in a line of launching function from workers object. There should be something like "do subsequent line, but first perform action prompted by an indicated signal". For now it is handled in a loop mentioned before, but I know, that it is not an optimal solution...

Christian Ehrlicher

Again: connect a signal which is emitted from the thread when the first part is finished. Connect this signal to a slot in the gui thread which does the work you need to do in the gui thread which then emits another signal to do the next work in the thread.

JohnCu

@Christian-Ehrlicher Yes, I added such signal. But how should I check if the signal was emitted? I can not do a while loop, which constantly checks whether signal was emitted, because gui thread will be locked in such loop. Maybe you meant something else?

J.Hilk

@JohnCu you don't need to check that yourself

simply connect a slot to the signal, as soon as it is emitted and your main application is in no loop (infinite loop for example) the slot will be called!

JohnCu

@J-Hilk Yes, but my workers thread computation are a little bit time consuming, and I need to be sure, that before going to subsequent line in gui code (just after launching computation on workers thread) the computations are done. Your approach would work, if there is no more code in function, inside which workers thread method is launched.

J.Hilk

@JohnCu
I don't see the problem, YOU decide when the signal is emitted inside your code, don't you?

Emit the allDone signal when you're all done, and not before

JohnCu

@J-Hilk Yes, of course, there is no problem with emitting a signal from workers thread, I do it when computations are done. But, take look at this code from gui class:

emit onConnecttoHardware();

// if connected to hardware
QMessageBox::information(this, "Message", "Hardware connected");

First i launch action, then i show, that it is finished. however, after emitting a signal to start the action, the main thread goes straight to show QMessageBox, which can not be prompted before computations are done... I need to wait at the first line of this code to obtain allDone signal and have my gui responsible while waiting, because i need to plot some results, which are also prompted by a signal emitted from workers thread. If I do as you suggested, main thread displays messagebox immediately, before action finished (allDone signal is emitted)...

Unsolved QThread with multiple methods