write on serial port in thread

[Hamed]

Hi. I faced a strange problem using serial port.
I can't write on serial port by a thread loop!!
however I can write on it by thread constructor!
I can read from serial port in thread but I can't write on it. I don't know where is the problem but by my personal experience in using QSerialPort I think Serial port in Qt have some issues with thread! because it's not my firs problem with QSerialPort and QThread together. maybe I'm wrong but I wrote this code before with c# and this sort of things are not issue there!
thanks in advices.

[SGaist]

Hi and welcome to devnet,

You are probably experiencing thread affinity problems. Use the worker object approach described in QThread's documentation in order to have your serial port in the correct thread to work with.

Hope it helps

[jalomic]

@Hamed said:

QSerialPort

SGaist right !
Probably you construct QSerialPort in QThread's constructor. Its mean that serial port is owned by main thread. Try to construct QSerialPort inside new thread function, or use moveToThread function

[Hamed]

I think you both right but I have two troubles to understand this.
first what is different between this two way to use thread in this case, I mean how moveToThread can help?
second I don't know how to use this worker approach and serial port correctly together.
would you please give me a short example for doing this?

[SGaist]

Creating your serial port instance in the constructor of the QThread subclass means it's created in the thread calling the constructor (in your case probably the main thread). The run method is running in the thread managed by QThread which means that you are using a serial port which has affinity with a different thread than the one that runs "run". The moveToThread function moves a QObject from the thread that created the object to the thread that is handled by QThread.

QThread's documentation shows the worker object approach. In your case create an object that will contain your QSerialPort (don't forget to set it as the parent of the QSerialPort) and that will handle the communication/processing you need for your application.

[Hamed]

@SGaist
thank you dear SGaist for your answers.
I did this as you said but it seems it not the problem. maybe I'm doing this wrong.
working with serial port in qt became a huge trouble for me! I can't figure out how to do this correctly! I need to read from arduino and write missions on it but in both sides I stocked! in reading I have run time errors after 2 or 3 hours and I had no success on writing! someone told me to not use thread and use Qt examples but managing serial port in this baud rate is a blocking operation and I have to use thread! and no matter how many times I try it's not working as it expected!! I'm saying again maybe I'm doing it all wrong but I can't find an example for this! :(

[SGaist]

Then can you describe how the communication should work ?
Can you share your current code ?

[Hamed]

yes I can. I know this is off topic but first I put my reading part of code that crashes after a while.
this is reading and managing part :

