Modbus Com in different thread

guerinoni

@J-Hilk
I have this class to handle modbus

class ModbusCom : public QObject
{
    Q_OBJECT

public:
    explicit ModbusCom(QObject *parent = nullptr);

    [[nodiscard]] bool connectModbus(const SerialPortSettings &settings);
    [[nodiscard]] bool connectModbus(const QString &portName,
                                     QSerialPort::BaudRate baudRate = QSerialPort::Baud9600,
                                     QSerialPort::DataBits dataBits = QSerialPort::Data8,
                                     QSerialPort::Parity parity = QSerialPort::NoParity,
                                     QSerialPort::StopBits stopBits = QSerialPort::OneStop);
    void disconnectModbus();

public slots:
    virtual void readRegisters(int startingAddress, uint16_t size);
    virtual void writeRegisters(int startingAddress, uint16_t size, std::vector<uint16_t> values);

private Q_SLOTS:
    void handleReadyRead();

Q_SIGNALS:
    void modbusConnected();
    void modbusDisconnected();
    void modbusError(const QString &errorString, QModbusDevice::Error error);
    void updateValue(int address, uint16_t value);

private:
    QModbusRtuSerialMaster m_modbusDevice;

    [[nodiscard]] bool open();
};

this is ctor

ModbusCom::ModbusCom(QObject *parent)
    : QObject(parent)
{
    m_modbusDevice.setParent(this);

    connect(&m_modbusDevice, &QModbusClient::errorOccurred, this, [this](QModbusDevice::Error error) {
        qCWarning(Serial) << QStringLiteral("Error occurred: %1 (code 0x%2)")
                                 .arg(m_modbusDevice.errorString())
                                 .arg(error);
        Q_EMIT modbusError(m_modbusDevice.errorString(), error);
    });

    connect(&m_modbusDevice,
            &QModbusClient::stateChanged,
            this,
            [this](QModbusDevice::State state) {
                if (state == QModbusDevice::UnconnectedState) {
                    Q_EMIT modbusDisconnected();
                }
            });
}

and from other class when emit read I just connect slot of this class to call a read

void ModbusCom::readRegisters(int startingAddress, uint16_t size)
{
    auto dataUnit = QModbusDataUnit(QModbusDataUnit::HoldingRegisters, startingAddress, size);

    if (auto *reply = m_modbusDevice.sendReadRequest(dataUnit, 1)) {
        //        if (!reply->isFinished()) {
        //            connect(reply, &QModbusReply::finished, this, &ModbusCom::handleReadyRead);
        //            connect(reply,
        //                    &QModbusReply::errorOccurred,
        //                    this,
        //                    [this, reply](QModbusDevice::Error error) {
        //                        Q_EMIT modbusError(reply->errorString(), error);
        //                        reply->deleteLater();
        //                    });
        //        } else {
        //            delete reply;
        //        }
    } else {
        qCCritical(Serial) << "Read error:" << m_modbusDevice.errorString();
    }
}

when sendReadRequest is called the error i shown

J.Hilk

@guerinoni ok, m_modbusDevice.setParent(this); should work, even so I'm usually assign the parent in the class initializer list.

Can you share the section where you create the instance of MosbusCom and where you move it to the thread ?

guerinoni

@J-Hilk
Ok, I can move it in .hpp file like QModbusRtuSerialMaster m_modbusDevice{this};
All is in a SerialWorker

class SerialComWorker
{
public:
    SerialComWorker();

    const ModbusCom &modbusCom() const;
    Protocol &protocol();

private:
    OvenPreferencesContainer::OvenTypes m_currentOvenProtocol;
    SerialPortSettings m_serialSettings;
    Protocol m_comProtocol;
    ModbusCom m_modbus;
    QThread m_thread;

    void loadProtocol();
};

and the magic in ctor

SerialComWorker::SerialComWorker()
    : m_currentOvenProtocol{OvenPreferencesContainer::MaxOvenTypes}
{
    m_modbus.moveToThread(&m_thread);
    QObject::connect(&m_thread, &QThread::started, &m_modbus, [&]() { loadProtocol(); });
    m_thread.start();
}

J.Hilk

@guerinoni
ok, heap allocate this one ModbusCom m_modbus; (~~without~~ with a nullptr as parent), it should be completely detached from your SerialComWorker class otherwise you also run the possible issue of deleting the ModbusCom while m_thread is still active

guerinoni

@J-Hilk
I have same problem

SerialComWorker::SerialComWorker()
    : m_currentOvenProtocol{OvenPreferencesContainer::MaxOvenTypes}
{
    m_modbus = new ModbusCom;
    m_modbus->moveToThread(&m_thread);
    QObject::connect(&m_thread, &QThread::started, m_modbus, [&]() { loadProtocol(); });
    m_thread.start();
}

I change only this. I'm on Qt 5.12.7 (embedded but it doesn't matter)

J.Hilk

@guerinoni
actually, looking into the source code of 5.12.7 there seem to be 2 member timers that life in the thread during creation oO

https://code.qt.io/cgit/qt/qtserialbus.git/tree/src/serialbus/qmodbusrtuserialmaster_p.h?h=5.12.7

mmh, what you can do, since you actually listen to the thread started signal,

You could create a init function, that instantiates the RtuMaster than and after that call loadProtocol()

would be a workaround!

guerinoni

@J-Hilk
ehehe because of this I posted this question... If I switch on Qt 5.14.x can I resolve this?

J.Hilk

@guerinoni
possibly , the QTimer's are replaced with a custom timer class
https://code.qt.io/cgit/qt/qtserialbus.git/tree/src/serialbus/qmodbusrtuserialmaster_p.h?h=5.14.2

I'm sure that was done for some reason ;)

guerinoni

@J-Hilk
It seems work correctly with 5.14.2 and the workaroud that you suggest :D
Thanks

SNOWA

because m_modbusDevice is created in the constructor of ModbusCom and ModbusCom is instanced in mainthread. So m_modbusDevice is the object belonging the mainthread, and its timeout QTimer can't be worked propertly.

To fix that, try to create the m_modbusDevice in m_thread, for exaple:

class ModbusCom
{
   QModbusRtuSerialMaster*  m_modbusDevice;
   void initModbus()
  {
    m_modbusDevice=new QModbusRtuSerialMaster();//created in m_thread

     //other initialization code...

    }
}
connect(SerialComWorker,SIGNAL(startModbus()), m_modbus,SLOT( initModbus()); //then initModbus() function will excute in m_thread and m_modbusDevice will be m_thread's obejct.
m_thread.start();
emit startModbus();