Sequential work with the port

[maratk1n]

Hello. I have a serious problem that I can not solve.
I have a device that sends a request to the board and is waiting for a response. Work with the device is done in a separate thread, so as not to slow down the gui. Also there is a class of algorithms that works in another thread. The algorithm calls the methods to open / close.

MainWindow:

/***mainwindow.h***/
#ifndef MAINWINDOW_H
#define MAINWINDOW_H
 
#include <QMainWindow>
#include <QThread>
#include "deviceworker.h"
#include "algorithm.h"
 
namespace Ui {
class MainWindow;
}
 
class MainWindow : public QMainWindow
{
    Q_OBJECT
 
public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();
    FirstDevice *device;
    Algorithm *algorithm;
    QThread *deviceThread;
    QThread *algorithmThread;
 
private slots:
    void on_pushButton_clicked();
 
private:
    Ui::MainWindow *ui;
 
};
 
#endif // MAINWINDOW_H
 
/***mainwindow.cpp***/
#include "ui_mainwindow.h"
 
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
}
 
MainWindow::~MainWindow()
{
    delete ui;
 
    QThread *deviceThread = new QThread();
    QThread *algorithmThread = new QThread();
    device = new FirstDevice();
    device->setReadTimer(333);
    device->moveToThread(deviceThread);
 
    algorithm = new Algorithm(device);
    algorithm->moveToThread(algorithmThread);
    deviceThread->start(QThread::TimeCriticalPriority);
    algorithmThread->start();
}
 
void MainWindow::on_pushButton_clicked()
{
    algorithm->run();
}

/***mainwindow.h***/
#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <QThread>
#include "deviceworker.h"
#include "algorithm.h"

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();
    FirstDevice *device;
    Algorithm *algorithm;
    QThread *deviceThread;
    QThread *algorithmThread;

private slots:
    void on_pushButton_clicked();

private:
    Ui::MainWindow *ui;

};

#endif // MAINWINDOW_H

/***mainwindow.cpp***/
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
}

MainWindow::~MainWindow()
{
    delete ui;

    QThread *deviceThread = new QThread();
    QThread *algorithmThread = new QThread();
    device = new FirstDevice();
    device->setReadTimer(333);
    device->moveToThread(deviceThread);

    algorithm = new Algorithm(device);
    algorithm->moveToThread(algorithmThread);
    deviceThread->start(QThread::TimeCriticalPriority);
    algorithmThread->start();
}

void MainWindow::on_pushButton_clicked()
{
    algorithm->run();
}

To copy to clipboard, switch view to plain text mode 

DeviceWorker:

/***deviceworker.h***/
#ifndef DEVICEWORKER_H
#define DEVICEWORKER_H
 
#include <QTimer>
#include <QtSerialPort/QSerialPort>
#include <QMutex>
#include <QDebug>
#include <QObject>
 
using namespace std;
 
 
class FirstDevice : public QObject {
    Q_OBJECT
 
private:
    QTimer readTimer;
public:
    explicit FirstDevice(QObject *parent = 0);
    ~FirstDevice(){}
 
    void setReadTimer(int ms){
        readTimer.start(ms);
    }
 
public slots:
    bool open(int id){
        emit switchStateSig(id, true);
    }
    bool close(int id){
        emit switchStateSig(id, false);
    }
    bool getAllStates();
 
private slots:
    bool switchState(int id, bool state);
 
signals:
    void remoteSignal(bool);
    void status(int, bool, bool);
    void switchStateSig(int, bool);
};
 
 
class ComPort : public QObject {    //singltone
    Q_OBJECT
private:
    QSerialPort *serial;
    QMutex *mutex;
    explicit ComPort(QObject *parent = 0){
        mutex = new QMutex();
        serial = new QSerialPort();
        connect(serial, SIGNAL(error(QSerialPort::SerialPortError)), this, SLOT(handleError(QSerialPort::SerialPortError)));
    }
 
    ~ComPort(){}
    ComPort(ComPort const&) = delete;
    ComPort& operator= (ComPort const&) = delete;
public:
    static ComPort& get()
    {
        static ComPort instance;
        return instance;
    }
 
