need ideas for list/table implementation



  • OK, I think I'm almost there. Now, what's the recommended behavior of the slot that receives the quit button signal? exit()? quit()? Something else?


  • Qt Champions 2017

    The quit button signal you should connect to QThread::quit, just as you had shown above. That's the way to do it.



  • Hmm...it's not working. I read this in the QThread page:

    This function [quit] does nothing if the thread does not have an event loop.

    Do I need to explicitly declare/start an event loop?



  • If you don't subclass QThread the native QThread::run starts an event loop. Can you show us your current Worker::process implementation?



  • Sure:

    void Worker::process()
    {
        int len;
    //    running.ref(); // set to value of 1
    //    while (true)
    //    {
            len = sm.recv(buffIn, sizeof(buffIn));
            if (len >= 0)
            {
                buffIn[len] = '\0';
                Message msg(nullptr, buffIn);
                emit(newMessage(&msg));
    
                // if message is a heartbeat, send an ack.
                if (msg.getType() == MSG_HEARTBEAT)
                {
                    sendHeartbeatAck();
                    updateDeviceTable(msg);
                }
            }
            else
            {
                SocketState ss = sm.getSocketState();
                if (ss == SOCKET_DISCONNECTED)
                {
                    //sm.init();
                }
            }
    //        Sleep(10);
    //    }
        //emit reachedEndOfThread();
    }
    
    


  • emit newMessage(&msg) is a race condition, msg might (and almost surely does) go out of scope before the slots connected to the signal can use the argument



  • Noted, but I don't think that's causing my quit button to be ignored, is it?

    I just realized my process() routine is only called once. Is there something wrong with my pollTimer?

    Worker::Worker(QObject *parent) : QObject (parent), devices(parent), pollTimer(this)
    {
        QObject::connect(&pollTimer, &QTimer::timeout, this, &Worker::process);
        pollTimer.setInterval(100);
    }
    void Worker::start()
    {
        pollTimer.start();
    }
    


  • This is weird... can you post the entire worker.h and worker.cpp?

    Hmm...it's not working.

    How are you testing if it's working or not?

    P.S.
    No need to give parents to stack-allocated QObjects like devices or pollTimer
    edit: wrong in this case, without a parent the objects will belong to the thread calling Worker constructor and not be moved with it



  • worker.h:

    #ifndef WORKER_H
    #define WORKER_H
    
    #include "winsock2.h"
    #include "ws2tcpip.h"
    
    #include <QAtomicInt>
    #include <QObject>
    //#include <QThread>
    #include <QTimer>
    
    #include "constants.h"
    #include "devices.h"
    #include "message.h"
    #include "socket.h"
    
    namespace Ui
    {
    class Widget;
    }
    
    class Worker : public QObject //QThread
    {
        Q_OBJECT
    
    private:
        SOCKET sock;
        SocketMC sm;
        int myErrno;
        Message msg;
        char msgOut[1024];
        char buffIn[1024];
        Devices devices;
        QTimer pollTimer;
    
        void sendHeartbeatAck();
        void updateDeviceTable(Message msg);
    public:
        explicit Worker(QObject *parent = nullptr);
        ~Worker();
        void readSocket();
        void doQuit();
    signals:
        void newMessage(Message *msg);
        void finished();
    //    void reachedEndOfThread(int rc = 0);
    
    public slots:
        void start();
        void process();
        void sendLedPatternChange(LedPattern newState);
        void sendLedBrightnessChange(int value);
        void sendScanRequest();
    };
    
    #endif // WORKER_H
    

    worker.cpp:

    #include <iostream>
    #include <string>
    #include <stdio.h>
    
    #include <QDateTime>
    
    #include "constants.h"
    #include "message.h"
    #include "worker.h"
    
    using namespace std;
    
    Worker::Worker(QObject *parent) : QObject (parent), devices(parent), pollTimer(this)
    {
        DeviceDetails dev;
    
    //    dev.macAddr = "macaddr2";
    //    dev.devName = "devname22";
    //    dev.latestHB = "hbtime222";
    //    devices.update(dev);
    
    //    dev.macAddr = "macaddr55";
    //    dev.devName = "devname55";
    //    dev.latestHB = "hbtime55";
    //    devices.update(dev);
    
        QObject::connect(&pollTimer, &QTimer::timeout, this, &Worker::process);
        pollTimer.setInterval(100);
    }
    Worker::~Worker()
    {
    }
    void Worker::doQuit()
    {
    //    running.deref(); // set to value of 0.
        emit finished();
    }
    
    void Worker::start()
    {
        pollTimer.start();
    }
    
    void Worker::sendLedPatternChange(LedPattern newState)
    {
        Message msg;
    
        msg.add(msgTags[TAGENUM_PRODUCT], PRODUCT_NAME);
        msg.add(msgTags[TAGENUM_PACKETTYPE], msgtypeText[MSG_LED_SET_PATTERN]);
        msg.add(msgTags[TAGENUM_NEWPATTERN], ledPatternText[newState]);
    
        sm.send(msg.encodeXml());
    }
    void Worker::sendLedBrightnessChange(int value)
    {
        Message msg;
        string s;
    
        s = std::to_string(value);
        msg.add(msgTags[TAGENUM_PRODUCT], PRODUCT_NAME);
        msg.add(msgTags[TAGENUM_PACKETTYPE], msgtypeText[MSG_LED_SET_BRIGHTNESS]);
        msg.add(msgTags[TAGENUM_NEWBRIGHTNESS], s);
    
        sm.send(msg.encodeXml());
    }
    
    void Worker::sendHeartbeatAck()
    {
        Message msg;
        msg.add(msgTags[TAGENUM_PRODUCT], PRODUCT_NAME);
        msg.add(msgTags[TAGENUM_PACKETTYPE], msgtypeText[MSG_HEARTBEAT_ACK]);
    
        sm.send(msg.encodeXml());
    }
    
    void Worker::updateDeviceTable(Message msg)
    {
        DeviceDetails dev;
        dev.macAddr = QString::fromStdString(msg.getValue(msgTags[TAGENUM_MACADDRESS]));
        dev.devName = QString::fromStdString(msg.getValue(msgTags[TAGENUM_DEVICENAME]));
        dev.latestHB = QDateTime::currentDateTimeUtc().toString(QString::fromStdString(timestampFormat));
    
        devices.update(dev);
    }
    void Worker::sendScanRequest(void)
    {
        Message msg;
        msg.add(msgTags[TAGENUM_PRODUCT], PRODUCT_NAME);
        msg.add(msgTags[TAGENUM_PACKETTYPE], msgtypeText[MSG_SCAN_REQUEST]);
    
        sm.send(msg.encodeXml());
    }
    void Worker::process()
    {
        cout << "worker::process() started." << endl;
        int len;
    //    running.ref(); // set to value of 1
    //    while (true)
    //    {
            len = sm.recv(buffIn, sizeof(buffIn));
            if (len >= 0)
            {
                buffIn[len] = '\0';
                msg.decodeXml(buffIn);
                emit(newMessage(&msg));
    
                // if message is a heartbeat, send an ack.
                if (msg.getType() == MSG_HEARTBEAT)
                {
                    sendHeartbeatAck();
                    updateDeviceTable(msg);
                }
            }
            else
            {
                SocketState ss = sm.getSocketState();
                if (ss == SOCKET_DISCONNECTED)
                {
                    //sm.init();
                }
            }
    //        Sleep(10);
    //    }
        //emit reachedEndOfThread();
    }
    

    The test for whether the exit works is if the window disappears, and the debugger exits. It doesn't. The test for how often process() is called is the cout at the top of the routine.


  • Qt Champions 2017

    @mzimmers said in need ideas for list/table implementation:

    Noted, but I don't think that's causing my quit button to be ignored, is it?

    Probably not, although it would eventually cause a segafault somewhere.

    I just realized my process() routine is only called once. Is there something wrong with my pollTimer?

    Not that I can see. Are you sure this:

    len = sm.recv(buffIn, sizeof(buffIn));
    

    isn't blocking? (also the reason for my question why aren't you using Qt's sockets)


  • Qt Champions 2017

    @VRonin said in need ideas for list/table implementation:

    No need to give parents to stack-allocated QObjects like devices or pollTimer

    That is incorrect! It's quite needed.



  • The socket call might block, but only for a few seconds. I just stepped through the code, and discovered that worker::start() is never called. In main, I have:

        worker->moveToThread(thread);
         ...
        thread->start();
    

    Am I supposed to explicitly call worker::start()?

    No good reason for using native sockets; I'm just re-using code that I implemented on the target device (which sadly doesn't have Qt [yet]). But I'm fairly sure they're not the problem here. Remember this was (sort of) working once, before VRonin MOG'd me and I started making changes.


  • Qt Champions 2017

    Am I supposed to explicitly call worker::start()?

    No, not really. Do you have that:

    QObject::connect(thread, &QThread::started, worker, &Worker::start);
    

    before calling thread->start();?



  • Well, I didn't (the slot was process, not start), but I do now...same result.

    Is there a way to see whether this slot is actually getting invoked:

        QObject::connect(&widget, &Widget::quitButtonPushed, thread, &QThread::quit);
    

    I have verified the signal in the debugger.

    EDIT:

    Wait a minute: I just realized I'm doing this a little differently than I have in the past, in that the worker, not the widget is in the new thread. I think maybe I am exiting the worker, but I still have to stop the main/widget thread. Since main() isn't a Qt object, I can't use connect, so what's the recommended technique here?


  • Qt Champions 2017

    @mzimmers said in need ideas for list/table implementation:

    Is there a way to see whether this slot is actually getting invoked:

    Subclass QThread for the testing purposes and override run(). In the run implementation only do:

    void MyThread::run()
    {
        int ret = QThread::exec();
        qDebug() << "ret"; //< If you get to here with the debugger, then the `quit` was called.
    }
    

  • Qt Champions 2017

    @mzimmers said in need ideas for list/table implementation:

    Wait a minute: I just realized I'm doing this a little differently than I have in the past, in that the worker, not the widget is in the new thread. I think maybe I am exiting the worker, but I still have to stop the main/widget thread. Since main() isn't a Qt object, I can't use connect, so what's the recommended technique here?

    You have QApplication for that purpose and you can get it from anywhere with QCoreApplication::instance(). Connect the quit button to the application quit slot as well as to the thread quit slot. Don't forget to wait for the worker thread to finish (call QThread::wait) before exiting the application.



  • emit newMessage(&msg) is (probably) still a race condition, msg.decodeXml might (and almost surely does) get called before the slots connected to the signal had the time to read the last one. Either serialise access inside Message via stuff like QReadWriteLock or emit passing by value



  • @kshegunov so my exit button now triggers two calls:

        QApplication a(argc, argv);
        Widget widget;
        QThread* thread = new QThread;
      ...
        QObject::connect(&widget, &Widget::quitButtonPushed, thread, &QThread::quit);
        QObject::connect(&widget, &Widget::quitButtonPushed, &a, &QApplication::quit);
    

    Regarding QThread::wait(), according to the docs:

    Blocks the thread until either of these conditions is met

    (where one of the conditions is the thread finishes)

    So, presumably, what documentation means is that the routine blocks the deletion of the thread, not the thread itself, right? So I can call the wait() routine like this?

        widget.show();
        thread->start();
        thread->wait();
        rc = a.exec();
    

    EDIT:

    wrong.

    OK, so where should the thread wait call go?



  • You can connect &QThread::finished insetad of &Widget::quitButtonPushed to &QApplication::quit for the application to wait for the thread to finish before closing down



  • Perfect. Thanks for the help on threads.

    And speaking of threads, this one has gone afield enough that I think it's best to consider it finished. Thanks to everyone who provided input on this.


Log in to reply
 

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