another beginner design question

[kshegunov]

@mzimmers said in another beginner design question:

If I do it your way, won't I need to change the slot for the run/slot button pressed every time it's pressed? (From startLoop -> stopLoop -> startLoop -> stopLoop etc.)

Really depends on your use case. I imagine 2 buttons - start and stop, so that's why I put two slots in the example. You can add an additional toggle slot, or merge them if you like.

I connected the button to startLoop() for now to test, and at least that much is working.

Good! I write code directly in the forum - no compiling, no testing, so it's good to know I haven't screwed up. ;)

@VRonin said in another beginner design question:

If you use the second method you probably want to make state atomic i.e. change State state; to std::atomic<State> state; to avoid race conditions in case you call isLoopRunning from another thread

Well, I'd discourage calling isLoopRunning from another thread to begin with, rather communicate the state change with a signal, otherwise I agree.

PS. The same considerations apply for the first piece of code - the one with the timer, but there you can't use an atomic integer.

[mzimmers]

VRonin - yes, that's how I had it originally. I thought kshegunov had separated them for a reason, but as he mentioned, he envisioned two buttons. I put it back now and it's working just fine.

This thread has been very helpful. I'm still curious, though: why is it considered better to avoid "while (true)" in favor of using the Qt constructs to effect a loop in the worker thread? It certainly is less simple.

Oh, another minor question: where/when does the text string "doStuffInLoop" (2nd parameter to invokeMethod()) get resolved to a pointer?

[VRonin]

@mzimmers said in another beginner design question:

why is it considered better to avoid "while (true)"

Because Qt relies queued on signals and slots to communicate between threads. If you block the event loop with a for(;;) you make the thread deaf of any incoming comunication from anywhere else; it can still talk (i.e. emit signals) though.

@mzimmers said in another beginner design question:

when does the text string "doStuffInLoop" (2nd parameter to invokeMethod()) get resolved to a pointer?

At runtime unfortunately, that's why the timer alternative is better as it is resolved at compile time. invokeMethod still uses pre-Qt5 string lookup to invoke a slot. You can inspect (most of) the code in the generated moc file

[mzimmers]

@VRonin: if I put a sleep inside my while (true), will the thread still block the event loop?

I tried kshegunov's first example, and I'm getting an error. Here's the source code for the worker thread:

Worker::Worker(QObject * parent)
    : QObject(parent), value(0)
{
    QObject::connect(&timer, &QTimer::timeout, this, &Worker::runSimulator);
}

void Worker::reportRunCount()
{
    emit counterChanged(value);
}

void Worker::toggleRunState()
{
    runState = ~runState;
    if (runState)
        timer.start();
}

void Worker::runSimulator()
{
    value++;
    QThread::msleep(100); // just to slow down the thread a little

    if (value % 10 == 0)
    {
        emit counterChanged(value);
    }
    if (runState)
        timer.start();
}

When I try to start the timer, I get this error:
QObject::startTimer: Timers cannot be started from another thread
I guess that the slot toggleRunState is being run from the context of the main thread. So...how do I initiate the first run of runSimulator()?

[kshegunov]

@mzimmers said in another beginner design question:

if I put a sleep inside my while (true), will the thread still block the event loop?

Every time.

I tried kshegunov's first example, and I'm getting an error. Here's the source code for the worker thread:

You forgot to parent the timer object to the worker object, take a close look at the example - the Worker class' constructor specifically.

PS.
If you want to slow down the looping, just fire the timer at longer intervals:

void Worker::toggleRunState()
{
    runState = ~runState;
    if (runState)
        timer.start(100); //< Every 100ms or so we'd get a timer event (i.e. the slot will be called).
}

PS2:
I can't tell from here what runState is, but don't use bit inversion. If it's a boolean use boolean negation, if it's a enum, use assignment, etc.

    runState = ~runState;

this just looks like you've shot yourself in the foot, but you still haven't felt it. ;)

[mzimmers]

Oops. Corrected and it works well now. Here are the pertinent routines:

void Worker::toggleRunState()
{
    runState = !runState;

    if (runState)
    {
        timer.start(TIMER_INTERVAL_MSEC);
    }
    else
    {
        timer.stop();
    }
}

void Worker::runSimulator()
{
    value++;

    if (value % 10 == 0)
    {
        emit counterChanged(value);
    }
    if (runState)
        timer.start(TIMER_INTERVAL_MSEC);
}

Now, if I wanted to let the worker thread run as fast as possible, would it just be a matter of setting the timer interval to 0? In other words, the timer would no longer be a timer, but a trigger for an iteration of runSimulator().

I ask this because once the simulator begins doing real work, I'll want it to go full throttle.

EDIT: and if the above is true, as I don't want to be flooding the widgets with updates, will I just implement a second timer to emit the counterChanged() signal?

[VRonin]

You don't need to re-start the timer in runSimulator. a QTimer is automatically repeated unless you call setSingleShot(true)

@mzimmers said in another beginner design question:

would it just be a matter of setting the timer interval to 0?

Yes. That tells the event loop to fire the slot connected to the timer as soon as it can

@mzimmers said in another beginner design question:

will I just implement a second timer to emit the counterChanged() signal?

Correct

[mzimmers]

VRonin - thanks for the information. Regarding the timer, and the worker loop, it's essential that one iteration of runSimulator finishes before another starts. Can I depend on this happening, or should I use the one shot, and repeat the timer at the end of runSimulator?

[VRonin]

@mzimmers said in another beginner design question:

one iteration of runSimulator finishes before another starts

Unless you call runSimulator from anothere thread (which you don't atm) there is phisically no way for the same thread to execute a (non recursive) function while the same did not finish yet

[CroCo]

@mzimmers
In this kind of scenarios, you should use timer to call a function periodically. NEVER EVER create a while-loop insider your GUI, this will freezes your app.
A suggested solution is

// dialog.cpp
#include "dialog.h"
#include "ui_dialog.h"
#include <QTimer>
#include <QDebug>

Dialog::Dialog(QWidget *parent) :
    QDialog(parent),
    ui(new Ui::Dialog),
    runFlag(false)
{
    ui->setupUi(this);
    QTimer *timer = new QTimer(this);
    connect(timer, SIGNAL(timeout()), this, SLOT(onWorkerThread()));
    timer->start(1000); // delay
}

Dialog::~Dialog()
{
    delete ui;
}

void Dialog::onWorkerThread()
{
    if (runFlag){
          qDebug() << "Worker Thread is running...";
    }else{
          qDebug() << "Worker Thread is NOT running...";
    }
}

void Dialog::on_ActivateButton_clicked()
{
    runFlag=true;
}

void Dialog::on_DisactivateButton_clicked()
{
    runFlag=false;
}

//dialog.h
#ifndef DIALOG_H
#define DIALOG_H

#include <QDialog>

namespace Ui {
class Dialog;
}

class Dialog : public QDialog
{
    Q_OBJECT

public:
    explicit Dialog(QWidget *parent = 0);
    ~Dialog();

private slots:
    void onWorkerThread();
    void on_ActivateButton_clicked();
    void on_DisactivateButton_clicked();

private:
    Ui::Dialog *ui;
    bool runFlag;
};

#endif // DIALOG_H

//main.cpp
#include "dialog.h"
#include <QApplication>

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    Dialog w;
    w.show();

    return a.exec();
}

The GUI is