    void open(){
        if(serial->isOpen())
            close();
        serial->setPortName("COM2");
        if (serial->open(QIODevice::ReadWrite)) {
            if (serial->setBaudRate(QSerialPort::Baud115200)
                    && serial->setFlowControl(QSerialPort::NoFlowControl)) {
            qDebug() << "open";
            } else {
                qDebug() << QString(serial->errorString());
                serial->close();
            }
        }
    }
    void close(){serial->close();}
    QByteArray requestResponse(const QByteArray &data){
        mutex->lock();
        qDebug() << "-------------------------";
        if(!serial->isOpen())
            open();
        int attempts = 1;
        QByteArray readBuf;
        while (attempts <= 3) {
            if (serial->isWritable())
            {
                serial->write(data);
                serial->waitForBytesWritten(3);
                while (serial->waitForReadyRead(300)) {    //(wait 300ms). Here, randomly breaks down
                    readBuf += serial->readAll();
                    if (readBuf.size() == 4){
                        close();
                        mutex->unlock();
                        return readBuf;
                    }
                }
                readBuf.clear();
                close();
                open();
                attempts++;
            }
            else
            {
                qDebug() << "port is not written";
                close();
                mutex->unlock();
                return 0;
            }
        }
        close();
        mutex->unlock();
        return 0;
    }
 
private slots:
    void handleError(QSerialPort::SerialPortError error){
        if (error == QSerialPort::ResourceError) {
            close();
            qDebug() << "Error!  ";
        }
    }
};
 
#endif // DEVICEWORKER_H
 
 
/***deviceworker.cpp***/
#include "deviceworker.h"
 
 
FirstDevice::FirstDevice(QObject *parent):QObject(parent)
{
    connect(this, SIGNAL(switchStateSig(int, bool)), this, SLOT(switchState(int, bool)));
    connect(&readTimer, SIGNAL(timeout()), this, SLOT(getAllStates()));
}
 
bool FirstDevice::getAllStates()
{
    QByteArray arr;
    arr.resize(2);
    arr[0] = 0xAB;
    arr[1] = 0x01;
    QByteArray response = ComPort::get().requestResponse(arr);
    if(response[0] == arr[0])
    {
        emit remoteSignal(1);
        return 1;
    }
    emit remoteSignal(0);
    return 0;
}
 
bool FirstDevice::switchState(int id, bool state)
{
    QByteArray arr;
    arr.resize(2);
    arr[0] = 0xAB;
    arr[1] = 0x01;
 
    QByteArray response = ComPort::get().requestResponse(arr);
    if(response[0] == arr[0]) //ok
    {
        emit status(id, state, true);
        return 1;
    }
    emit status(id, state, false);
    return 0;
}

/***deviceworker.h***/
#ifndef DEVICEWORKER_H
#define DEVICEWORKER_H

#include <QTimer>
#include <QtSerialPort/QSerialPort>
#include <QMutex>
#include <QDebug>
#include <QObject>

using namespace std;


class FirstDevice : public QObject {
    Q_OBJECT

private:
    QTimer readTimer;
public:
    explicit FirstDevice(QObject *parent = 0);
    ~FirstDevice(){}

    void setReadTimer(int ms){
        readTimer.start(ms);
    }

public slots:
    bool open(int id){
        emit switchStateSig(id, true);
    }
    bool close(int id){
        emit switchStateSig(id, false);
    }
    bool getAllStates();

private slots:
    bool switchState(int id, bool state);

signals:
    void remoteSignal(bool);
    void status(int, bool, bool);
    void switchStateSig(int, bool);
};


class ComPort : public QObject {    //singltone
    Q_OBJECT
private:
    QSerialPort *serial;
    QMutex *mutex;
    explicit ComPort(QObject *parent = 0){
        mutex = new QMutex();
        serial = new QSerialPort();
        connect(serial, SIGNAL(error(QSerialPort::SerialPortError)), this, SLOT(handleError(QSerialPort::SerialPortError)));
    }

    ~ComPort(){}
    ComPort(ComPort const&) = delete;
    ComPort& operator= (ComPort const&) = delete;
public:
    static ComPort& get()
    {
        static ComPort instance;
        return instance;
    }

    void open(){
        if(serial->isOpen())
            close();
        serial->setPortName("COM2");
        if (serial->open(QIODevice::ReadWrite)) {
            if (serial->setBaudRate(QSerialPort::Baud115200)
                    && serial->setFlowControl(QSerialPort::NoFlowControl)) {
            qDebug() << "open";
            } else {
                qDebug() << QString(serial->errorString());
                serial->close();
            }
        }
    }
    void close(){serial->close();}
    QByteArray requestResponse(const QByteArray &data){
        mutex->lock();
        qDebug() << "-------------------------";
        if(!serial->isOpen())
            open();
        int attempts = 1;
        QByteArray readBuf;
        while (attempts <= 3) {
            if (serial->isWritable())
            {
                serial->write(data);
                serial->waitForBytesWritten(3);
                while (serial->waitForReadyRead(300)) {    //(wait 300ms). Here, randomly breaks down
                    readBuf += serial->readAll();
                    if (readBuf.size() == 4){
                        close();
                        mutex->unlock();
                        return readBuf;
                    }
                }
                readBuf.clear();
                close();
                open();
                attempts++;
            }
            else
            {
                qDebug() << "port is not written";
                close();
                mutex->unlock();
                return 0;
            }
        }
        close();
        mutex->unlock();
        return 0;
    }

private slots:
    void handleError(QSerialPort::SerialPortError error){
        if (error == QSerialPort::ResourceError) {
            close();
            qDebug() << "Error!  ";
        }
    }
};

