Using QTimer in a QThread

[Cruz]

I don't get it. I want to start a thread that periodically executes a function using a timer.

Thread::Thread(QObject *parent) : QThread(parent)
{
	start();
}
 
void Thread::run()
{
	QTimer t;
	connect(&t, SIGNAL(timeout()), this, SLOT(tick()));
	t.start(100);
	//t.moveToThread(this);
	exec();
}
 
void Thread::tick()
{
	qDebug() << currentThreadId() << "tick()";
}
 
 
 
class ThreadTest : public QWidget
{
 
    Thread thread;
 
   ...
};
 
ThreadTest::ThreadTest(QWidget *parent)
    : QWidget(parent)
{
	qDebug() << QThread::currentThreadId() << "is the main thread.";
}

Thread::Thread(QObject *parent) : QThread(parent)
{
	start();
}

void Thread::run()
{
	QTimer t;
	connect(&t, SIGNAL(timeout()), this, SLOT(tick()));
	t.start(100);
	//t.moveToThread(this);
	exec();
}

void Thread::tick()
{
	qDebug() << currentThreadId() << "tick()";
}



class ThreadTest : public QWidget
{

    Thread thread;

   ...
};

ThreadTest::ThreadTest(QWidget *parent)
    : QWidget(parent)
{
	qDebug() << QThread::currentThreadId() << "is the main thread.";
}

To copy to clipboard, switch view to plain text mode 

And the output I get is:

0x12b8 is the main thread. 
0x12b8 tick() 
0x12b8 tick() 
0x12b8 tick() 
0x12b8 tick()
...

0x12b8 is the main thread. 
0x12b8 tick() 
0x12b8 tick() 
0x12b8 tick() 
0x12b8 tick()
...

To copy to clipboard, switch view to plain text mode 

Obviously, the timer is still running on the main thread. Using the moveToThread() doesn't make a difference. What's not right about this?

Changing the timer to an infinite loop with sleep() works though.

void Thread::run()
{
	forever
	{
		tick();
		sleep(1);
	}
}

void Thread::run()
{
	forever
	{
		tick();
		sleep(1);
	}
}

To copy to clipboard, switch view to plain text mode 

0xa04 is the main thread. 
0xfa0 tick() 
0xfa0 tick() 
0xfa0 tick() 
...

0xa04 is the main thread. 
0xfa0 tick() 
0xfa0 tick() 
0xfa0 tick() 
...

To copy to clipboard, switch view to plain text mode 

[MarekR22]

Problem is that you are doing that wrong!

class TestClass : QObject
{
    Q_OBJECT
public:
    TestClass(QObject *parent=0);
 
public slots:
    void tick();
};
 
TestClass::TestClass(QObject *parent) : QObject(parent)
{
     QTimer *t = new QTimer(this);
     connect(t, SIGNAL(timeout()), this, SLOT(tick()));
}
 
void TestClass::tick()
{
	qDebug() << currentThreadId() << "tick()";
}
 
// and somewhere
QThread *thread = new QThread(something);
TestClass *object = new TestClass(); // can't pass parent!!!!!!
object->moveToThread(thread);
thread->start();
 
....
delete object;
thread->WaitFor();
// delete thread;

class TestClass : QObject
{
    Q_OBJECT
public:
    TestClass(QObject *parent=0);
    
public slots:
    void tick();
};

TestClass::TestClass(QObject *parent) : QObject(parent)
{
     QTimer *t = new QTimer(this);
     connect(t, SIGNAL(timeout()), this, SLOT(tick()));
}

void TestClass::tick()
{
	qDebug() << currentThreadId() << "tick()";
}

// and somewhere
QThread *thread = new QThread(something);
TestClass *object = new TestClass(); // can't pass parent!!!!!!
object->moveToThread(thread);
thread->start();

....
delete object;
thread->WaitFor();
// delete thread;

To copy to clipboard, switch view to plain text mode 

[high_flyer]

Problem is that you are doing that wrong!

I don't see what he is doing wrong, in respect to the link you posted, except for what he tried in the second bit, which is wrong because you need to be in the "other thread" to call moveToThread().
The link you provided is talking about using moveToThread() wrong usage, and that you should not always use a subclass of QThread for your threads.
But there are many cases where this is the RIGHT way to go.
Even if from an OOP design point of view this is wrong, from implementation point of view this still should work (the original code with out moveToThread()).
And if it doesn't we need to understand why.