QByteArray header;
quint8 ID;
quint8 msg_len;
quint8 first;
quint8 second;
quint8 seq;
QByteArray payload;
void ReaderThread::read()
{//1
while(serial->isOpen())
{//2
if(serial->bytesAvailable() >= 200)
{//3
QThread::msleep(1);
header = serial->read(1);
if(header.contains(254))
{//4
QThread::msleep(5);
header.append(serial->read(5));
msg_len = header[1];
seq = header[2];

           first = header[3];
           second = header[4];
           ID = header[5];
           if(msg_len != NULL && first != NULL && second != NULL && ID != NULL && seq != NULL)
           {//5
               if(msg_len > 0 && msg_len < 100)
               {//6
                   if((first == 01) && (second == 01))
                   {//7
                       if(seq >= 0 && seq <=254)
                       {
                           packet_counter++;
                       }
                       if(seq == 255)
                       {
                           packet_counter++;
                           if(packet_counter <= 256)
                           {
                                rssi = (packet_counter*100)/256;
                                emit mRssi(rssi);
                           }
                           packet_counter = 1;
                       }
                       QThread::msleep(12);
                       payload = serial->read(msg_len);
                       switch (ID)
                       {
                           case (ATTITUDE) :
                               {
                                   mavlink_attitude_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_attitude_t));
                                   emit attitude(data);
                               }

                               break;
                           case (SCALED_PRESSURE) :
                               {
                                   mavlink_scaled_pressure_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_scaled_pressure_t));
                                   emit scaled_pressure(data);
                               }
                               break;
                           case (SENSOR_OFFSETS) :
                               {
                                   mavlink_sensor_offsets_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_sensor_offsets_t));
                                   emit sensor_offsets(data);
                               }
                               break;
                           case (SYS_STATUS) :
                               {
                                   mavlink_sys_status_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_system_time_t));
                                   emit sys_status(data);
                               }
                               break;

                           case (GPS_RAW_INT) :
                               {
                                   mavlink_gps_raw_int_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_gps_raw_int_t));
                                   emit gps_raw_int(data);
                               }
                               break;
                           case (SERVO_OUTPUT_RAW) :
                               {
                                   mavlink_servo_output_raw_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_servo_output_raw_t));
                                   emit servo_output_raw(data);
                               }
                               break;
                           case (RC_CHANNELS_RAW) :
                               {
                                   mavlink_rc_channels_raw_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_rc_channels_raw_t));
                                   emit rc_channels_raw(data);
                               }
                               break;
                           case (WIND) :
                               {
                                   mavlink_wind_t data;
                                   memcpy(&data,payload.data(),sizeof(mavlink_wind_t));
                                   emit wind(data);
                               }
                               break;
                       }
                   }//7
                   else
                   {//8
                       header.clear();
                   }//8
               }//6
           }//5

       }//4
       else
       {//9
           header.clear();
       }//9
   }//3
}//2

}//1

if there is something unclear tell me to explain it.
in the writing part I want to send packets.
one of them is heart bit that I need to send it every second for wireless connection.
and the other packets are including gps coordinates for giving missions to plane.
I'm working on GCS for UAV. every part of program is completed but this part! and it's the most important one!

[SGaist]

I suppose the communication follows a protocol like:

• message start
• header
• data
• maybe message end
  right ?

If so you should rather use the asynchronous method, cumulate the data in a buffer and once you have a enough data, parse them and do what you must with them.

Using a QTimer, you can send at regular interval the heat beat

[Hamed]

yes this is the protocol.
by asynchronous you mean without thread?
I tried to do so but it has a huge delay!!
I'm reading in 115200 baud rate from arduino (ardupilot) and I think this baud rate force me to use thread!
Am I wrong?

[SGaist]

No, I mean using Signals and Slots.

The baud rate doesn't mean you need a thread, you'll have to check that once you can benchmark the load of your application. First thing to do is get the communication working, then and only then consider threading.

[Hamed]

Ok I will test this once again in the morning. it's 1:00am in my country. I don't have access to arduino.
sorry for wasting your time dear SGaist and thank you for your responses :)

[kuzulis_]

Do not use it in thread if you do not know what do you do (and how it works). Please, see the QSerialPort examples and documentation, and the Qt documentation (about signals/slots, reentrant, event-loop) at first. And then, please provide a complete code how you creates and works with QSerialPort.

Because I already see that you use QThread && QSerialPort wrong.

by asynchronous you mean without thread?
I tried to do so but it has a huge delay!!

What you tried ? I do not see any code.

[Hamed]

@kuzulis_
Yes I used QThread and QSerialPort wrong together but it's not mean that I don't know what I'm doing!
I'm newbie in qt and still have trouble to use some of qt libraries.
but you are right. using thread was wrong at the first place! I didn't need it. Thanks to SGaist I realize it now!
I tried without thread before and it has delay. today I figured out using readyRead() signal is not my solution!
I used QTimer and 10 milliseconds for connecting to parser function(slot) and it's solved my problem for good.
thank you for your response. :)

[SGaist]

Can you show the last version of your working code ?

[Hamed]

@SGaist
of course!
but I cant find how to show it as code in this new forum and I also cant find solved button!
anyway here is the code :

