Unsolved 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); } }
-
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 theQTimer
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); }
- why do you have a parent parameter for
-
@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.
-
-
@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 withQTimer::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.
-
@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
-
@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.