Reading from COM port yields strange results

mrjj

@mzimmers said in Reading from COM port yields strange results:

This response should come in on a single read,

But it wont always. Its not how it works. Depending on internal cache etc, it will/might be broken up.

But its easy to fix

buffer = port->readAll();
should be
buffer += port->readAll();
and when buffer.size() == expected_size you process the input.

and QByteArray buffer; should move to a class member and not be defined locally.

mzimmers

The message coming back is of variable size, so I can't use the size as a criterion for termination. I can use the string "</XML>" though.

I didn't post the entire read() routine for brevity, but I guess I should:

void CdSerial::read()
{
    QByteArray buffer;
    static QString qBuffer;
    int nBytes = port->bytesAvailable();
    if (nBytes > 0)
    {
        buffer = port->readAll();
    }
    // only show XML formatted input.
    if (strstr(buffer.toStdString().c_str(), XmlStartTag) != NULL)
    {
        // may need to trim off some text ahead of the <XML>.
        buffer.right(buffer.indexOf(XmlStartTag));
        XmlState = STARTED;
    }
    if (strstr(buffer.toStdString().c_str(), XMLStopTag) != NULL)
    {
        // may need to trim off some text behind the </XML>.
        buffer.truncate(buffer.indexOf(XMLStopTag) + XMLStopTag.length());
        XmlState = ENDING;
    }

    if ((XmlState == STARTED) || (XmlState == ENDING))
    {
        qDebug() << nBytes << " bytes read from serial port.";
        qDebug() << buffer.toHex() << endl;

        qBuffer = buffer;
        emit newText(qBuffer);
        if (XmlState == ENDING)
        {
            XmlState = ENDED;
        }
    }
}

I welcome any critique of this method.

Thanks...

mzimmers

@mrjj said in Reading from COM port yields strange results:

@mzimmers said in Reading from COM port yields strange results:

This response should come in on a single read,

But it wont always. Its not how it works. Depending on internal cache etc, it will/might be broken up.

But it seems to be when another program built in MSVC does the same thing. That's why I'm wondering whether I've failed to properly configure the port or something.

mrjj

Hi
When input length is dynamic a data maker is often used.
So if /XML can be used for that its a valid design.
But you still need buffer += port->readAll(); :)

mrjj

@mzimmers
If the MSVC is using native serial API , it might be buffered more.

mzimmers

@mrjj said in Reading from COM port yields strange results:

@mzimmers
If the MSVC is using native serial API , it might be buffered more.

So...are there any settings I can change, or do I need to handle the input stream in the application?

I'm still unclear why I'm getting readyRead signals with 0 bytes available for reading, too.

mrjj

So...are there any settings I can change, or do I need to handle the input stream in the application?

As far as I know, you should handle the case when comes in bulks.
buffer = buffer + port->readAll();

I'm still unclear why I'm getting readyRead signals with 0 bytes available for reading, too.

That also puzzle me as not seen that. I wonder if you use flow control or tx etc?

kuzulis

I'm still unclear why I'm getting readyRead signals with 0 bytes available for reading, too.

It is impossible in principle, because the readyRead() emits only when QSP internally reads bytes more than 0.

PS: You can parse like this (pseudocode):

int startIndex = -1;
int endIndex = -1;

void CdSerial::read()
{
    for (;;) {
        const auto av  = serial->bytesAvailable();
        if (av < 11) // <XML></XML> - minimum size
            return;

        const auto peeked = serial->peek(av); 

        startIndex = peeked.indexOf("<XML>");

        if (startIndex == -1) {
            serial->read(av); // clear all as we did not found start tag
        } else if (startIndex > 0) {
            serial->read(startIndex); // synchronize up to first start tag
            startIndex = -1;
        } else {
            // we have found the start tag, so, now we need to found the end tag
            endIndex = peeked.indexOf("</XML>");  
            if (endIndex == -1) {
                 // we did not found end tag yet
                 return;
            } else {
                 // we found the end tag
                 const auto content = serial->read(endIndex + 6); // +6 == length of </XML> tag
                 // now, content contains whole <XML>blablabla</XML> 
                 // parse it 

                // prepare to next synching
                startIndex = -1;
                endIndex = -1;
            }
        }
    }
}

hskoglund

@mzimmers This might happen if there was an error reading the data, like buffer overflow, try checking what port->error(); says.

kuzulis

This might happen if there was an error reading the data, like buffer overflow

not!

mzimmers

The error code isn't revealing anything (other than a design flaw I've posted in another thread).

Every 2nd time that readyRead() signals, the read returns 0 bytes. According to the docs:

readyRead() is not emitted recursively; if you reenter the event loop or call waitForReadyRead() inside a slot connected to the readyRead() signal, the signal will not be reemitted (although waitForReadyRead() may still return true).

It doesn't seem to me like I need a waitForReadyRead() call...anyone disagree with this?

mrjj

@mzimmers said in Reading from COM port yields strange results:

waitForReadyRead

I agree. its for blocking use and you are using the asynchronous api so no need for that.

kuzulis

@mzimmers said in Reading from COM port yields strange results:

Every 2nd time that readyRead() signals, the read returns 0 bytes

Did you hear me? It is impossible!

mzimmers

I found the cause of the readyRead() firing with 0 bytes available -- it had to do with my QComboBox somehow emitting 2 signals when the value changed. That problem is solved.

It's still a mystery to me why the response to my discovery request comes in pieces, but I think I've got it fixed. This routine also trims off any spurious traffic (the com port is used for much more than responding to discovery requests).

void CdSerial::read()
{
    buffer += port->readAll();
    qDebug() << "buffer contains " << buffer.size() << " characters.";

    if (buffer.contains(XML_START_TAG))
    {
        if (buffer.contains(XML_STOP_TAG))
        {
            // trim off any "fluff" and convert to QString.
            trim(buffer, qBuffer);
            emit newText(qBuffer);
            buffer.clear();
        }
    }
    else // currently not interested in any non-XML traffic.
    {
        buffer.clear();
    }
}

It's a lot of work being done in a slot, but given that this is responding to a comparatively slow device (the serial port), I'm assuming it's OK. I welcome any feedback on my routine above.

Thanks for all the assistance.

kshegunov

I'd consider implementing it with QXmlStreamReader instead of reinventing the wheel. It supports incremental parsing too, which is quite convenient for the thing you seem to be trying to accomplish.

mzimmers

@kshegunov I actually did experiment with QXmlStreamReader. Given the nature of the input stream, though, I found it safer to form the string and then parse it. At this time, I'm not really doing anything with the response other than displaying it, so parsing isn't necessary. I'm sure that will soon change, and I'll probably take another look at it. Thanks for the suggestion.