Reciving data from barcode scanner

Damian7546

Hi,

In qt I have simple receiving data from my barcode scanner.
The received data is in format:
"IR01XXXXXXXX\r\n"

The receive() is my slot from readyread.

I think that my parsing data in slot is not good:

void ScannerSerialWorker::recive()
{
    m_receivedDataTimeout = false;
    m_receivedDataTimeoutTimer.start(20);
    qDebug() << "raw recivedData: " << receivedData;
    while(m_Serial->bytesAvailable() && !m_receivedDataTimeout)
    {
       receivedData.append(m_Serial->readAll());
       if(receivedData.endsWith("\r\n") && receivedData.startsWith("IR01")) {
           receivedFrame = receivedData.remove(receivedData.count()-2,2);

           qDebug() << "Recived frame: " << receivedData.toHex();
           receivedData.clear();
           parseRecivedData();
           m_receivedDataTimeout = false;
           m_receivedDataTimeoutTimer.stop();
        }
    }
    if(m_receivedDataTimeout)
    {
        receivedData.clear();
        m_receivedDataTimeout = false;
        m_receivedDataTimeoutTimer.stop();
    }

}

And timeout is realized by:

connect(&m_receivedDataTimeoutTimer, &QTimer::timeout, this, [&](){m_receivedDataTimeout = true;} );

Someone can help me how write better function, more reliable ?

JonB

@Damian7546
Don't know what you mean by "more reliable".

I wouldn't do the parsing inside the code which repeatedly reads from the serial port. UI would probably have that function emit a signal when a "line" has been received, and let something which handles that do the parsing then. Also have that deal with startsWith("IR01") rather than here. Similarly I wouldn't put a timeout in here. I don't even think that this code will allow the timer to tick to expire/timeout, did you see whether it actually even follows that route?

Damian7546

@JonB said in Reciving data from barcode scanner:

I wouldn't do the parsing inside the code which repeatedly reads from the serial port. UI would probably have that function emit a signal when a "line" has been received, and let something which handles that do the parsing then. Also have that deal with startsWith("IR01") rather than here. Similarly I wouldn't put a timeout in here. I don't even think that this code will allow the timer to tick to expire/timeout, did you see whether it actually even follows that route?

How do you think the above code should appear? please for any example .

JonB

@Damian7546

