Correct usage of QThread

[Patrik]

Hi, I'm developing a qt application that interfaces with an external hardware device. My application consists of just 2 start/stop buttons and a listwidget used to display data received from the device. Since the data acquisition from the device requires me to call a blocking function, I've decided to put the call in a separated thread because data acquisition can be stopped only by calling another function from another thread. So I've created this QThread subclass:

class InventoryThread : public QThread
{
    Q_OBJECT
 
private:
    int handle;
    RFID_18K6C_INVENTORY_PARMS * inventoryParms;
    RFID_18K6C_INVENTORY_PARMS * SetInventoryParams(const RFID_RADIO_HANDLE handle, const int cycles);
 
public:
    explicit InventoryThread(QObject *parent = 0, int h = 0);
 
protected:
    void run();
 
signals:
 
public slots:
 
};

class InventoryThread : public QThread
{
    Q_OBJECT

private:
    int handle;
    RFID_18K6C_INVENTORY_PARMS * inventoryParms;
    RFID_18K6C_INVENTORY_PARMS * SetInventoryParams(const RFID_RADIO_HANDLE handle, const int cycles);

public:
    explicit InventoryThread(QObject *parent = 0, int h = 0);

protected:
    void run();

signals:

public slots:

};

To copy to clipboard, switch view to plain text mode 

The relevant implementation is this:

InventoryThread::InventoryThread(QObject *parent, int h) :
    QThread(parent)
{
    handle = h;
}
 
void InventoryThread::run()
{
    RFID_18K6CTagInventory(handle, SetInventoryParams(handle, 5), 0);
}

InventoryThread::InventoryThread(QObject *parent, int h) :
    QThread(parent)
{
    handle = h;
}

void InventoryThread::run()
{
    RFID_18K6CTagInventory(handle, SetInventoryParams(handle, 5), 0);
}

To copy to clipboard, switch view to plain text mode 

The main window code will call InventoryThread::start() when I click on the start button, then RFID_18K6CTagInventory will be called (which is a blocking function). This function can be stopped only by calling RFID_RadioCancelOperation in the main thread. Now, I'm not sure if my approach is correct, the documentation about QThread its main loop il pretty confusing.

What should I do?

[MarekR22]

http://labs.qt.nokia.com/2010/06/17/...oing-it-wrong/

[Patrik]

Ok, that clarified some things. I'm still having a little issue. My classes look as follows:

class InventoryThread : public QObject
{
    Q_OBJECT
 
private:
    int handle;
    QListWidget * list;
    RFID_18K6C_INVENTORY_PARMS * inventoryParms;
    RFID_18K6C_INVENTORY_PARMS * SetInventoryParams(const RFID_RADIO_HANDLE handle, const int cycles);
    QTimer * timer;
 
public:
    explicit InventoryThread(QObject *parent = 0, int h = 0, QListWidget *lst = 0);
 
signals:
 
public slots:
    void DoInventory();
};
 
InventoryThread::InventoryThread(QObject *parent, int h, QListWidget * lst) : QObject(parent)
{
    handle = h;
    list = lst;
    timer = new QTimer(this);
    list->connect(timer,SIGNAL(timeout()),SLOT(clear()));
    //connect(timer,SIGNAL(timeout()),list,SLOT(clear()));
    timer->setSingleShot(false);
    timer->start(1500);    
}
 
void InventoryThread::DoInventory()
{
    printf("Doing inventory\n");
    RFID_18K6C_INVENTORY_PARMS * parms = SetInventoryParams(handle, 5);
    printf("Tag Inventory returned %d\n", RFID_18K6CTagInventory(handle, parms, 0));
    free(parms);
}

class InventoryThread : public QObject
{
    Q_OBJECT

private:
    int handle;
    QListWidget * list;
    RFID_18K6C_INVENTORY_PARMS * inventoryParms;
    RFID_18K6C_INVENTORY_PARMS * SetInventoryParams(const RFID_RADIO_HANDLE handle, const int cycles);
    QTimer * timer;

public:
    explicit InventoryThread(QObject *parent = 0, int h = 0, QListWidget *lst = 0);

signals:

public slots:
    void DoInventory();
};

