another beginner design question



  • ...and by "design" I mean the application, not the UI...

    My UI thread has a pushbutton to signal the worker thread. The worker's slot will toggle a run state flag. The worker should behave like this:

    while (true)
    {
       if (runFlag)
       {
          do something
       }
       else
       {
          sleep for a bit
       }
    }
    

    So, it's like the first time this button is pressed, it starts the above routine, and then after that, it needs to signal another routine to toggle the run flag. I can think of a few ways to do this, but they all seem hacky. Can someone please advise on the preferred method of doing this?

    Thank you.


  • Qt Champions 2016

    To run a loop through the event loop use:

    QMetaObject::invokeMethod(object, "doSomethingSlot", Qt::QueuedConnection);
    

    or

    QTimer::singleShot(0, std::bind(object, &Class::doSomethingSlot));
    

    or

    QTimer loopTimer;
    QObject::connect(&loopTimer, &QTimer::timeout, object, &Class::doSomethingSlot);
    loopTimer.start(0);
    

    or a variation thereof. In 99% of cases you don't want to run a thread without an event loop and in 100% of those cases you don't want to block the event loop's message processing. In the above snippets object is an instance of the QObject derived class Class - the worker object.



  • @kshegunov said in another beginner design question:

    In 99% of cases you don't want to run a thread without an event loop and in 100% of those cases you don't want to block the event loop's message processing.

    Repost. just because it's the single most important takeaway here.

    basically while(true) is almost always a bad idea in this context



  • So, the QMetaObject::invokeMethod presumably goes in main() somewhere after I've connected the widget/worker signals and slots, yes?

    If a while (true) is a bad idea, how do I code a worker thread that runs continuously (pausing to check the run flag)?


  • Qt Champions 2016

    @mzimmers said in another beginner design question:

    So, the QMetaObject::invokeMethod presumably goes in main() somewhere after I've connected the widget/worker signals and slots, yes?

    Nope, in the loop body.

    If a while (true) is a bad idea, how do I code a worker thread that runs continuously (pausing to check the run flag)?

    class Worker : public QObject
    {
        Q_OBJECT
    
    public:
        Worker(QObject * parent = nullptr)
            : QObject(parent), loopTimer(this)
        {
            QObject::connect(&loopTimer, &QTimer::timeout, this, &Worker::doStuffInLoop);
        }
    
        bool isLoopRunning() const
        {
            return loopTimer.isActive();
        }
    
    public slots:
        void startLoop()
        {
            loopTimer.start(0);
        }
    
        void stopLoop()
        {
            loopTimer.stop();
        }
    
    private slots:
        void doStuffInLoop()
        {
            // Something to do in the loop body
        }
    
    private:
        QTimer loopTimer;
    };
    

    is the simplest implementation for such a thing. Connect your UI signals to startLoop and stopLoop and that's pretty much it.

    OR with invokeMethod:

    class Worker : public QObject
    {
        Q_OBJECT
    
        enum State { Active, Inactive };
    
    public:
        Worker(QObject * parent = nullptr)
            : QObject(parent), state(Inactive)
        {
        }
    
        bool isLoopRunning() const
        {
            return state == Active;
        }
    
    public slots:
        void startLoop()
        {
            if (state == Active)
                return;
    
            state = Active;
            QMetaObject::invokeMethod(this, "doStuffInLoop", Qt::QueuedConnection);
        }
    
        void stopLoop()
        {
            state = Inactive;
        }
    
    private slots:
        void doStuffInLoop()
        {
            // Something to do in the loop body
    
            // Loop back ;)
            if (state == Active)
                QMetaObject::invokeMethod(this, "doStuffInLoop", Qt::QueuedConnection);
        }
    
    private:
        State state;
    };
    


  • kshegunov: thank you for the detailed answer. I am going to use the 2nd example you gave, because I understand it better, and I'd prefer to eliminate the timer from the logic.

    I notice that you implemented two slots for run control: startLoop() and stopLoop(). My thinking was to have only one, to toggle the run state. 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.)

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

    Thanks again for the assistance.



  • @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.)

    It's trivial to merge the two methods together in a toggleLoop(bool start) and do different things based on the boolean passed.

    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


  • Qt Champions 2016

    @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.



  • 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?



  • @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



  • @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()?


  • Qt Champions 2016

    @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. ;)



  • 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?



  • 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



  • 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?



  • @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



  • @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

    0_1500683973309_Capture.PNG


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.