Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

Watchdog class



  • I've written a watchdog class, the intention is that the watchdog has to be kicked to keep it up to date, if the internal timer is not kicked then it will raise a signal expired this can be used to do something, the intention is that my process will terminate if the watchdog is allowed to expire.

    For some reason it appears the timer is not running, can someone please take a look and see if I've done anything obviously wrong?

    Prototype:

        class clsWatchdog : public QObject {
        Q_OBJECT
    
        private:
            static const int mscint16Default;
    
            QTimer* mptmrExpiry;
    
        public:
            explicit clsWatchdog();
            ~clsWatchdog();
    
            void kick();
            void pause();
            void setExpiry(quint16 uint16Expiry, QObject* pParent);
    
        signals:
            void expired();
        };
    

    Implementation:

    #include "clsWatchdog.h"
    //Static initialisation
    const int clsWatchdog::mscint16Default = 12 * 1000; //Milliseconds
    /**
     * @brief clsWatchdog::clsWatchdog - Class constructor
     * @param pParent : Pointer to parent
     * @param intTimeInSecs : Optional, Milliseconds to set inital time
     */
    clsWatchdog::clsWatchdog() : mptmrExpiry(nullptr) {
    }
    /**
     * @brief clsWatchdog::~clsWatchdog - Class destructor
     */
    clsWatchdog::~clsWatchdog() {
        if ( mptmrExpiry != nullptr ) {
            if ( mptmrExpiry->isActive() == true ) {
                mptmrExpiry->stop();
            }
            QObject::disconnect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::expired);
        }
    }
    /**
     * @brief clsWatchdog::kick - Keep watchdog from expiring
     */
    void clsWatchdog::kick() {
        if ( mptmrExpiry != nullptr ) {
            int intInterval = mptmrExpiry->interval();
    
            if ( mptmrExpiry->isActive() == true ) {
                mptmrExpiry->stop();
            }
            mptmrExpiry->start(std::chrono::milliseconds(intInterval));
        }
    }
    /**
     * @brief clsWatchdog::pause - Use kick to restart
     */
    void clsWatchdog::pause() {
        if ( mptmrExpiry != nullptr ) {
            mptmrExpiry->stop();
        }
    }
    /**
     * @brief clsWatchdog::setExpiry
     * @param uint16Expiry : Expiry time in milliseconds
     * @param pPatent : Pointer to parent
     */
    void clsWatchdog::setExpiry(quint16 uint16Expiry, QObject* pParent) {
        mptmrExpiry = new QTimer(pParent);
    
        if ( mptmrExpiry != nullptr ) {
            mptmrExpiry->setInterval(uint16Expiry);
            mptmrExpiry->setSingleShot(false);
            QObject::connect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::expired);
        }
    }
    

  • Lifetime Qt Champion

    Hi,

    Since you are asking, the class looks like overkill since a QTimer should be enough for a watchdog.

    That said, just call restart no need for the stop and go code.



  • @SGaist before writing this class that's exactly what I did, but its the restarting of the timer that doesn't appear to be working, granted I haven't called restart, will try that now.

    Just tried and I'm using Qt 5.15.2 I don't see any restart function in the context help? In my non-class version I was just calling start which according to the documentation should just restart the timer, but I'm having a problem because it stops the timer and doesn't start it.

    I've modified the class to, prototype:

        class clsWatchdog : public QObject {
        Q_OBJECT
    
        private:
            static const int mscint16Default;
    
            QTimer* mptmrExpiry;
    
        public:
            explicit clsWatchdog();
            ~clsWatchdog();
    
            void kick();
            void pause();
            void setExpiry(quint16 uint16Expiry, QObject* pParent);
    
        signals:
            void expired();
    
        public slots:
            void onTimeout();
        };
    

    Implementation:

    #include "clsWatchdog.h"
    //Static initialisation
    const int clsWatchdog::mscint16Default = 12 * 1000; //Milliseconds
    /**
     * @brief clsWatchdog::clsWatchdog - Class constructor
     * @param pParent : Pointer to parent
     * @param intTimeInSecs : Optional, Milliseconds to set inital time
     */
    clsWatchdog::clsWatchdog() : mptmrExpiry(nullptr) {
    }
    /**
     * @brief clsWatchdog::~clsWatchdog - Class destructor
     */
    clsWatchdog::~clsWatchdog() {
        if ( mptmrExpiry != nullptr ) {
            if ( mptmrExpiry->isActive() == true ) {
                mptmrExpiry->stop();
            }
            QObject::disconnect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
        }
    }
    /**
     * @brief clsWatchdog::kick - Keep watchdog from expiring
     */
    void clsWatchdog::kick() {
        if ( mptmrExpiry != nullptr ) {
            auto durMS = std::chrono::milliseconds(mptmrExpiry->interval());
            mptmrExpiry->start(durMS);
        }
    }
    /**
     * @brief clsWatchdog::onTimeout
     */
    void clsWatchdog::onTimeout() {
    qDebug() << "clsWatchdog::onTimeout";
        emit expired();
    }
    /**
     * @brief clsWatchdog::pause - Use kick to restart
     */
    void clsWatchdog::pause() {
        if ( mptmrExpiry != nullptr ) {
            mptmrExpiry->stop();
        }
    }
    /**
     * @brief clsWatchdog::setExpiry
     * @param uint16Expiry : Expiry time in milliseconds
     * @param pPatent : Pointer to parent
     */
    void clsWatchdog::setExpiry(quint16 uint16Expiry, QObject* pParent) {
        mptmrExpiry = new QTimer(pParent);
    
        if ( mptmrExpiry != nullptr ) {
            mptmrExpiry->setInterval(uint16Expiry);
            mptmrExpiry->setSingleShot(false);
            QObject::connect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
        }
    }
    

    I started the timer, kicked it once to start the timer, but I never see onTimeout, its as if the timer isn't running.



  • @SPlatten This class implementation looks strange to me:

    • why do you have a parent parameter for setExpiry()?
    • your destructor() also looks strange to me!
    • why not simply call QTimer::start() in kick()??
    • and why do you say calling start() is stopping to timer? I never saw this.
    • why do you not use clsWatchdog instance as parent for the QTimer instance? This will ensure timer would be stopped and destroyed when clsWatchdog is destroyed.

    My suggestion would be to remove destructor and change setExpiry()/kick() to:

    void clsWatchdog::kick()
    {
         if(mptmrExpiry)
            mptmrExpiry->start(); // restart timer with default interval
    } 
    
    void clsWatchdog::setExpiry(quint16 uint16Expiry) 
    {
        if(!mptmrExpiry)
        {
            mptmrExpiry = new QTimer(this);
            connect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
            mptmrExpiry->setSingleShot(false);
        }
    
        mptmrExpiry->setInterval(uint16Expiry);
    }
    


  • @KroMignon , have you looked at the source?

    • why do you have a parent parameter for setExpiry()?
      The parameter is the expiry time to use for the timer.

    • your destructor() also looks strange to me!
      Why? It cleans up stopping any started time and disconnects the connection.

    • why not simply call QTimer::start() in kick()??
      Thats exactly what it does, it calls an overloaded version of QTimer::start passing the time to it.

    • why do you not use clsWatchdog instance as parent for the QTimer instance? This will ensure timer would be stopped and destroyed when clsWatchdog is destroyed.
      I've had a better idea today which I'm going to implement. Will post back when I've completed it.


  • Lifetime Qt Champion

    @SPlatten said in Watchdog class:

    The parameter is the expiry time to use for the timer.

    The question was about QObject* pParent parameter... Setting an instance of another class as parent is indeed strange.

    Why is QTimer* mptmrExpiry actually a pointer? Why not a simple member variable?



  • @jsulm It certainly could be.



  • @SPlatten said in Watchdog class:

    have you looked at the source?

    Yes I do.
    But you don't take in account my remarks.
    As usual, you always over-engineering your implementation without taking documentation in account and doing stuff because you suppose how it works but not taking time to learn how it really works.

    QObject class/sub-classes mostly have a parent parameter to simplify memory and threading management. You should already know this.
    Defining an external parent for the QTimer in clsWatchdog do not made sense. If you want to define a parent, the class instance should be the parent!

    When calling QTimer::start(), as written in documentation, the QTimer instance will be (re)started with default interval (defined with QTimer::setInterval()), so why do you do this?:

            auto durMS = std::chrono::milliseconds(mptmrExpiry->interval());
            mptmrExpiry->start(durMS);
    


  • @KroMignon, kick function is supposed to restart the timer keeping it from timing out, it calls the timer interval method in order to get the interval that was originally set so it can restart the timer with it.


  • Lifetime Qt Champion

    @SPlatten said in Watchdog class:

    @KroMignon, kick function is supposed to restart the timer keeping it from timing out, it calls the timer interval method in order to get the interval that was originally set so it can restart the timer with it.

    So have your read https://doc.qt.io/qt-5/qtimer.html#start-1 ?

    Starts or restarts the timer with the timeout specified in interval.

    If the timer is already running, it will be stopped and restarted.

    So mptmrExpiry->start(); is enought for your need.

    Regards


  • Lifetime Qt Champion

    @SPlatten I will cite the documentation here:
    void QTimer::start()

    This function overloads start().

    Starts or restarts the timer with the timeout specified in interval.

    If the timer is already running, it will be stopped and restarted.

    So, there is no need to call mptmrExpiry->interval() each time you call start(). That's what @KroMignon is saying.



  • @SPlatten said in Watchdog class:

    kick function is supposed to restart the timer keeping it from timing out, it calls the timer interval method in order to get the interval that was originally set so it can restart the timer with it.

    And this is exactly what QTimer::start() do!
    Because you are unable to read yourself documentation:

    void QTimer::start()
    This function overloads start().
    Starts or restarts the timer with the timeout specified in interval.
    If the timer is already running, it will be stopped and restarted.
    If singleShot is true, the timer will be activated only once.



  • @KroMignon , reading something is not proof that it does what it says! The reason I posted here in the first place is because having read the documentation, it doesn't seem to do what it is documented.

    I wish you would stop playing the same old record, its very tiresome and boring, just because once I said someplace that I didn't read the documentation does not make it my life story so get over it and move on.

    For the record I have been developing and writing software since 1987 professionally, so to say I don't read is insulting and wrong.



  • @SPlatten said in Watchdog class:

    it doesn't seem to do what it is documented.

    Just for the record, I am using QTimer for years now with Windows/Android/Linux system and have implemented such kind of "Watchdog timer" and I am using QTimer::start() and it simply works.

    The only thing why QTimer do not work could be:

    • the thread event loop is not working/locked
    • call start()/stop() slot from the wrong thread

    PS: I, and I suppose many of Qt forum contributors, doing software development for years/decades... This not preserve us from doing bad stuff!



  • @KroMignon , likewise I've been using instances of QTimer all over, I posted here because it seems not to be working and since everyone can make mistakes its not always obvious what has been done wrong, which was why I was hoping someone could explain why it doesn't seem to be restarting.

    As mentioned in another post I've thought about a work around.



  • @SPlatten said in Watchdog class:

    As mentioned in another post I've thought about a work around.

    This would be my suggestion (maybe there are some typo):

    class clsWatchdog : public QObject 
    {
        Q_OBJECT
    
    private:
        QTimer* mptmrExpiry;
    
    public:
        explicit clsWatchdog(int interval, QObject* parent = nullptr)
        : QObject(parent)
        , mptmrExpiry(new QTimer(this))
        {
            connect(mptmrExpiry, &QTimer::timeout, this, &clsWatchdog::expired);
            mptmrExpiry->setInterval(interval);
            mptmrExpiry->setSingleShot(true);
            //Note: timer will NOT be started here    
        }
    
        void setExpiry(quint16 uint16Expiry)
        {
            //Note: timer will NOT be (re)started here    
            mptmrExpiry->setInterval(uint16Expiry);
        }
        
    public slots:
        void kick()
        {
            // ensure called in right thread
            if(QThread::currentThread() != thread())
            {
                QTimer::singleShot(0, this, &clsWatchdog::kick);
                return;
            }
            // start or restat the timer
            mptmrExpiry->start();
        }
        void pause()
        {
            // ensure called in right thread
            if(QThread::currentThread() != thread())
            {
                QTimer::singleShot(0, this, &clsWatchdog::pause);
                return;
            }
            if(mptmrExpiry->isActive())
                mptmrExpiry->stop();
        }
    
    signals:
        void expired();
    };
    


  • @KroMignon Very soon I will post the new version of the class, which is simpler.

    In developing this now, I am getting lots of what I believe are wrong clazy warings, e.g.:

        mtmrExpiry.setInterval(std::chrono::milliseconds(clsWatchdog::mscint16Timer));
        mtmrExpiry.setSingleShot(false);
        QObject::connect(&mtmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
        mtmrExpiry.start();
    

    There is a clazy warning on the connect line:

    QTimer::timeout is not a signal [clazy-connect-non-signal]
    

    What could be causing this? QTimer is defined as a member of the class, as is the slot onTimeout:

        class clsWatchdog : public QObject {
        Q_OBJECT
    
        private:
            static const int mscint16Default;
            static const int mscint16Timer;
    
            QTimer mtmrExpiry;
       ...
        private slots:
            void onTimeout();
        };
    

    Another clazy message:

    Emit keyword being used with non-signal clsWatchdog::expired [clazy-incorrect-emit]
    

    This is on:

    emit expired();
    

    Which is called in one of my class methods, there is the prototype in the class:

        signals:
            void expired();
    


  • Here is the finished clsWatchdog which despite the clazy messages works, prototype:

    #ifndef CLSWATCHDOG_H
            #define CLSWATCHDOG_H
        
            #include <QTimer>
        
            class clsWatchdog : public QObject {
            Q_OBJECT
        
            private:
                static const int mscint16Default;
                static const int mscint16Timer;
        
                QTimer mtmrExpiry;
                quint16 muint16Counter, muint16Setpoint;
        
            public:
                explicit clsWatchdog();
                ~clsWatchdog();
        
                void kick();
                void pause();
                void resume();
        
            signals:
                void expired();
        
            private slots:
                void onTimeout();
            };
        #endif // CLSWATCHDOG_H
    

    Implementation:

    /**
     * File:    clsWatchdog.cpp
     * Notes:   Contains implementation of the watchdog class
     * History: 2021/01/14 Created by Simon Platten
     */
    #include "../clsDebugService.h"
    #include "clsWatchdog.h"
    //Static initialisation
    const int clsWatchdog::mscint16Default = 12;    //Counter setpoint
    const int clsWatchdog::mscint16Timer = 1000;    //Milliseconds
    /**
     * @brief clsWatchdog::clsWatchdog - Class constructor
     */
    clsWatchdog::clsWatchdog() : muint16Counter(0)
                               , muint16Setpoint(clsWatchdog::mscint16Default) {
        mtmrExpiry.setInterval(std::chrono::milliseconds(clsWatchdog::mscint16Timer));
        mtmrExpiry.setSingleShot(false);
        QObject::connect(&mtmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
        mtmrExpiry.start();
    }
    /**
     * @brief clsWatchdog::~clsWatchdog - Class destructor
     */
    clsWatchdog::~clsWatchdog() {
        if ( mtmrExpiry.isActive() == true ) {
            mtmrExpiry.stop();
        }
        QObject::disconnect(&mtmrExpiry, &QTimer::timeout, this, &clsWatchdog::onTimeout);
    }
    /**
     * @brief clsWatchdog::kick - Keep watchdog from expiring
     */
    void clsWatchdog::kick() {
        //Reset counter
        muint16Counter = 0;
    }
    /**
     * @brief clsWatchdog::onTimeout
     */
    void clsWatchdog::onTimeout() {
        //Increment the counter
        if ( ++muint16Counter >= muint16Setpoint ) {
            emit expired();
        }
    }
    /**
     * @brief clsWatchdog::pause - Use resume to restart
     */
    void clsWatchdog::pause() {
        mtmrExpiry.stop();
    }
    /**
     * @brief clsWatchdog::resume
     */
    void clsWatchdog::resume() {
        mtmrExpiry.start();
    }
    


  • So going back to my original purpose...having written and tested this class which works perfectly when running the application it resides in as a standalone module.

    Now I'm launching this application from another Qt process, the process of launching this is:

        QProcess::setProgram(crstrProgram);
        //Register the module
        clsModule::registerModule(this, crstrAlias);
    
        if ( blnStart != true ) {
            return;
        }
        //Start the process
        start();
        //Monitor when this process changes state
        connect(this, &QProcess::stateChanged, this
                    , [this](QProcess::ProcessState newState) {
            const qint64 cint64PID = processId();
    
            if ( newState == QProcess::NotRunning ) {
        //Only create the new instance if 'this' module already has a known PID
        //and the PID valid and different from existing knownPID
                if ( mint64PID != 0 && cint64PID > 0 && cint64PID != mint64PID ) {
        //Remove this instance of the module
                    clsModule::newInstanceOf(*this);
                }
            } else if ( newState == QProcess::Running && cint64PID > 0 ) {
                const QString& crstrProgram(program());
                emit PID(crstrProgram, cint64PID);
            }
        });
    

    crstrProgram is a passed in parameter defined as const QString&, it contains the name of the module / process to launch in this case mdFileIO.

    I can see by looking at the running processes that both the launcher and mdFileIO are running, however when mdFileIO is started this way, the watchdog does not work and when I attach to the process the timer isn't running. Can anyone suggest why?



  • @SPlatten said in Watchdog class:

    Can anyone suggest why?

    As I have written before: I suppose you try to start the timer from the wrong thread. QTimer start()/stop() must be called within the thread in which the timer is running.



  • @KroMignon, The instance of clsWatchdog is in the application class:

        class clsModHelper : public QObject {
        Q_OBJECT
    
        protected:
            bool mblnStandalone;
            double mdblVersion;
            FILE* mfpDbgLog;
            qint64 mint64AppPID;
            clsWatchdog mobjWatchdog;
    ...
    

    This class is the base class for the application:

        class clsModFileIO : public clsModHelper {
    

    The only instance of this class is created in:

    int main(int intArgc, char* parystrArgv[]) {
        QCoreApplication a(intArgc, parystrArgv);
        clsModFileIO obj(intArgc, parystrArgv);
        return a.exec();
    }
    

    The watchdog timer is started in its own default constructor which as its declared as a member of clsModHelper, must be in the main thread ?

    Again, what I'm struggling to understand is why it works perfectly when the process is launched directly, but not from the other process using QProcess ?



  • @SPlatten It is very complicated to follow you.

    I don't really understand what is the purpose of your code, and which clsWatchdog instance you are talking about.

    What I believe to have understand:
    You have create software which starts other application with help of a custome QProcess class, why not.

    What I do not understand:

    • what is the purpose of the clsWatchdog ?
    • is clsWatchdog used by "launcher" application? Is this one which is not working as expected?
    • is clsWatchdog used by "external" application? Is this one which is not working as expected?


  • @KroMignon , the launcher is the main engine, the processes it launchers are helper modules. The launcher will communicate with helpers to request services which the helpers will provide. Each helper will use the watchdog, it’s kept alive by the regular communication from the engine. If the engine is terminated then the communication stops and the purpose of the helpers ends so the watchdog will expire and each helper will terminate.

    Hope that helps?


  • Lifetime Qt Champion

    @SPlatten said in Watchdog class:

    If the engine is terminated then the communication stops and the purpose of the helpers ends so the watchdog will expire and each helper will terminate.

    Use a QTimer instead some strange self-written classes.



  • @SPlatten said in Watchdog class:

    @KroMignon , reading something is not proof that it does what it says! The reason I posted here in the first place is because having read the documentation, it doesn't seem to do what it is documented.

    You seem really dedicated to having an XY problem for some reason: https://xyproblem.info/

    Why not just post about your original confusion with the documentation of QTimer? You say "it doesn't seem to do what it is documented" but you haven't actually explained what problem you were having with it. If there is a bug (in either the docs or the implementation), sharing the specifics will help get it fixed. If there isn't a bug in QTimer, sharing the specifics will help you get past the bug in your code using it.


Log in to reply