void ScannerSerialWorker::receiveData()
{
       receivedData.append(m_Serial->readAll());
       if (receivedData.endsWith("\r\n")) {
           receivedFrame = receivedData.remove(receivedData.count()-2,2);
}

Other stuff, like parsing and if you really need to do timeouts (you should not, the "protocol" of the received data from the barcode scanner should indicate this, e.g. by appending \r\n for each "line" if that is what it does), I would do elsewhere in the calling code, not in the readyRead() slot. Maybe emit receivedDataLine() from receiveData() if/when it gets a "complete line" and is ready to parse.

Damian7546

@JonB and what do you think about below solution ?

void ScannerSerialWorker::initSerialPort(QString port)
{
    // Serial Port Initialization
    m_Serial = new QSerialPort(this);
    m_Serial->setPortName(port);
    m_Serial->setBaudRate(QSerialPort::Baud115200);
    m_Serial->setDataBits(QSerialPort::Data8);
    m_Serial->setParity(QSerialPort::NoParity);
    m_Serial->setStopBits(QSerialPort::OneStop);
    m_Serial->setFlowControl(QSerialPort::NoFlowControl);

    if ( m_Serial->open(QIODevice::ReadOnly)){

        QObject::connect(m_Serial, &QSerialPort::readyRead, this, [&]() {
            const QByteArray scannedData = m_Serial->readAll();
            qDebug() << "Scanned data by scanner: " << scannedData;

            receivedData.append(scannedData);
            if(receivedData.endsWith("\r\n")) {
                parseRecivedData();
             }
        });
    }
    qDebug() << "SerialPort for Barcode reader status: " << m_Serial->isOpen();
}

void ScannerSerialWorker::parseRecivedData()
{
    ScannerFrame *m_inFrame = nullptr;
    if (m_inFrame == nullptr)
        m_inFrame = new ScannerFrame();

    else{
        m_inFrame->Clear();
    }

    receivedFrame = receivedData.remove(receivedData.count()-2,2);
    receivedData.clear();

    for (int i=receivedFrame.indexOf("IR01"); i< receivedFrame.count(); i++)
        m_inFrame->AddByte(receivedFrame[i]);

    receivedData.clear();

    emit this->frameReceived(m_inFrame);
    delete m_inFrame;
}

JonB

@Damian7546
This look about right.

Personally I would do the receivedFrame = receivedData.remove(receivedData.count()-2,2); in the readyRead() slot next to where you look for the terminating \r\n as they are linked.

Your code assumes you will only get one "line" any time readyRead() is signalled. I don't know if that is guaranteed: in terms of readyRead() itself it could be fired just once when multiple "lines" arrive. If you care about this possibility you would need to handle it.

Christian Ehrlicher

@JonB said in Reciving data from barcode scanner:

if that is guaranteed

No, it's not - it's a stream like any other stream too.

Damian7546

I gave up with :

 receivedFrame = receivedData.remove(receivedData.count()-2,2);

Now is my code:

void ScannerSerialWorker::initSerialPort(QString port)
{
    // Serial Port Initialization
    m_Serial = new QSerialPort(this);
    m_Serial->setPortName(port);
    m_Serial->setBaudRate(QSerialPort::Baud115200);
    m_Serial->setDataBits(QSerialPort::Data8);
    m_Serial->setParity(QSerialPort::NoParity);
    m_Serial->setStopBits(QSerialPort::OneStop);
    m_Serial->setFlowControl(QSerialPort::NoFlowControl);

    if ( m_Serial->open(QIODevice::ReadOnly)) {
        QObject::connect(m_Serial, &QSerialPort::readyRead, this, [&]() {
            const QByteArray scannedData = m_Serial->readAll();
            qDebug() << "Scanned data by scanner (RAW): " << scannedData;

            receivedData.append(scannedData);
            if(receivedData.contains("\r\n")) {
                parseRecivedData();
                qDebug() << "Scanned data by scanner: " << receivedData;
             }
        });
    }
    qDebug() << "SerialPort for Barcode reader status: " << m_Serial->isOpen();
}

void ScannerSerialWorker::parseRecivedData()
{
    ScannerFrame *m_inFrame = nullptr;
    if (m_inFrame == nullptr)
        m_inFrame = new ScannerFrame();

    else{
        m_inFrame->Clear();
    }

    for (int i=receivedData.indexOf("IR01"); i < receivedData.indexOf("\r\n", 3) ; i++)
        m_inFrame->AddByte(receivedFrame[i]);

    receivedData.clear();

    emit this->frameReceived(m_inFrame);
    delete m_inFrame;
}

@JonB said in Reciving data from barcode scanner:

Your code assumes you will only get one "line" any time readyRead() is signalled.

..so how do i fix my code?

Christian Ehrlicher

Why not simply use QIODevice::canReadLine() / readLine()? Your approach only reads the first line, ignores if there one more line or a part of it is available since it throws it away.

Damian7546

@Christian-Ehrlicher so it's enough if I only change one line in my code:

const QByteArray scannedData = m_Serial->readAll();

to

const QByteArray scannedData = m_Serial->readLine();

?
Using readLine() I will have \r\n in recived buffor ?

JonB

@Damian7546 said in Reciving data from barcode scanner:

Using readLine() I will have \r\n in recived buffor ?

Try it and see?

Christian Ehrlicher

@Damian7546 said in Reciving data from barcode scanner:

to
const QByteArray scannedData = m_Serial->readLine();

?

No, since there can be more than one line in your buffer. Wrap it in a canReadLine() call.

Damian7546

@Christian-Ehrlicher Like below ?

void ScannerSerialWorker::initSerialPort(QString port)
{
    // Serial Port Initialization
    m_Serial = new QSerialPort(this);
    m_Serial->setPortName(port);
    m_Serial->setBaudRate(QSerialPort::Baud9600);
    m_Serial->setDataBits(QSerialPort::Data8);
    m_Serial->setParity(QSerialPort::NoParity);
    m_Serial->setStopBits(QSerialPort::OneStop);
    m_Serial->setFlowControl(QSerialPort::NoFlowControl);

    if ( m_Serial->open(QIODevice::ReadOnly)) {
        QObject::connect(m_Serial, &QSerialPort::readyRead, this, [&]() {

            while (m_Serial->canReadLine()){
                const QByteArray scannedData = m_Serial->readLine();
                qDebug() << "Scanned data by scanner (RAW): " << scannedData;

                receivedData.append(scannedData);
                if(receivedData.contains("\r\n"))
                    parseRecivedData();
            }
        });
    }
    qDebug() << "SerialPort for Barcode reader status: " << m_Serial->isOpen();
}

Christian Ehrlicher

@Damian7546 said in Reciving data from barcode scanner:

            receivedData.append(scannedData);
            if(receivedData.contains("\r\n"))
                parseRecivedData();

Why do you read it line by line just to append it to a QByteArray and split it again afterwards?