Watchdog class

SPlatten

@jsulm It certainly could be.

KroMignon

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

SPlatten

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

aha_1980

@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

jsulm

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

KroMignon

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

SPlatten

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

KroMignon

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

SPlatten

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

KroMignon

@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();
};

SPlatten

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

SPlatten

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();
}

SPlatten

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?

KroMignon

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

SPlatten

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

KroMignon

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

SPlatten

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

Christian Ehrlicher

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

wrosecrans

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