His code is almost the same as the example code QThread docs, just that he uses a QTimer and not a QTcpSocket.
Since the QTimer is allocated in run() its should have the thread affinity of the new thread, and from what the OP shows, it looks like its affinity is to the main GUI thread, which is surprising.

My guess is that it has to do with calling start() in the thread constructor.
Try removing the start() form the constructor, and call it normally from outside, see if this changes things.

[MarekR22]

Read carefully QObject::connect especially about last 5ht argument (Qt::ConnectionType-enum)!

---

Added after 5 minutes:

Note that your QThread belongs to main thread so default connection here is redirected through posting an event.
Read article I gave you at beginning to avoid other mistakes.

[high_flyer]

Read carefully QObject::connect especially about last 5ht argument (Qt::ConnectionType-enum)!

Right as if you read carfully you will discover that the default is Qt::AutoConnection, which is:

Qt::AutoConnection 0 (default) If the signal is emitted from a different thread than the receiving object, the signal is queued, behaving as Qt::QueuedConnection. Otherwise, the slot is invoked directly, behaving as Qt:irectConnection. The type of connection is determined when the signal is emitted.

EDIT:
However, now I see why the thread affinity is "wrong".
Its not - since the QThread object it self is in the main GUI thread, you are getting the main thread id.
The timer is correctly in the new thread, but the signal is evoking a slot in the main thread!

Note that your QThread belongs to main thread so default connection here is redirected through posting an event.

So, it has nothing to do with the fact the an object created in the QThread::run() should have the new thread affinity!

Read article I gave you at beginning to avoid other mistakes.

I have read it more than once, and I know it not from yesterday.
The article is good, but has nothing to do with the question in the first post! - unless you can show otherwise.

Conclution:
@OP: your code is behaving correctly.
Your timer IS in the new thread, but your slot is in the main thread.

[Cruz]

@OP: your code is behaving correctly.
Your timer IS in the new thread, but your slot is in the main thread.

Thanks, this is a very reasonable explanation. A simple thread.moveToThread(&thread) in the constructor of the gui did the trick and now the slot is executed in the worker thread.

[MarekR22]

NOOOOO!
Read article I gave you! NEVER EVER move thread to itself! You are corrupting handling of signals and slots from QThread which should be used to manipulate thread.
Look on my code!

[Cruz]

Ah sorry, I didn't study your instructions thoroughly enough. So if I understand right, moving the thread to itself means that the inherited signals and slots from QThread will also be handled in that event loop instead of in the main thread. So what? How does that corrupt things?

[high_flyer]

A simple thread.moveToThread(&thread) in the constructor of the gui did the trick and now the slot is executed in the worker thread.

Well, then comes the question of design.
Even though moving the the thread object to the thread the QThread object created is permited, its a clear sign of a design flaw.
Maybe if you explain what you are trying to do the correct way can be offered.

I want to start a thread that periodically executes a function using a timer.

If you need some functionality done in some time interval, then you better do things the other way around.
Have a timer in the main thread, and in the timeout slot create a new thread that does the work do be done.
You might want to have a look at QtConcurrent for a very simple way of doing that.
http://doc.qt.nokia.com/latest/qtconcurrent.html

[Cruz]

OK here is what I want to do. I have a GUI application that controls a robot via serial communication. The only requirement is simple. The communication loop with the robot should execute with a steady pace (every 10 ms), no matter how busy the gui keeps the CPU. The gui shows an interactive OpenGL 3D model of the robot, so when the user is rotating the model wildly and frequent redraws occur, the gui can get very cpu hungry.

[high_flyer]

In that case there are several options:
1. using your original construct you can use QtConcurrent::run() in the timer slot to run a method in which you handle the serial communication to the robot.
This will ensure that both your timer and the serial communication are not done in the GUI thread.
2. Put the serial communication functionality in a QObject, allocate that object also in the run() where your timer is allocated, and connect the timer timeout to a slot of your serial communication object.
3. handle the serial communication in run(), in a while loop, and sleep for 10ms every iteration, then you don't need signals slots and timers.
This you also can implement using QtConcurrent.

[Cruz]

When I implemented option 3. I found that sleeping in a loop gives me a much more steady timing than using a QTimer. Actually, I measure the runtime of each iteration using Windows' high performance counter and adjust the amount of time to sleep accordingly. This gives very satisfying results. Also, Sleep() seems to be a bit more accurate than msleep().

