Sending a sequence of commands and waiting for a response

Question

I need to update firmware and settings on a device connected to a serial port. Since this is done using a sequence of commands, I send a command and wait until I get a response. Inside the answere (many lines), I am looking for a line that indicates whether the operation completed successfully.

Serial->write("boot", 1000); Serial->waitForKeyword("boot successful"); Serial->sendFile("image.dat"); … 

So, Ive created a new thread for this read / write lock method. Inside the stream, I use the waitForX () functions. If I call watiForKeyword (), it will call readLines () until it finds a keyword or timeout

 bool waitForKeyword(const QString &keyword) { QString str; // read all lines while(serial->readLines(10000)) { // check each line while((str = serial->getLine()) != "") { // found! if(str.contains(keyword)) return true; } } // timeout return false; } 

readLines () reads all available and splits it into lines, each line is placed inside a QStringList and to get a line I call getLine (), which returns the first line in the list and removes it.

 bool SerialPort::readLines(int waitTimeout) { if(!waitForReadyRead(waitTimeout)) { qDebug() << "Timeout reading" << endl; return false; } QByteArray data = readAll(); while (waitForReadyRead(100)) data += readAll(); char* begin = data.data(); char* ptr = strstr(data, "\r\n"); while(ptr != NULL) { ptr+=2; buffer.append(begin, ptr - begin); emit readyReadLine(buffer); lineBuffer.append(QString(buffer)); // store line in Qstringlist buffer.clear(); begin = ptr; ptr = strstr(begin, "\r\n"); } // rest buffer.append(begin, -1); return true; } 

The problem is that I send the file through the terminal to check the readLines () application will read only part of the smale file (5 lines or so). Because these lines do not contain a keyword. the function will start again, but this time dosnt is waiting for a timeout, readLines immediately returns false immediately. What's wrong? Also I'm not sure if this is the right approach ... Does anyone know how to send a sequence of commands and wait for an answer every time?

Answer 1

Use QStateMachine to make it simple. Recall how you would like this code to look:

 Serial->write("boot", 1000); Serial->waitForKeyword("boot successful"); Serial->sendFile("image.dat"); 

Put it in a class that has explicit state members for each state that a programmer can be in. We will also have send , expect , etc. action generators that attach the given actions to the states.

 // https://github.com/KubaO/stackoverflown/tree/master/questions/comm-commands-32486198 #include <QtWidgets> #include <private/qringbuffer_p.h> #include <type_traits> [...] class Programmer : public StatefulObject { Q_OBJECT AppPipe m_port { nullptr, QIODevice::ReadWrite, this }; State s_boot { &m_mach, "s_boot" }, s_send { &m_mach, "s_send" }; FinalState s_ok { &m_mach, "s_ok" }, s_failed { &m_mach, "s_failed" }; public: Programmer(QObject * parent = 0) : StatefulObject(parent) { connectSignals(); m_mach.setInitialState(&s_boot); send (&s_boot, &m_port, "boot\n"); expect(&s_boot, &m_port, "boot successful", &s_send, 1000, &s_failed); send (&s_send, &m_port, ":HULLOTHERE\n:00000001FF\n"); expect(&s_send, &m_port, "load successful", &s_ok, 1000, &s_failed); } AppPipe & pipe() { return m_port; } }; 

This is fully functional, complete code for the programmer! Completely asynchronous, non-blocking, and it also handles timeouts.

It is possible to create an infrastructure that generates states on the fly, so you do not need to manually create all the states. The code is much smaller and IMHO is easier to understand if you have explicit states. Only for complex communication protocols with 50-100 + states would it be wise to get rid of explicit named states.

AppPipe is a simple in- AppPipe bidirectional channel that can be used as a backup for a real serial port:

 // See http://stackoverflow.com/a/32317276/1329652 /// A simple point-to-point intra-process pipe. The other endpoint can live in any /// thread. class AppPipe : public QIODevice { [...] }; 

StatefulObject contains a state machine, some basic signals useful for monitoring the state of a state machine, and the connectSignals method used to connect signals to states:

 class StatefulObject : public QObject { Q_OBJECT Q_PROPERTY (bool running READ isRunning NOTIFY runningChanged) protected: QStateMachine m_mach { this }; StatefulObject(QObject * parent = 0) : QObject(parent) {} void connectSignals() { connect(&m_mach, &QStateMachine::runningChanged, this, &StatefulObject::runningChanged); for (auto state : m_mach.findChildren<QAbstractState*>()) QObject::connect(state, &QState::entered, this, [this, state]{ emit stateChanged(state->objectName()); }); } public: Q_SLOT void start() { m_mach.start(); } Q_SIGNAL void runningChanged(bool); Q_SIGNAL void stateChanged(const QString &); bool isRunning() const { return m_mach.isRunning(); } }; 

State and FinalState are simple Qt 3-style namespace wrappers. They allow us to declare a state and give it a name at a time.

 template <class S> struct NamedState : S { NamedState(QState * parent, const char * name) : S(parent) { this->setObjectName(QLatin1String(name)); } }; typedef NamedState<QState> State; typedef NamedState<QFinalState> FinalState; 

Action generators are also quite simple. The meaning of the action generator is "to do something when a given state is entered." The state that needs to act is always given as the first argument. The second and subsequent arguments are specific to the action. Sometimes an action may also need a target state, for example. if he succeeds or fails.

 void send(QAbstractState * src, QIODevice * dev, const QByteArray & data) { QObject::connect(src, &QState::entered, dev, [dev, data]{ dev->write(data); }); } QTimer * delay(QState * src, int ms, QAbstractState * dst) { auto timer = new QTimer(src); timer->setSingleShot(true); timer->setInterval(ms); QObject::connect(src, &QState::entered, timer, static_cast<void (QTimer::*)()>(&QTimer::start)); QObject::connect(src, &QState::exited, timer, &QTimer::stop); src->addTransition(timer, SIGNAL(timeout()), dst); return timer; } void expect(QState * src, QIODevice * dev, const QByteArray & data, QAbstractState * dst, int timeout = 0, QAbstractState * dstTimeout = nullptr) { addTransition(src, dst, dev, SIGNAL(readyRead()), [dev, data]{ return hasLine(dev, data); }); if (timeout) delay(src, timeout, dstTimeout); } 

The hasLine test simply checks all the lines that can be read from the device for this needle. This is great for this simple communication protocol. You will need a more sophisticated technique if your messages are more active. You must read all the lines, even if you find your needle. This is because this test is called from the readyRead signal, and in this signal you should read all the data that matches the selected criterion. The criterion here is that the data forms a complete row.

 static bool hasLine(QIODevice * dev, const QByteArray & needle) { auto result = false; while (dev->canReadLine()) { auto line = dev->readLine(); if (line.contains(needle)) result = true; } return result; } 

Adding secure state transitions is a bit cumbersome with the default API, so we will wrap it in order to simplify its use and so that action generators can read above:

 template <typename F> class GuardedSignalTransition : public QSignalTransition { F m_guard; protected: bool eventTest(QEvent * ev) Q_DECL_OVERRIDE { return QSignalTransition::eventTest(ev) && m_guard(); } public: GuardedSignalTransition(const QObject * sender, const char * signal, F && guard) : QSignalTransition(sender, signal), m_guard(std::move(guard)) {} GuardedSignalTransition(const QObject * sender, const char * signal, const F & guard) : QSignalTransition(sender, signal), m_guard(guard) {} }; template <typename F> static GuardedSignalTransition<F> * addTransition(QState * src, QAbstractState *target, const QObject * sender, const char * signal, F && guard) { auto t = new GuardedSignalTransition<typename std::decay<F>::type> (sender, signal, std::forward<F>(guard)); t->setTargetState(target); src->addTransition(t); return t; } 

What about this - if you had a real device, that’s all you need. Since I don't have a device, I will create another StatefulObject to emulate the intended behavior of the device:

 class Device : public StatefulObject { Q_OBJECT AppPipe m_dev { nullptr, QIODevice::ReadWrite, this }; State s_init { &m_mach, "s_init" }, s_booting { &m_mach, "s_booting" }, s_firmware { &m_mach, "s_firmware" }; FinalState s_loaded { &m_mach, "s_loaded" }; public: Device(QObject * parent = 0) : StatefulObject(parent) { connectSignals(); m_mach.setInitialState(&s_init); expect(&s_init, &m_dev, "boot", &s_booting); delay (&s_booting, 500, &s_firmware); send (&s_firmware, &m_dev, "boot successful\n"); expect(&s_firmware, &m_dev, ":00000001FF", &s_loaded); send (&s_loaded, &m_dev, "load successful\n"); } Q_SLOT void stop() { m_mach.stop(); } AppPipe & pipe() { return m_dev; } }; 

Now let's make everything well visualized. We will have a window with a text browser displaying the contents of the messages. Below are the start / stop buttons for the programmer or device, as well as labels indicating the status of the emulated device and programmer:

 int main(int argc, char ** argv) { using Q = QObject; QApplication app{argc, argv}; Device dev; Programmer prog; QWidget w; QGridLayout grid{&w}; QTextBrowser comms; QPushButton devStart{"Start Device"}, devStop{"Stop Device"}, progStart{"Start Programmer"}; QLabel devState, progState; grid.addWidget(&comms, 0, 0, 1, 3); grid.addWidget(&devState, 1, 0, 1, 2); grid.addWidget(&progState, 1, 2); grid.addWidget(&devStart, 2, 0); grid.addWidget(&devStop, 2, 1); grid.addWidget(&progStart, 2, 2); devStop.setDisabled(true); w.show(); 

We will connect the device and programmer AppPipe s. We also visualize that the programmer sends and receives:

  dev.pipe().addOther(&prog.pipe()); prog.pipe().addOther(&dev.pipe()); Q::connect(&prog.pipe(), &AppPipe::hasOutgoing, &comms, [&](const QByteArray & data){ comms.append(formatData(">", "blue", data)); }); Q::connect(&prog.pipe(), &AppPipe::hasIncoming, &comms, [&](const QByteArray & data){ comms.append(formatData("<", "green", data)); }); 

Finally, we will connect the buttons and labels:

  Q::connect(&devStart, &QPushButton::clicked, &dev, &Device::start); Q::connect(&devStop, &QPushButton::clicked, &dev, &Device::stop); Q::connect(&dev, &Device::runningChanged, &devStart, &QPushButton::setDisabled); Q::connect(&dev, &Device::runningChanged, &devStop, &QPushButton::setEnabled); Q::connect(&dev, &Device::stateChanged, &devState, &QLabel::setText); Q::connect(&progStart, &QPushButton::clicked, &prog, &Programmer::start); Q::connect(&prog, &Programmer::runningChanged, &progStart, &QPushButton::setDisabled); Q::connect(&prog, &Programmer::stateChanged, &progState, &QLabel::setText); return app.exec(); } #include "main.moc" 

Programmer and Device can live in any thread. I left them in the main thread, since there is no reason to pull them out, but you can put both in a dedicated stream or each in your own stream or in streams shared with other objects, etc. It is completely transparent because AppPipe supports AppPipe . This will also be the case if AppPipe used instead of QSerialPort . All that matters is that each QIODevice instance is used from only one thread. Everything else happens through signal / slot connections.

eg. if you want Programmer live in a dedicated thread, you must add the following to main :

  // fix QThread brokenness struct Thread : QThread { ~Thread() { quit(); wait(); } }; Thread progThread; prog.moveToThread(&progThread); progThread.start(); 

A little helper formats the data to make it easier to read:

 static QString formatData(const char * prefix, const char * color, const QByteArray & data) { auto text = QString::fromLatin1(data).toHtmlEscaped(); if (text.endsWith('\n')) text.truncate(text.size() - 1); text.replace(QLatin1Char('\n'), QString::fromLatin1("<br/>%1 ").arg(QLatin1String(prefix))); return QString::fromLatin1("<font color=\"%1\">%2 %3</font><br/>") .arg(QLatin1String(color)).arg(QLatin1String(prefix)).arg(text); } 

Answer 2

I am not sure if this is the right approach.

You are polling with waitForReadyRead() . But since the serial port is a QIODevice , it emits a void QIODevice::readyRead() signal when something arrives at the serial port. Why not connect this signal to your syntax input code? There is no need for waitForReadyRead() .

Also, on the other hand: "... this time it does not wait for a timeout, readLines immediately returns false immediately. What's wrong?"

Indication of documentation:

If waitForReadyRead () returns false, the connection was closed or an error occurred.

(emphasis is mine) In my experience as a built-in developer, it is possible that you put the device in a kind of “firmware update mode”, and thus, the device rebooted into a special boot mode (without starting the firmware you are going to update to) and thus closed the connection. It is impossible to say if this is not documented / you have contact with device developers. It is not so obvious to check using a serial terminal to type your commands and testify about it, I use minicom , connected to my devices daily, and it is quite resistant to rebooting - good for me.