InventoryThread::InventoryThread(QObject *parent, int h, QListWidget * lst) : QObject(parent)
{
    handle = h;
    list = lst;
    timer = new QTimer(this);
    list->connect(timer,SIGNAL(timeout()),SLOT(clear()));
    //connect(timer,SIGNAL(timeout()),list,SLOT(clear()));
    timer->setSingleShot(false);
    timer->start(1500);    
}

void InventoryThread::DoInventory()
{
    printf("Doing inventory\n");
    RFID_18K6C_INVENTORY_PARMS * parms = SetInventoryParams(handle, 5);
    printf("Tag Inventory returned %d\n", RFID_18K6CTagInventory(handle, parms, 0));
    free(parms);
}

To copy to clipboard, switch view to plain text mode 

In my main class I have

class QThreadEx : public QThread
{
protected:
    void run() { exec(); }
};
 
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
    QObject::connect(this->ui->start,SIGNAL(clicked()),this,SLOT(StartInventory()));
    invThread = new InventoryThread(0, handle, this->ui->listWidget);
    thr = new QThreadEx();
    invThread->moveToThread(thr);
    invThread->connect(thr,SIGNAL(started()),SLOT(DoInventory()));
}

class QThreadEx : public QThread
{
protected:
    void run() { exec(); }
};

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
    QObject::connect(this->ui->start,SIGNAL(clicked()),this,SLOT(StartInventory()));
    invThread = new InventoryThread(0, handle, this->ui->listWidget);
    thr = new QThreadEx();
    invThread->moveToThread(thr);
    invThread->connect(thr,SIGNAL(started()),SLOT(DoInventory()));
}

To copy to clipboard, switch view to plain text mode 

Finally, when I click on my start button I call thr->start() and when I click on the end button I call thr->quit(). This scheme works only for the first time. If I click on the start button again the thread will not launch (I can't see the printf's output on my terminal). Obviously I'm still missing something. How can I launch again my thread after quitting it (or after DoInventory() slot ended its work)?

[MarekR22]

Rename InventoryThread to InventoryWorker or something similar.
You really don't need QThreadEx.

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
    ui->setupUi(this);
 
    invThread = new QThread(this);
    worker = new InventoryWorker; // parent is forbidden here
    worker->moveToThread(invThread);
    connect(ui->start,SIGNAL(clicked()), 
               invThread, SIGNAL(startTimer())); // add slot startTimer in InventoryWorker
    invThread->start();
}
 
MainWindow::~MainWindow() {
    worker->deleteLater();
    invThread->terminate();
    invThread->waitFor(3000);
}
 
InventoryWorker::InventoryWorker(QObject *parent, int h, QListWidget * lst) : QObject(parent)
{
    handle = h;
    list = lst;
    timer = new QTimer(this);
    list->connect(timer,SIGNAL(timeout()),SLOT(clear()));
    connect(timer,SIGNAL(timeout()),list,SLOT(clear()));
    timer->setSingleShot(false);
}
 
InventoryWorker::startTimer() {
    timer->start(1500);
}

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    invThread = new QThread(this);
    worker = new InventoryWorker; // parent is forbidden here
    worker->moveToThread(invThread);
    connect(ui->start,SIGNAL(clicked()), 
               invThread, SIGNAL(startTimer())); // add slot startTimer in InventoryWorker
    invThread->start();
}

MainWindow::~MainWindow() {
    worker->deleteLater();
    invThread->terminate();
    invThread->waitFor(3000);
}

InventoryWorker::InventoryWorker(QObject *parent, int h, QListWidget * lst) : QObject(parent)
{
    handle = h;
    list = lst;
    timer = new QTimer(this);
    list->connect(timer,SIGNAL(timeout()),SLOT(clear()));
    connect(timer,SIGNAL(timeout()),list,SLOT(clear()));
    timer->setSingleShot(false);
}

InventoryWorker::startTimer() {
    timer->start(1500);
}

To copy to clipboard, switch view to plain text mode 

[Patrik]

How does your code call InventoryWorker:: DoInventory()? To put it simply what I need is:

every time ui->start emits clicked() then run InventoryWorker:: DoInventory() in a different thread

[MarekR22]

This code I gave you is not complete. I wrote it just to show you a pattern. I don't know what you code do I don't need to.

[Patrik]

Ok, after reading it twice I got it. Tank you all, you've been really helpful