In option 1. I would use a thread to run the QTimer and then invoke another thread (or take one from a thread pool) each time I call QtConcurrent::run()? It sounds to me like it has one too many complications.

Option 2. can be used as a complementary technique along with either option 1 or 3. The only reason I would do that is to circumvent the "you are not doing it right" hack that was linked above. Personally, I find it very natural and intuitive to subclass QThread and do all thready things right there. And if it requires a moveToThread() call that is frowned upon, then so be it. Only if it really does mess something up I'm willing to introduce yet another object, but my question from my earlier post is still unanswered.

Also I'm hoping that using a QThread and giving it a high priority might improve the serialcom steadiness when the GUI is going nuts, but I haven't stress tested this assumption yet.

So, what's the advantage of using QtConcurrent::run() instead of a QThread::run()? Since QtConcurrent apparently takes threads from a pool, I assume it's faster if you start a lot of small threads all the time. But does it really matter for one thread that runs through in an infinite loop as in option 3.? The remark in the documentation saying "Note that the function may not run immediately; the function will only be run when a thread is available." is discouraging.

[wysota]

How steady is "steady"? If you are not running a real-time operating system then you can't get exact timings no matter how hard you try. Threading will not help you here in any way.

[high_flyer]

Only if it really does mess something up I'm willing to introduce yet another object,

If objects are designed correctly, such that each object encapsulates a specific functionality, they are correct to be used, even if they are many.
Otherwise, you get pieces of code sharing implementation of several functionalities, causing dependencies which is a bad thing.

Any way, I offered the three options, since I don't know your full requirements.
Each of them has advantages and disadvantages, dependent on your needs.
Pick the one you think best suits your needs.

So, what's the advantage of using QtConcurrent::run() instead of a QThread::run()?

The advantage is encapsulation, which is a very good thing when programming with OOP language.
And a slim code.

. Threading will not help you here in any way.

@wysota:
Threading helps him to avoid the GUI thread, which according to him, can get very busy, thus influencing the serial communication he needs.

[wysota]

@wysota:
Threading helps him to avoid the GUI thread, which according to him, can get very busy, thus influencing the serial communication he needs.

This is virtual safety. Threads are scheduled by the operating system. If it decides not to schedule the communication thread in a particular point in time, it won't. If the communication thread has little to do compared to the other thread, it will get less time from the operating system increasing a chance to lose the "steadiness" required. Only real-time operating systems give guarantees about times and scheduling.

[high_flyer]

only real-time operating systems give guarantees about times and scheduling.

True.
But if you put things in another thread you have more control over what is going on.
You can also prioritize the threads for example.
Of course you can't expect real time behavior in a non real time system (well, that too is a question of granularity requirements), but if you know the limits in which you are working, the accuracy of the system maybe acceptable.
At any rate, by putting the two pars in separate threads you are decoupling them, which I think was one of the main gaols here.

[wysota]

But if you put things in another thread you have more control over what is going on.

Again, that's "virtual" control.

You can also prioritize the threads for example.

Not really and not always.

Of course you can't expect real time behavior in a non real time system (well, that too is a question of granularity requirements), but if you know the limits in which you are working, the accuracy of the system maybe acceptable.

That's gambling. At any point another process may pop in and be given priority over yours so that your process is scheduled not often enough to reach the required communication frequency and you have totally no influence on that.

At any rate, by putting the two pars in separate threads you are decoupling them, which I think was one of the main gaols here.

Sure. Only that it's completely pointless.

[high_flyer]

Common witek, so what are you saying?
You should not implement tasks that need time resolutions in ms range in non RTOS?
I am sure you, just as I have, made many such usages of threads that are "real time like" under windows/linux.

[MarekR22]

Ah sorry, I didn't study your instructions thoroughly enough. So if I understand right, moving the thread to itself means that the inherited signals and slots from QThread will also be handled in that event loop instead of in the main thread. So what? How does that corrupt things?

Simple example:
lets say user starts thread by pressing button, so you connect buttons click signal with start slot of thread. Every think should work since is done by the book.
But it will not work since you made a queued connection (objects are in two diffident threads since you've moved thread to itself and auto-detection of queued connection was done) and signal to start thread is waiting in event loop of thread which is not started so you have dead lock.

[high_flyer]

@MarekR22:
You example is wrong.
If QThread object didn't call start(), its run() is not running, thus the read does not exist, and QThread object's affinity in to the thread in which is was created.
You can't moveToThread() that does not exist.