[SOLVED] QSerialPort - Multiple Write/Read
-
I am working on a project that controls a serial device, specifically an INSTEON PowerLinc Modem. The device connects to the COM port and interfaces with serial commands. I am using QSerialPort to send commands, up to this point I have been okay just sending and doing some inefficient error checking to ensure it sent. Now I need to send a request and actually digest the response. I have been testing the send/receive using DockLight (http://www.docklight.de). When a message is sent successfully to the PowerLinc modem, it replies with an ACK byte (0x06). The following is sent to query devices on the network to receive their state:
02621DE94B051900
The correct response is:
02621DE94B05190006025020CBCF1EDAF72100FFMy system may have any number of devices registered in the database. My program saves these devices into a QList of "Devices", the list is iterated through. Whenever the type is determined to be a "Light", the program structures a QByteArray message to send to the device using the serial port. I am using the QSerialPort example of "BlockingMaster" to create a thread to handle the write and read using the serial port. This action is referenced here:
@void Device::currentStatus(QList<Device *> * deviceList){
QString devID, updateQry;
int devStatus, updateStatus;
updateStatus=0;
QSqlQuery query;
for(int i=0; i<deviceList->size(); i++){
if(deviceList->at(i)->type == "Light"){
devStatus = deviceList->at(i)->status;
devID = deviceList->at(i)->deviceID;
QByteArray msg;
bool msgStatus;
msg.resize(8);msg[0] = 0x02; msg[1] = 0x62; msg[2] = 0x00; msg[3] = 0x00; msg[4] = 0x00; msg[5] = 0x05; msg[6] = 0x19; msg[7] = 0x00; msg.replace(2, 3, QByteArray::fromHex( devID.toLocal8Bit() ) ); qDebug() << "Has device " << deviceList->at(i)->name << "Changed?"; //send(msg,&msgStatus, &updateStatus); //msg.clear(); thread.setupPort("COM3",500,msg); if(devStatus!=updateStatus){ qDebug() << deviceList->at(i)->name << " is now: " << updateStatus; updateStatus = !updateStatus; } } }
}@
"setupPort" is used to initialize and run the serial port IO
@void serialThread::setupPort(const QString &portName, int waitTimeout, const QByteArray &msg){
qDebug() << "Send Message " << msg.toHex();
QMutexLocker locker(&mutex);
this->portName = portName;
this->waitTimeout = waitTimeout;
this->msg = msg;
if(!isRunning())
start();
else
cond.wakeOne();
}@The Run function handles the sending and receiving. I am using a slightly modified of the Run function used in BlockingMaster, instead
of the requested write message being a QString, it is already a QByteArray.When the thread emits the signal "response", it is handled by the connected slot "handleResponse", which simply prints the response using qDebug.
Currently I receive the following output:
@Has device "Living Room Light" Changed?
Send Message "02621de94b051900"
Has device "Bedroom Light" Changed?
Send Message "026220cbcf051900"
Thread executed
Read: "026220cbcf05190006"
Polling for changes...
Has device "Living Room Light" Changed?
Send Message "02621de94b051900"
Has device "Bedroom Light" Changed?
Send Message "026220cbcf051900"
Read: "025020cbcf1edaf721000002621de94b05190006"
Polling for changes...
Has device "Living Room Light" Changed?
Send Message "02621de94b051900"
Has device "Bedroom Light" Changed?
Send Message "026220cbcf051900"
Read: "02501de94b1edaf72100ff02621de94b05190006" @There are two devices in the QList:
Living Room Light: 1DE94B
Bed Room Light: 20CB2FBoth messages are sending, but I am only getting a read response for the living room light. What is even more strange is that the first read only returns the response up to "026220cbcf05190006" and on the next read, I receive "025020cbcf1edaf721000002621de94b05190006". I should receive:
"026220cbcf05190006" + "025020cbcf1edaf7210000". The next read should then be "02621de94b05190006" + "02501de94b1edaf72100ff"I never see a read response for the bedroom light. How should I modify this so that a write/read is performed for each device as I iterate through the QList?
-
I mentioned using a slightly modified version of Run from BlockingMaster - below is my modified Run function:
@void serialThread::run(){
bool currentPortNameChanged = false;
qDebug() << "Thread executed";
mutex.lock();
QString currentPortName;
if(currentPortName != portName){
currentPortName = portName;
currentPortNameChanged = true;
}int currentWaitTimeout = waitTimeout; QByteArray sendMsg = msg; mutex.unlock(); QSerialPort serial; while(!quit){ if(currentPortNameChanged){ serial.close(); serial.setPortName("COM3"); if (!serial.open(QIODevice::ReadWrite)) { emit error(tr("Can't open %1, error code %2") .arg(portName).arg(serial.error())); return; } if (!serial.setBaudRate(QSerialPort::Baud19200)) { emit error(tr("Can't set baud rate 9600 baud to port %1, error code %2") .arg(portName).arg(serial.error())); return; } if (!serial.setDataBits(QSerialPort::Data8)) { emit error(tr("Can't set 8 data bits to port %1, error code %2") .arg(portName).arg(serial.error())); return; } if (!serial.setParity(QSerialPort::NoParity)) { emit error(tr("Can't set no patity to port %1, error code %2") .arg(portName).arg(serial.error())); return; } if (!serial.setStopBits(QSerialPort::OneStop)) { emit error(tr("Can't set 1 stop bit to port %1, error code %2") .arg(portName).arg(serial.error())); return; } if (!serial.setFlowControl(QSerialPort::NoFlowControl)) { emit error(tr("Can't set no flow control to port %1, error code %2") .arg(portName).arg(serial.error())); return; } } //write request serial.write(msg); if (serial.waitForBytesWritten(waitTimeout)) { //! [8] //! [10] // read response if (serial.waitForReadyRead(currentWaitTimeout)) { QByteArray responseData = serial.readAll(); while (serial.waitForReadyRead(10)){ responseData += serial.readAll(); } QByteArray response = responseData; //! [12] emit this->sendResponse(response); //! [10] //! [11] //! [12] } else { emit this->timeout(tr("Wait read response timeout %1") .arg(QTime::currentTime().toString())); } //! [9] //! [11] } else { emit timeout(tr("Wait write request timeout %1") .arg(QTime::currentTime().toString())); } mutex.lock(); cond.wait(&mutex); if (currentPortName != portName) { currentPortName = portName; currentPortNameChanged = true; } else { currentPortNameChanged = false; } currentWaitTimeout = waitTimeout; sendMsg = msg; mutex.unlock(); } serial.close();
}@
-
Blocking approach is not a good solution, is better - need to use anync approach with
A little unclear to me:
- To one COM port are physically connected two or more devices?
- What used physical interface: RS-232, 485 or something else?
- Give a minimum working project which reproduces the problem (so I could build it), because your pieces of code does not give me the full picture.
-
I am sorry I can see that I certainly left out some crucial details.
1. To one COM port are physically connected two or more devices?
To the COM port (COM3 in this instance) there is one physically connected device. The device is an INSTEON powerlinc modem, used as a controller in a home automation system - "INSTEON PLM 2413U":http://www.insteon.com/pdf/2413Uqs.pdf. The device specifics can be related to this technical manual for a previous issue of the device. "Technical Reference Manual":http://www.aartech.ca/docs/2412sdevguide.pdfHow I use the device - INSTEON messages are standard of 8 bytes. Actions can be sent to the INSTEON PLM in the form of 8Byte HEX messages across the serial port, based upon the destination device address/ID the PLM forwards INSTEON message to the target device (example: light switch).
2. What used physical interface: RS-232, 485 or something else?
Physical interface is Serial USB, the USB - Serial adapter on the device is RS-232.
3. Give a minimum working project which reproduces the problem (so I could build it), because your pieces of code does not give me the full picture.
My project is available at "This Github Repo":https://github.com/homelogic/app
Currently the project loads the device list from a database. Device status (light on/off) in the database can be changed from a website. Every 2-3 seconds the program checks the current status of the database to identify if any devices have been modified. If they have been modified the correct action to be taken will be identified. For each action a QByteArray is structured and sent as "msg" to the send function.
Send uses QSerialPort to open serial port (COM3) and send the message. This is currently working using the nonblocking "Send" function in Device.cpp.
What I need to be able to do:
I need to be able to poll each light switch to identify its current status. This can be used to see if the switch has been thrown manually and to check to ensure the device status did change when requested. To do this I am going to have to build a more thought through implementation to send messages. serialThread.cpp highlights my current approach to send messages using the BlockingMaster example. I have tried using variables/flags for establishing synchronous control, but have failed.
Thank you for your help, please let me know if I can provide more clarification!
-
You're too is complicated, lumped together and I do not understand your code. And to be honest - that I have no desire to dig your code.
-
Use async approach as Master example (non blocking).
-
Try simplify the code, send one-by-one request individually (e.g. by button click) by hand and make sure you get the necessary response packages from desired device.
-
As I can see from your log - you send multiple requests to the devices (via one COM3), and then wait for them to answer. But it is correct behavior for your automation system protocol? Maybe need send one command to one device and wait for response from it, and after receive response - send request to other device and etc by ordered? Maybe there are conflicts / collision, when you're trying to get an answer from several devices at the same time?
-
-
That is certainly reasonable, I do not expect you to try to sift through and understand what I am thinking in every position :) . I appreciate your efforts to this point!
Last night I was trying to figure out the handshaking. I will investigate the Master non-blocking example.
Your suggestion aligns with my intent. My goal is to do the following two routines:
- Change device status
- Every 2-3 seconds check the status of each device in the database.
- Check device, if it should be updated:
- Send Serial Message to initiate action
- Check response to ensure action has occurred
- Repeat (b) for each device
- Poll Devices
- Every 5 seconds, check the status of each device
- Send query message to device
- Receive response from device - handle response
- Repeat b-c for each device
For this method you believe Asynchronous Non-Blocking Serial Communication would be the best approach?
-
bq. For this method you believe Asynchronous Non-Blocking Serial Communication would be the best approach?
Yes, I'm sure!
Asynchronous approach is much easier to implement: does not require mutexes, threads, etc. which is very easy to make a mistake when implementation.
-
Kuzulis, my code is performing much better. However, I am still experiencing a problem with queuing the write requests. As I try to dispatch multiple write requests, only the first executes, the remaining requests do not wait until the first one finishes to execute. What is the proper way to setup this type of "queuing"?
I believe something like this is what I would want to implement, but it sends the program into an infinite loop:
@while (!sent){
if(readySend){
emit writeRequest(msg);
readySend = false;
}
else{
Sleep(50);
}
}
// Now that we have sent the message, reset the readySend flag
readySend = true; //Ready to send next message@When serial.write(msg) is called, a singleshot timer is started for 500ms. Once it expires, processTimeout receives the timeout signal.
Sent
flag is set to true - should now exit the while loop.
@
void Device::processTimeout(){
qDebug() << "Read: " << response.toHex();
sent = true;
response.clear();
}@I believe my problem to be in the while loop. My logic is not being realized, the message sends, but the timeout is not received and processed by the "processTimeout" slot. My loop is blocking this.
I apologize for what probably seems very remedial programming problems.
-
I see, that you use Windows OS, so you can install serial port sniffer (e.g. free serial port monitor) and really see that happens to the data streams. This way you can find out the cause of your problems.
Once again, asynchronous approach - is the best solution for any problem. Asynchronous Master sample - this is only a simplest example, which should not be taken literally.
I got the impression that you do not understand some features of the behavior of the class, and in general serial programming.
Why did you add Sleep() where there is not necessary? Why did you use threads where not needed? Why do not you want to simplify your example and write a simple test application, simply to send multi-requests (with button click) with the simple expectation of responses, in order to make sure that it works?
E.g. why you can not just do:
@
MyClass::MyClass()
{
connect(button, SIGNAL(clicked()), this, SLOT(onClick()));
connect(port, SIGNAL(readyRead()), this, SLOT(onRead()));
}MyClass::onClick()
{
QByteArray request1;
request1 << blablabla1;QByteArray request2; request2 << blablabla2; port->write(request1); port->write(request2);
}
MyClass::onRead()
{
qDebug() << port->readAll().toHex();
}@
to check?
-
Kuzulis, I appreciate the example. Let me say, the code I posted including "wait" was posted more so as an example of what I am trying to implement, not the exact implementation. I did do basic testing by sending 1 message at a time. I get the exact response I anticipate regularly (solves 1 problem I previously had).
1 Transaction:
@20:44:30:666 STATUS_SUCCESS 02 62 1d e9 4b 05 19 00 <--- Send
20:44:30:669 STATUS_SUCCESS 02 62 1d e9 4b 05 19 00 06 <--- Receive
20:44:30:875 STATUS_SUCCESS 02 <--- Receive
20:44:30:881 STATUS_SUCCESS 50 1d e9 4b 1e da f7 21 00 ff <--- Receive@Sent: 02 62 1d e9 4b 05 19 00
Received: 02 62 1d e9 4b 05 19 00 06 02 50 1d e9 4b 1e da f7 21 00 ff (Correct)Now, if I send multiple messages, I never receive a correct response.
1 Transaction
@20:51:01:720 STATUS_SUCCESS 02 62 1d e9 4b 05 19 00 <--- Send
20:51:01:721 STATUS_SUCCESS 02 62 20 cb cf 05 19 00 <--- Send
20:51:01:822 STATUS_SUCCESS 02 62 1d e9 4b 05 19 00 02 62 20 <--- Receive
20:51:01:825 STATUS_SUCCESS cb cf 05 19 00 15 (0x15 = NACK) <--- Receive
20:51:02:227 STATUS_SUCCESS 02 62 20 cb cf 05 19 00 <--- Send
20:51:02:229 STATUS_SUCCESS 02 62 20 cb cf 05 19 00 06 <--- Receive
20:51:02:431 STATUS_SUCCESS 02 50 20 cb cf 1e da f7 21 00 ff <--- Receive@Sent: 02 62 1d e9 4b 05 19 00
Sent: 02 62 20 cb cf 05 19 00
Received: 02 62 1d e9 4b 05 19 00 02 62 20 cb cf 05 19 00 15
0x15 is Negative ACK, so the program tries to write the last message again. The last write was 02 62 20 cb cf 05 19 00. Thus, the program continues as:
Sent: 02 62 20 cb cf 05 19 00
Received: 02 62 20 cb cf 05 19 00 06 02 50 20 cb cf 1e da f7 21 00 ff (Good Receive)My problem is that the two writes, one right after another, are arriving to the Modem and appearing as one message. The modem does not know how to handle the message and posts back the reserved NACK byte "0x15".
My intention for using "wait" was to show that I need some method to delay writing the second message until the first message has completed successfully. How can I implement the following:
- Write message 1
- Read Message 1 - Set okay to send again
- Once it is okay to send, Write message 2
- Read Message 2 - Set okay to send again
... etc.
I must check my response to ensure it is valid, before continuing to send the next message. That is why I initially attempted to implement the design using a separate thread. This is my first serial port project, I am sorry for all of the remedial issues - your help has been greatly appreciated! :)
-
Try to send requests via timer QTimer with some period. But with small period is not fact that in reality first request will complete to transfer before next request. So you can check really count of bytes transferred by signal bytesTransferred(int) and if first request is transferred complete, only then run QTimer and when timer triggered - to send next request...
Of course, I'm talking about a full asynchronous approach, without waitForXXX(), without Sleep()...
But, IMHO, is best solution - is send request and wait response by order (and full async approach).
-
[quote author="wakeuky" date="1362708116"]
My problem is that the two writes, one right after another, are arriving to the Modem and appearing as one message. The modem does not know how to handle the message and posts back the reserved NACK byte "0x15".
[/quote]Don't you think, that it could be also a problem of the modem? I also would not use QTimer for your program as kuzulis mentioned, better use (like also kuzulis mentioned) the readyRead() signal on a slot, where you have implemented a state machine which also send data.
So for example
@void ClassName::dataReaded() {static int state = 0;
switch(state) {
( ... ) // Check start string etc.case ACK: if(queue_not_empty) writeNextDataInQueue(); else no_data = true; break; case NACK: writeDataAgain(); break;
}
void ClassName::sendNewData()
{
if(!no_data)
putDataInQueue();
else {
sendData();
no_data=false;
}@
Or something like that :)
EDIT: I use something similar with QNetwork. For serial port, I send the data with no checking for buffer and everything else and use a state machine to check, if the data was send or read the response or something else...
-
Kuzulis and Serenity I appreciate all of your diligent work in helping me solve my problem. I believe I have finally reached a working solution. Serenity, thank you very much for your example - I had tried implementing something similar before but could never get out of deadlock due to my poor semantics in the implementation. Your example was very concise and easy to follow.
Solution:
Setup Signal -> Slot connections + initialize no_data flag
@QObject::connect(&serial, SIGNAL(readyRead()),
this, SLOT(handleResponse()));
QObject::connect(&serialTimer, SIGNAL(timeout()),
this, SLOT(processTimeout()));
QObject::connect(this, SIGNAL(writeRequest(QByteArray)),
this, SLOT(writeSerial(QByteArray)));
no_data = true;
serialTimer.setSingleShot(true);@Private members of Device Class:
@QByteArray response;
QByteArray msgRequest;
QByteArray msgQueue;
bool no_data;@
"Response" stores the bytes read from the serial port.
"msgRequest" stores the current message to be written to the serial port
"no_data" flags whether or not the serial port is available to be written to or if the message should be queued to send at the next available time.
"msgQueue" provides a variable to store messages until the serial port free for use.I have a function that packs messages into a QByteArray, once the message has been packaged, I emit my "writeRequest" signal. This signal connects to "writeSerial" Slot. Note: Below "devID" is a QString in the format of A1B2C3 that is split into the appropriate indices of the QByteArray "msg".
@QByteArray msg;
msg.resize(8);
msg[0] = 0x02;
msg[1] = 0x62;
msg[2] = 0x00;
msg[3] = 0x00;
msg[4] = 0x00;
msg[5] = 0x05;
msg[6] = 0x19;
msg[7] = 0x00;
msg.replace(2, 3, QByteArray::fromHex( devID.toLocal8Bit() ) );
emit writeRequest(msg);@"writeSerial" sets up the Serial Port. If do_data is true, write to the serial port - otherwise, add the message to the end of the current message queue (msgQueue).
@void Device::writeSerial(const QByteArray &msg){
if (serial.portName() != "COM3") {
serial.close();
serial.setPortName("COM3");if (!serial.open(QIODevice::ReadWrite)) { processError(tr("Can't open %1, error code %2") .arg(serial.portName()).arg(serial.error())); return; } // // Setup params here // } if(!no_data){ msgQueue.append(msg); } else{ serial.write(msg); qDebug() << "Write: " << msg.toHex(); no_data=false; }
}@
Whenever the serial port reads bytes in, readyRead() signal is emitted. This signal is connected to "handleResponse". The response is analyzed and assigned a current state, depending upon what we expect and what has been read "handleResponse" will either wait for more bytes to be read or write the next message from the message queue.
@void Device::handleResponse(){
response.append(serial.readAll());
static int state = 0;
int msgSize = response.length();
if(msgSize<9)
state = 0; //not ready
else if(response.at(8)==0x15)
state = 1;
else if( (response.endsWith(0xFF) || response.endsWith(QByteArray(0x00))) && response.contains(0x21))
state = 4;
else if(response.at(8)==(0x06) && response.at(6)==0x19)
state = 3;
else if(response.at(8)==(0x06))
state = 2;
else
state = 0;switch(state){ case 0: serialTimer.start(500); //Not ready - do nothing, wait for more bytes. break; case 1: //NACK serialTimer.stop(); qDebug() << "NACK: " << response.toHex(); response.clear(); serial.write(msgRequest); //write again break; case 2: //ACK serialTimer.stop(); qDebug() << "Read: " << response.toHex(); response.clear(); if(msgQueue.length() > 0) writeNextQuery(); else no_data=true; break; case 3: //ACK of Dev Status //Wait for the rest of Device Status break; case 4: //Status serialTimer.stop(); qDebug() << "Device Status Read: " << response.toHex(); response.clear(); if(msgQueue.length() > 0) this->writeNextQuery(); else no_data=true; break; default: //Do Nothing break; }
}@
"WriteNextQueue" chops the first message in the queue and writes it to the serial port.
@void Device::writeNextQueue(){
int size = msgQueue.length();
msgRequest = msgQueue.left(8);
msgQueue = msgQueue.mid(8,(size-8));
serial.write(msgRequest);
qDebug() << "Write: " << msgRequest.toHex();
no_data=false;
}@I noticed that some messages were never properly received by my Modem. The modem did not respond with an ACK or NACK byte, which would leave me in Case 0 - always waiting for more bytes to be read. Whenever case 0 is matched, I start a singleshot timer "serialTimer" for 500ms. If a timeout() is received by "processTimeout" Slot, I skip to the next message in the queue.
@void Device::processTimeout(){
qDebug() << "Timeout";
response.clear();
if(msgQueue.length() > 0)
this->writeNextQueue();
else
no_data=true;
}@Many thank yous to everyone who assisted me in making my way to this solution! This was a very educational experience, I hope this will help others with their QSerialPort implementations.
EDIT: I tried to clean my post up, if anyone has recommendations as to how the solution should be displayed - please advise.