#endif // DEVICEWORKER_H


/***deviceworker.cpp***/
#include "deviceworker.h"


FirstDevice::FirstDevice(QObject *parent):QObject(parent)
{
    connect(this, SIGNAL(switchStateSig(int, bool)), this, SLOT(switchState(int, bool)));
    connect(&readTimer, SIGNAL(timeout()), this, SLOT(getAllStates()));
}

bool FirstDevice::getAllStates()
{
    QByteArray arr;
    arr.resize(2);
    arr[0] = 0xAB;
    arr[1] = 0x01;
    QByteArray response = ComPort::get().requestResponse(arr);
    if(response[0] == arr[0])
    {
        emit remoteSignal(1);
        return 1;
    }
    emit remoteSignal(0);
    return 0;
}

bool FirstDevice::switchState(int id, bool state)
{
    QByteArray arr;
    arr.resize(2);
    arr[0] = 0xAB;
    arr[1] = 0x01;

    QByteArray response = ComPort::get().requestResponse(arr);
    if(response[0] == arr[0]) //ok
    {
        emit status(id, state, true);
        return 1;
    }
    emit status(id, state, false);
    return 0;
}

To copy to clipboard, switch view to plain text mode 

algorithm:

#ifndef ALGORITHM_H
#define ALGORITHM_H
#include <QObject>
#include "deviceworker.h"
class Algorithm: public QObject
{
    Q_OBJECT
private:
    FirstDevice *device;
public:
    Algorithm(FirstDevice *device_){
        device = device_;
    }
    void run(){
        device->open(3); //Here I must wait for an answer
        //do something
        device->close(3); //Here I must wait for an answer
    }
};
 
#endif // ALGORITHM_H

#ifndef ALGORITHM_H
#define ALGORITHM_H
#include <QObject>
#include "deviceworker.h"
class Algorithm: public QObject
{
    Q_OBJECT
private:
    FirstDevice *device;
public:
    Algorithm(FirstDevice *device_){
        device = device_;
    }
    void run(){
        device->open(3); //Here I must wait for an answer
        //do something
        device->close(3); //Here I must wait for an answer
    }
};

#endif // ALGORITHM_H

To copy to clipboard, switch view to plain text mode 

So, I have two problems:
1. How to wait for a response from a card without using the method waitForReadyRead()? This is what I need, but I can not use it. The program periodically falls arbitrarily in this place. I read that this function is unstable on windows.
2. Inside the Algorithm method run(), I open / close the device. But since the device and the algorithm work in different threads, I had to connect them with signals and slots. Therefore, I do not wait for an answer from the board, but I go further. That is, in fact, the board may not respond, and I will ignore it, which is very bad.

I apologize for the long post, but I do not know how to make it even shorter.

[high_flyer]

As far as I can understand what you asked, both 1 and 2 are the same question - or did I misunderstand?
Since you already are in a different thread, and from what I understand you want a blocking wait why not just implement your own waitForReadReady()?:

//NUM_BYTES is a place holder for the number of bytes you deem as enough to wait on, 1 would be the minimum, but maybe you prefer to wait for a full packet (what ever a full packet is in your case).
void ComPort::waitForReadReady() const
{
   while(serial->bytesAvailable() < NUM_BYTES){
     //depending on needs you could add a sleep here to no hog the CPU too much
   }
}

//NUM_BYTES is a place holder for the number of bytes you deem as enough to wait on, 1 would be the minimum, but maybe you prefer to wait for a full packet (what ever a full packet is in your case).
void ComPort::waitForReadReady() const
{
   while(serial->bytesAvailable() < NUM_BYTES){
     //depending on needs you could add a sleep here to no hog the CPU too much
   }
}

To copy to clipboard, switch view to plain text mode 

[maratk1n]

did I misunderstand?

Not certainly in that way.
In fact, device->open()/close() is just the emission of a signal. I emit a signal switchStateSig, then the slot switchState in the thread of the device is called. I do not control this, I just go further in the thread of the algorithm.
I can not call directly, because the device and the algorithm are in different threads, so I had to connect them with a signal and a slot.
This should work like this: from the thread of the algorithm, I call the open/close of the device, I wait for an answer, and only then I go further. Now I just emit a signal and, without waiting for an answer, I go forward. This is very bad. This is the first problem.