Reader::Reader(QSerialPort *parent) :
    QSerialPort(parent)
{
    serial = new QSerialPort;

    foreach (const QSerialPortInfo &serialPortInfo, QSerialPortInfo::availablePorts())
    serial->setPortName(serialPortInfo.portName());

    serial->setBaudRate(QSerialPort::Baud115200);

    serial->setDataBits(QSerialPort::Data8);

    serial->setParity(QSerialPort::NoParity);

    serial->setFlowControl(QSerialPort::NoFlowControl);

    serial->setStopBits(QSerialPort::OneStop);

    serial->open(QIODevice::ReadWrite);

    QTimer *timer = new QTimer(this);
    connect(timer , SIGNAL(timeout()) , this , SLOT(read()));
    timer->start(5);
}


int packet_counter = 0;
int rssi = 2;
QByteArray header;
QByteArray payload;
quint8 ID;

void Reader::read()
{//1
   if(serial->bytesAvailable() >= 10)
   {//3
       header = serial->read(1);
       if(header.contains(254))
       {//4
           header = serial->read(5);
           seq = header[1];
           if((header.at(2) == 01) && (header.at(3) == 01))
           {//7
               ID = header.at(4);
               if(seq >= 0 && seq <=254)
               {
                   packet_counter++;
               }
               if(seq == 255)
               {
                   packet_counter++;
                   if(packet_counter <= 256)
                   {
                        rssi = (packet_counter*100)/256;
                        emit mRssi(rssi);
                   }
                   packet_counter = 1;
               }
               payload = serial->read(header.at(0));
               switch (ID)
               {
                   case (ATTITUDE) :
                       {
                           mavlink_attitude_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_attitude_t));
                           emit attitude(data);
                       }
                       break;
                   case (SCALED_PRESSURE) :
                       {
                           mavlink_scaled_pressure_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_scaled_pressure_t));
                           emit scaled_pressure(data);
                       }
                       break;
                   case (SENSOR_OFFSETS) :
                       {
                           mavlink_sensor_offsets_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_sensor_offsets_t));
                           emit sensor_offsets(data);
                       }
                       break;
                   case (SYS_STATUS) :
                       {
                           mavlink_sys_status_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_system_time_t));
                           emit sys_status(data);
                       }
                       break;

                   case (GPS_RAW_INT) :
                       {
                           mavlink_gps_raw_int_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_gps_raw_int_t));
                           emit gps_raw_int(data);
                       }
                       break;
                   case (SERVO_OUTPUT_RAW) :
                       {
                           mavlink_servo_output_raw_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_servo_output_raw_t));
                           emit servo_output_raw(data);
                       }
                       break;
                   case (RC_CHANNELS_RAW) :
                       {
                           mavlink_rc_channels_raw_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_rc_channels_raw_t));
                           emit rc_channels_raw(data);
                       }
                       break;
                   case (WIND) :
                       {
                           mavlink_wind_t data;
                           memcpy(&data,payload.data(),sizeof(mavlink_wind_t));
                           emit wind(data);
                       }
                       break;
               }
           }//7
           else
           {//8
               header.clear();
           }//8

       }//4
       else
       {//9
           header.clear();
       }//9
   }//3
}//1

this is the reading part. I also tested writing and it's worked just fine. writing part is not complete yet. I will put that here as soon as it's completed.
thanks again SGaist. :)

[edit: Added missing coding tags SGaist]

[SGaist]

Why is Reader a QSerialPort ?

Also, your loop in the constructor opens every serial port of your computer

[Hamed]

@SGaist
What should it be?
Yes I should create a list for available ports and let user choose. it's still not complete. I should work more on details yet.

[SGaist]

Use e.g. a default value that you know is working and add a configuration dialogue

[Hamed]

@SGaist
Did it :)
Thank you :)

[SGaist]

There's also QSettings to store that value so you can reuse it on next startup, that will avoid your user to change it each time they load your application if the port is not the same as yours

[Hamed]

@SGaist
I think if I store available ports string in a combo box will do this for me. right?