(seemingly) simple Qt app: creating a progress bar

[mzimmers]

bq. Did you happen to call exit instead of return?

In which routine: workerThread::run()? I don't call anything there; I just let the routine run to completion:

@void WorkerThread::run()
{
long curr, percent;
string myStr;
static long counter = 0;

fileSize = getFileSize(myFile);

while (myFile.good() && !endSignaled)
{
myFile >> myStr;
curr = myFile.tellg();

// special handling for end of file.

if (curr != -1)
percent = curr * 100 / fileSize;
else
percent = 100;

counter++;
if ((counter % 20 == 0) || (percent == 100))
emit percentChanged(percent);
}
myFile.close();
}
@

[mzimmers]

I'm stepping through the program in the debugger. From what I'm seeing, I'd guess that the requestFinish() slot is never getting called. When I hit close, then yes, the program continues to run in the background until EOF is hit, at which point run() ends naturally. (This brings up a side point: is it considered OK to allow the worker thread to continue until the confirmation button is hit, or should it pause when the close is hit?)

Here's the c'tor of my MainWindow, in case it helps:

@MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent)
{
m_bar = new QProgressBar (this);
m_button = new QPushButton (this);
m_worker = new WorkerThread ();
m_thread = new QThread ();

setupWidgets();

connect(m_button,
SIGNAL(clicked()),
this,
SLOT(onQuitButtonClicked()));

connect(this,
SIGNAL(requestWorkerToFinish()),
m_worker,
SLOT(requestFinish()));

// run the run() method of the worker object once the thread has started

QObject::connect(m_thread,
SIGNAL(started()),
m_worker,
SLOT(run()));

// enable the updating of the progress bar

QObject::connect(m_worker,
SIGNAL(percentChanged(int)),
this,
SLOT(setBarValue(int)));

m_worker->moveToThread(m_thread);
m_thread->start();
}
@

[mzimmers]

I've run through the program in the debugger a few times. I'm pretty sure that the
m_worker->requestFinish() slot doesn't execute until the m_thread->wait() has timed out. Is it possible that the wait is somehow blocking the delivery of the slot?

[foxyz]

Usually ,This link error means you have a member function defined but not implemented it

[foxyz]

I'm not sure why you don't want to use just what the Qt provided. I usually use QProgessBar and QCoreApplication::processEvents to do this. like:
@
QProgressBar bar(...);
do {
do something;
bar.setValue();

QCoreApplication::processEvents ();

}
@

Edit: Code formatting. Please wrap code in @ tags. -mlong]

[mzimmers]

foxyz: I'm not sure I understand you...are you suggesting a different approach to what has been posted in this thread?

I'm pretty sure the current implementation is close to working; I just need to find what's causing the signal not to be delivered in a timely fashion.

[mlong]

Foxyz's proposing a non-threaded approach where you embed manual event processing within your working logic. While it is technically possible to do that, I believe the threaded approach you're taking currently is a much cleaner solution.

[mzimmers]

From a novice's point of view, they both seem to have their advantages. It looks (from a first glance) that foxyz's approach would result in fewer signals/slots/connects, and that's where I seem to be stumbling right now. On the other hand, the approach that mlong and Volker are showing me is HIGHLY transferable to an existing program. Once I get this working right, it seems to be a matter of putting my (non-Qt) main code into the run() routine, establishing the sync between the worker and the UI threads, and that's it.

Any idea on my question above?

Thanks.

[mlong]

While the embedded event processing has a certain appeal to it, you still are limited by the amount of work being done in between calls to processEvents(), and in certain situations you have to make sure that your GUI, etc, is not able to reenter your worker code because of getting some sort of event to restart it, otherwise things can get hairy.

As for the other question, I don't have anything helpful to add at the moment...

[mzimmers]

OK, thanks, mlong. Any tips on putting other breakpoints or qDebug telltales anywhere for more information on what's going on?

BTW, the one remaining line of code in main.cpp is a connect() that I commented out:

@// app.connect(&app, SIGNAL(lastWindowClosed()), &app, SLOT(quit()));
@

The presence or absence of this call isn't relevant to the current problem, is it?

[goetz]

Can you put your current code into a ZIP for downloading? Just replace the actual workload in the run method with some dummy loop. It's hard to say what's going on without seing the whole picture.

[mzimmers]

Sure, I can do that. Where would you like me to put it? Or, I can email it to you.

Thanks, Volker.

EDIT: Just remembered this site. Here's the zip:

http://www.mediafire.com/download.php?qp6lelkebbni20i

Thanks again.

[goetz]

Ok, I made it run as expected. First, I had a mistake in my outline of the onQuitButtonClicked() method. The call to close() wasn't conditional, it was always called, that's why your app close, even if you clicked on "no".

To make it run:

add a signal workDone() to WorkerThread:
@
signals:
void percentChanged(int percent);
void workDone();
@

