QSerialPort readAll() does not work with 5.14.0 anymore

kuzulis

@JonB said in QSerialPort readAll() does not work with 5.14.0 anymore:

Hopefully @kuzulis will read this and may have a comment to make!

Yes, maybe it is a bug in QSP >= 5.14.x, but a problem is that it is hard to reprocuce it.. So...

JonB

@rasc
Never tried it, but I would have assumed you can "emulate" waitFor...() by setting up your signals and using QEventLoop::exec() + QEventLoop::exit(), plus a QTimer(), to get the "synchronous" behaviour.

Now, I do not know how that compares to what is actually in the waitFor...()s. And since @kuzulis wrote that, and apparently its behaviour has changed, it may be more complex, and his code may cover more cases than this approach. I don't know if you want to test that out to see how it compares....

@kuzulis Since you have just replied, is it worth OP trying this, or is it more complex/hassle than that?

kuzulis

@rasc,

Just don't rely on the return value from waitForReadyRead () then, just check the bytesAvailable() then after waitForReadyRead() (if you want to use a blocking approach).

PS: I have no time and resources now to fix it.

kuzulis

@JonB said in QSerialPort readAll() does not work with 5.14.0 anymore:

OP trying this, or is it more complex/hassle than that?

An non-blocking approach it is a best/right choose for you! :)

JonB

@kuzulis
I think you intend to address @rasc, not me, when you say "you"! :)

I have said that non-blocking is preferable for the OP. But I can anticipate his reluctance to change existing, blocking code --- for right or for wrong --- as he moves between Qt versions and just wants to know why something which worked does not now.... Hence my suggestion to preserve his code's synchronicity. But if he can change over to signals/slots/asynchronous maybe the issue will go away and he will be better in the long-run, I think he is aware of this :)

kuzulis

@JonB said in QSerialPort readAll() does not work with 5.14.0 anymore:

not me, when you say "you"! :)

Oops, yes, sorry.. It is my copy/paste. :)

rasc

@kuzulis said in QSerialPort readAll() does not work with 5.14.0 anymore:

@rasc,

Just don't rely on the return value from waitForReadyRead () then, just check the bytesAvailable() then after waitForReadyRead() (if you want to use a blocking approach).

I tried to do that and came up with this function as a replacement for waitForReadyRead():

bool com::waitForReady(int timeout) {
	waitForReadyRead(50); // Does not work without this line
	QElapsedTimer elapsed;
	elapsed.start();
	while (bytesAvailable() == 0 && elapsed.elapsed() < timeout) {
		QThread::msleep(3);
	}
	if (elapsed.elapsed() < timeout) {
		return true;
	}
	return false;
}

I just check for bytesAvailable() during the timeout. While this basically means I have to sleep for some miliseconds on every call, it indeed works. But what puzzles me is that bytesAvailable() will only work after I called waitForReadyRead(50) once, otherwise it will be 0 all the time. My understanding of QSP is probably a bit limited, so maybe this is expected (I could imagine it is a threading issue, and calling QThread::msleep() never allows the EventLoop to run while waitForReadyRead() does, I don't know).

PS: I have no time and resources now to fix it.

I completely understand that, thanks a lot for your feedback. Just in case you decide to try to debug it, I am happy to help if I can.

@JonB said

I have said that non-blocking is preferable for the OP. But I can anticipate his reluctance to change existing, blocking code --- for right or for wrong --- as he moves between Qt versions and just wants to know why something which worked does not now.... Hence my suggestion to preserve his code's synchronicity. But if he can change over to signals/slots/asynchronous maybe the issue will go away and he will be better in the long-run, I think he is aware of this :)

This is a pretty accurate summary :)
Anyway, since checking bytesAvailable() somehow works, I guess I am fine for now. And since both of you suggest to move to signals/slots, I will try to do that in the long run.

Should I mark this topic as solved then and accept an answer?

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

bytesAvailable() will only work after I called waitForReadyRead(50) once

Yes, the bytes available just returns a number of bytes in an internal QSP buffer. This buffer incremented after the waitForXXX() methods only, because this methods is a 'tricks' to schedule the I/O. Because in the blocking approach the Qt-event loop is not used.

PS: And yes, you don't need in QThread::msleep(3), because it has not sense (because the sleeping already does by waitForXXX() methods).

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

completely understand that, thanks a lot for your feedback. Just in case you decide to try to debug it, I am happy to help if I can.

You can create a bug report for the QSP, and pass there a link to this topic. And then, when I got a free time, then, maybe I try to fix it. :)

rasc

@kuzulis said in QSerialPort readAll() does not work with 5.14.0 anymore

PS: And yes, you don't need in QThread::msleep(3), because it has not sense (because the sleeping already does by waitForXXX() methods).

Ah, ok. I guess

bool com::waitForReady(int timeout) {
	waitForReadyRead(1);
	QElapsedTimer elapsed;
	elapsed.start();
	while (bytesAvailable() == 0 && elapsed.elapsed() < timeout) {
		waitForReadyRead(1);
	}
	if (elapsed.elapsed() < timeout) {
		return true;
	}
	return false;
}

is the "correct" solution then.

Update: Just for the sake of completeness, here is the patch for my program: https://github.com/BrillouinMicroscopy/BrillouinAcquisition/pull/135