Qt MainWindow is not updating in multithreaded application
-
got a tip:
I have to return the event handler from the upper classes:
@return QMainWindow::event(event);@ -
There are a number of problems in your code, but in the end they can all be summed up by: "You're doing it wrong":http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/. I am assuming the code you posted is not everything, otherwise the signals defined in the QThread subclass would be obsolete. Your QThread subclass doesn't have a working event loop because you forgot to start it by calling exec() in the run() functions reimplementation. This results in the fact that your subclass cannot emit or receive signals. If this was fixed still all the signals emited by the QThread subclass are emitted in the context of the main thread. But in the end it all boils down to the advice to not subclass QThread but to use "the worker object approach":http://qt-project.org/wiki/QThreads_general_usage. Alternatively you could implement a subclass of "QRunnable":http://qt-project.org/doc/qt-4.8/qrunnable.html. This would be more similar to your current implementation but less error prone.
For more informations about multithreading in Qt you can have a look at these links:
"Thread basics":http://doc-snapshot.qt-project.org/5.0/qtcore/thread-basics.html
"Thread support in Qt":http://doc-snapshot.qt-project.org/5.0/qtcore/threads.htmlUnfortunatly the above two links do still propagate the old subclassing approach. An attempt to correct this by a community member (JKSH see "link to discussion":http://qt-project.org/forums/viewthread/22205 ) didn't make it into the 5.0 version. You can read his draft "here":https://docs.google.com/file/d/0B71n5SNZeRJyNzZ6U0tVUUM5bUk/edit?pli=1.
-
thanks for your detailed information! i was thinking qt threading is similar to other implementations styles so i was creating a subclass. I will take your advice and will rewrite the classes for the nfc part (i was also not very happy with this approach)
-
Of course you can subclass QThread, but it has some pitfalls (which you unfortunatly all discovered). So using the worker object approach or another threading purpose class (e.g. QRunnable, QtConcurrent) can safe you a lot of headaches.
-
As KA510 said, you can subclass, but I think you'd be better off doing what the blog said.
Also a good read http://qt-project.org/wiki/QThreads_general_usage -
Actually, a thread without an event loop can send signals, just not receive them. And these signals are emitted in the context of the thread, not of the main thread.
The documentation has in fact been "modified":/doc/qt-5.0/qtcore/qthread.html, and has been "modified":https://codereview.qt-project.org/#change,45271 again afterwards. The worker approach is now the prime example in the docs, but the subclassing of QThread is also still there. For some use cases, it is just a better approach.
Note that there is another issue in the code: the stop flag won't work. The check for it will be optimized out. I'd suggest using a QAtomicInt for the flag, or at least make the bool volatile.
-
thank you for your help guys!
I changed the class to a worker object with will be moved to the thread.now i have another problem:
I have i QTimer which should come around 500ms (is not a big deal if it is a little bit later).
But with the new implementation i get no event from the timer anymore. I think the problem is, what Andre was writing, that the new thread has no event loop.some code:
@NFC_Thread::NFC_Thread(NFC_Reader * Reader)
{
mReader = Reader;
mError = 0;
mStopPolling = false;if(mpTimer != 0) { delete mpTimer; } mpTimer = new QTimer; QObject::connect(mpTimer, SIGNAL(timeout()), this, SLOT(ResetTargets())); mpTimer->start(mTimerMS);
}
void NFC_Thread::doPolling()
{
mError = 0;
mStopPolling = false;if(mpTimer != 0) { delete mpTimer; } mpTimer = new QTimer; QObject::connect(mpTimer, SIGNAL(timeout()), this, SLOT(ResetTargets())); mpTimer->start(mTimerMS);
while(!mStopPolling)
{
nfc_target Target;
mError = nfc_initiator_poll_target(mReader->GetDevice(), nmModulations, szModulations, mPollNr, mPollPeriod, &Target);
if(mError > 0)
{
cout << "NFC: found target" << endl;
}
#warning Bug in driver: Timeout generate a NFC_EIO Error, 'https://code.google.com/p/libnfc/issues/detail?id=224'
else if(mError > 0)
{
cout << "NFC: Error" << endl;
mStopPolling = true;
}
else
{
cout << "NFC: no target found" << endl;
}
}
}void NFC_Thread::ResetTargets()
{
for(QList<nfc_target>::iterator iter = mTargets.begin(); iter != mTargets.end(); iter++)
{
emit TargetRemoved(*iter);
}
mTargets.clear();
mpTimer->start(mTimerMS);
Debug("NFC: Reseting List with timeout!");
}@I create the Thread object in a other class with this:
@
mpThread = new NFC_Thread(this);
mpRunningThread = new QThread;
mpThread->moveToThread(mpRunningThread);connect(mpRunningThread, SIGNAL(started()), mpThread, SLOT(doPolling())); connect(mpThread, SIGNAL(finished()), mpRunningThread, SLOT(quit())); connect(mpThread, SIGNAL(finished()), mpThread, SLOT(deleteLater())); connect(mpRunningThread, SIGNAL(finished()), mpRunningThread, SLOT(deleteLater()));
@
and start the thread with this:
@mpRunningThread->start();@now the question is, where should i put the event loop...
the main problem is: i want to write some classes around a lib, to get support of signals and slots. -
By default a QThread already has its own eventloop. The default implementation of run() already calls exec(). Your problem is the busy loop :
@
while(!mStopPolling)
{
nfc_target Target;
.
.
.
}
@
while this is running your code does not return to the eventloop. So no signals are handled. Infinite loops or polling in loops is not a good idea in an event driven framework like Qt. -
Ok! But i have this libnfc, and i can not change it, so i want to write a wrapper around it (from poll to event driven).
what is the best option for me to do this? -
don't loop forever, use a timer to poll every n ms instead. Returning quickly when there is nothing.
-
How long do you estimate does this call need to return ?
@
mError = nfc_initiator_poll_target(mReader->GetDevice(), nmModulations, szModulations, mPollNr, mPollPeriod, &Target);
@ -
Sometimes a tight loop is a good solution, but then you just have to make sure that you don't use any slots. If you're in a loop, and you need a timer-like solution, you could also just check a [[doc:QElapsedTimer]] in every iteration.
-
The poll needs currently 5 sek to return (still a bug in the lib), so it is like really long. I think the best is, to wait until the bug is fixed, making the poll short as possible and i can change the howl algorithm to something better. the timer is just there because the poll needs so long. Example: If i am returning a tag from the reader and putting the tag again in the 5 sek, the lib does not recognizing that there is a "new" tag.
-
KA510; I don't know how long it takes to return from nfc_initiator_poll_target(), there is no way to know without measuring since I don't know that call.
I suggest using a timer and start it just before the call, and print 'elapsed()' directly after the call completed.
Make sure you do it in different situations and repeatedly to get a good idea how long it takes on average.
-
[quote author="Thomas Zander" date="1360915643"]KA510; I don't know how long it takes to return from nfc_initiator_poll_target(), there is no way to know without measuring since I don't know that call.
I suggest using a timer and start it just before the call, and print 'elapsed()' directly after the call completed.
Make sure you do it in different situations and repeatedly to get a good idea how long it takes on average.[/quote]
I wasn't asking you but the original poster of course. Why would you think that question was directed to you?
On topic: If the call takes 5 seconds you can not expect a timer with a 500 ms timeout to work as expected ...
Instead of the loop just call the slot doProlling() at start of the thread (as you do now, but without the loop). When the nfc call returns emit a signal connected to a slot in the worker object, which then checks all your abort flags and such (you could also update your targets here) and then call the doPolling slot again if the abort flag is not true. This way your thread is still blocked while executing the nfc blocking call but atleast returns to the event loop right after that. -
this sounds good to me, i will give change it to this structure.
I will also change to another lib for the nfc part because the reader is not very well supported.