emit the signal at the end of the run method:
@

void WorkerThread::run()
{
qDebug() << "WorkerThread::run(): STARTED";

// your code goes here.

myFile.close();
qDebug() << "WorkerThread::run(): finished";
emit workDone();

}
@

in the MainWindow constructor connect the signal:
@
MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent)
{
// ...

connect(m_worker,
        SIGNAL(workDone()),
        m_thread,
        SLOT(quit()));
// ...

}
@

modify MainWindow::onQuitButtonClicked() to this version
@
void MainWindow::onQuitButtonClicked()
{
QMessageBox::StandardButton answer = QMessageBox::question(
this,
tr("Quit applicaton?"),
tr("Do you really want to quit the application?"),
QMessageBox::Yes | QMessageBox::No,
QMessageBox::No
);

if (answer != QMessageBox::No)
{
    if(m_thread->isRunning()) {
        qDebug() << "thread is running - trying to terminate it";
        emit requestWorkerToFinish(); // no direct call!
        bool finishOk = m_thread->wait(10000);
        if (!finishOk)
        {
            qDebug() << "WARNING: request to finish thread timed out!";
            m_thread->terminate();
            m_thread->wait();
        }
    } else {
        qDebug() << "Thread not running";
    }
    close();
}

}
@

Some additional hints:

At the end of a method that has no (void) return type, you do not need to call return explicitly.

All the QObjects you create using new() should have a parent:

@
MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent)
{
m_bar = new QProgressBar (this);
m_button = new QPushButton (this);
m_worker = new WorkerThread (this);
m_thread = new QThread (this);

setupWidgets();
// ...

}
@

If they do have a parent, you do not need to delete them manually, so remove the deletes from the destructor:

@
MainWindow::~MainWindow()
{
}
@

And to have a nice layout, you should put everything into a layout or use a Qt Designer based form.

[mzimmers]

Hmm...it's not working as I'd expect it to. Here's the app output when I push the "yes" button:

bq. WorkerThread::run(): STARTED
thread is running - trying to terminate it
WorkerThread::run(): finished
reached end of file.
WARNING: request to finish thread timed out!
The program has unexpectedly finished.

Based on my telltales in run(), it appears that the slot requestFinish() isn't called until run() is finished. I added a telltale to requestFinish(), and here's the output I got:

bq. WorkerThread::run(): STARTED
thread is running - trying to terminate it
reached end of file.
WorkerThread::requestFinish() called.
WorkerThread::run(): finished
WARNING: request to finish thread timed out!
The program has unexpectedly finished.

Another issue: this line in mainwindow.cpp doesn't compile:

@ m_worker = new WorkerThread (this);
@

Perhaps this is because my c'tor didn't have an argument, so I modified its declaration to be this:

@ WorkerThread(QObject * parent = 0);
@

Now it compiles (though I get a compiler warning that parent is never used). Is this OK?

[goetz]

Nope, you must also change the constructor implementation:

@
WorkerThread::WorkerThread(QOjbect parent)
: QObject(parent)
{
// ...
}
@

It is always a good idea to always add the parent parameter to any QObject subclass. If you use the new class wizard in Qt Creator, it's even done automatically, if you state that it is a QObject subclass :-)

Regarding the finish: I need to prepare a test case, my files were far too small to add reasonable workload to the worker thread. It was always finished before I had the chance to click the button :)

[mzimmers]

Hmm, I now get a run-time warning/error:

bq. QObject::moveToThread: Cannot move objects with a parent

The documentation on QObject confirms this.

Also:

bq. And to have a nice layout, you should put everything into a layout or use a Qt Designer based form.

...meaning, that I don't use Designer to create the UI, but I somehow move my existing stuff into Designer context? If you could point me at a page that talks more about this, I'll get started on it.

Thanks.

[goetz]

Ah, I'm the cause of the error. Just replace this one

@
m_worker = new WorkerThread (this);
@

with this one:

@
// The WorkerThread has no parent
m_worker = new WorkerThread;
@

In this case you must delete the worker in the destructor to avoid a memory leak:

@

MainWindow::~MainWindow()
{
delete m_worker;
}
@

For the layout: You can either use Qt Designer or just put everything into a layout manally, the [[Doc:QLayout]] docs and subclasses have examples.

[mzimmers]

OK, fixed that...but I'm still having the same problem as described above: I hit "yes" and...it continues on its way. Only after the workerThread gets to the end of the file, is requestFinish() called.

Maybe I'm not understanding how this should work -- once the requestWorkerToFinish signal is sent, the corresponding slot should execute immediately, right? So, my worker loop should terminate very quickly. That's what I was expecting, anyway.

[goetz]

Sorry, I didn't have the time yet to prepare a test case. It'll last some more time...

[mzimmers]

No sweat, Volker...I appreciate your help. Would you like a fresh copy of the source code? Mine should be about the same as yours, but...you never know.