The second problem is that the function waitForReadyRead() unpredictably breaks the whole program. I tried to do this, but I did not get a single byte. Apparently, something is blocked:

void ms_delay(int ms){
    QElapsedTimer ms_timer;
    ms_timer.start();
    while(ms_timer.elapsed() < ms){}
    return;
}
 
QByteArray ComPort::requestResponse(const QByteArray &data)
{
    mutex->lock();
    qDebug() << "-------------------------";
    if(!serial->isOpen())
        open();
    int attempts = 1;
    QElapsedTimer waitTimer;
    readBuf.clear();
    while (attempts <= 3) {
        if (serial->isWritable())
        {
            serial->write(data);
            waitTimer.restart();
            while (waitTimer.elapsed() < 333){
                ms_delay(5);
                readBuf += serial->readAll();
                if (readBuf.size() == 4){
                    close();
                    mutex->unlock();
                    return readBuf;
                }
            }
            readBuf.clear();
            qDebug() << "Timeout...";
            close();
            open();
            attempts++;
        }
        else
        {
            qDebug() << "Port is not written";
            close();
            mutex->unlock();
            return 0;
        }
 
    }
    close();
    mutex->unlock();
    return 0;
}

void ms_delay(int ms){
    QElapsedTimer ms_timer;
    ms_timer.start();
    while(ms_timer.elapsed() < ms){}
    return;
}

QByteArray ComPort::requestResponse(const QByteArray &data)
{
    mutex->lock();
    qDebug() << "-------------------------";
    if(!serial->isOpen())
        open();
    int attempts = 1;
    QElapsedTimer waitTimer;
    readBuf.clear();
    while (attempts <= 3) {
        if (serial->isWritable())
        {
            serial->write(data);
            waitTimer.restart();
            while (waitTimer.elapsed() < 333){
                ms_delay(5);
                readBuf += serial->readAll();
                if (readBuf.size() == 4){
                    close();
                    mutex->unlock();
                    return readBuf;
                }
            }
            readBuf.clear();
            qDebug() << "Timeout...";
            close();
            open();
            attempts++;
        }
        else
        {
            qDebug() << "Port is not written";
            close();
            mutex->unlock();
            return 0;
        }

    }
    close();
    mutex->unlock();
    return 0;
}

To copy to clipboard, switch view to plain text mode 

[high_flyer]

So let me see if I understand:
Calling the QDevice::waitForReadRead() is not relible on windows, so you don't call it and want to have some other function to call instead that delivers the same functionality.
Is that correct?
(All the story around it, how things in other threads are, are not the problem, more the motivation, as far as I can see)

If so, did you try what I offered in my last post, and if you did, what was the problem?

[maratk1n]

Is that correct?

Yes that's right. This is problem number 1. I have not tried your method yet, because no access to the board. I would like to monitor the reception with time, because if my request does not reach board for some reason, she will not answer me, no matter how much time I wait. For this reason, I make repeated requests after the timeout.

All the story around it, how things in other threads are, are not the problem, more the motivation, as far as I can see.

I can not say with certainty. But I tried to call directly device->switchState(7, true); from algorithm, which got the error that you can not call the methods of another thread, being not in the main thread. Therefore, I connected them with signals.

[high_flyer]

because if my request does not reach board for some reason, she will not answer me, no matter how much time I wait. For this reason, I make repeated requests after the timeout.

or you can set a timeout as well.

[maratk1n]

or you can set a timeout as well.

So, how can I do this?..

[high_flyer]

So, how can I do this?..

there are several ways...
Probably the least "intrusive" would be to have a timer run and you can check the elapsed time in your while() loop where you are checking the available bytes from the serial device.
Once the timeout is reached you simply return from your custom waitForReadRead() with false.

[maratk1n]

there are several ways...
Probably the least "intrusive" would be to have a timer run and you can check the elapsed time in your while() loop where you are checking the available bytes from the serial device.
Once the timeout is reached you simply return from your custom waitForReadRead() with false.

Thank for you answer. But I already tried to do this way, the port "was silent".
http://www.qtcentre.org/threads/6904...574#post301574

[high_flyer]

Thank for you answer. But I already tried to do this way, the port "was silent".

Do you mean by that the the port didn't answer within the time you specified?
If yes, then try prolonging the timeout.

[maratk1n]

Do you mean by that the the port didn't answer within the time you specified?
If yes, then try prolonging the timeout.

I tried to increase. Ping using the function waitForReadRead() was 2-5 milliseconds. With the use of a timer, the board absolutely did not answer anything for 333 milliseconds and more. I concluded that data reception is somehow blocked.