Problem with running timer in another thread

JoeCFD

Damian7546

@hskoglund said in Problem with running timer in another thread:

Ah, I see in your cctalk_device.h file, startTimer() is declared under protected:
try moving it under slots: (like your function void timerIteration();)

Problem is more complicated then can I thought, probably not related with startTimer() function .
I totally remove calling startTimer() from my project and still have the same error :

QML debugging is enabled. Only use this in a safe environment.
ThreadID SerialWorker:   0x79e36d8867c0
ThreadID CctalkLinkController:   0x79e36d8867c0
ThreadID CctalkDevice:   0x79e36d8867c0
Using  "/home/developer/.config/CCtalkProj.ini"  as a user-specific settings file
ThreadID MyCcTalkApp:   0x79e36d8867c0
"* Coin acceptor configured device: /dev/ttyUSB0"
"* Opening port \"/dev/ttyUSB0\"."
"Requested device state change from ShutDown to: Initialized"
test test test
"* Port \"/dev/ttyUSB0\" opened."
"> ccTalk request: SimplePoll, address: 2, data: (empty)"
"> Request: 020001feff"
"< Full response: 020001feff01000200fd"
"< ccTalk response from address 2, data: (empty)"
"* Device is alive (answered to simple poll)"
QObject::startTimer: Timers cannot be started from another thread

Below I publish code wihich I try run:
[https://github.com/ienecode/cctalk.git](link url)

Damian7546

@JoeCFD @JoeCFD code analysis should be do from the creating object - MyCcTalkApp, where are created
Coin acceptor communicator - qtcc::Coin AcceptorDevice coin acceptor _;
Analyzing further do you see any problems ?

J.Hilk

Still the same problem as originally.

your 'serial_port_' is not moved with move ToThread QSerialPort uses internally QTimer thats why you see the message.

Damian7546

@J-Hilk So, how can I fix this code ? Can you give me tips ?

Christian Ehrlicher

Pass the correct parent to your timer.

J.Hilk

@Damian7546

Fix your constructor of your QObject based worker: More of a principal matter in this case

class SerialWorker : public QObject {
	Q_OBJECT
	public:

		/// Constructor
		SerialWorker();
...->...
class SerialWorker : public QObject {
	Q_OBJECT
	public:

		/// Constructor
		explicit SerialWorker(QObbject *parent = nullptr);
.....
SerialWorker::SerialWorker()
{
...->...
SerialWorker::SerialWorker(QObject*parent) : QObject(parent)
{

Drop the scoped pointer, you're trying to be fancy when there's no need for it and it's causing trouble

QScopedPointer<QSerialPort> serial_port_; 
...->
QSerialPort * const serial_port_; //Raw dogging the serial port, hell ya

Init the QSerialport correctly:

SerialWorker::SerialWorker(QObject*parent) : QObject(parent), serial_port_{new QSerialPort(this)}
{

Pl45m4

@J-Hilk said in Problem with running timer in another thread:

Fix your constructor of your QObject based worker: More of a principal matter in this case

Drop the scoped pointer, you're trying to be fancy when there's no need for it and it's causing trouble

Init the QSerialport correctly:

TBH these things are also "weird" / designed badly in the original repository, @Damian7546 just copied/forked this structure.
I'm wondering if any of the original stuff actually works... from looking at it I would say it also has many issues.
Usually just including the class inQObject's parent-child structure and therefore moving the thread affinity with the parent would fix this. That's what I wrote/thought earlier, but it seems that this is not so easy to do due to the way the whole thing is designed.

QSerialPort * const serial_port_; //Raw dogging the serial port, hell ya

LMAO :D

Damian7546

@J-Hilk After changes proposed by you , the logs are this:

"* Coin acceptor configured device: /dev/ttyUSB0"
"* Opening port \"/dev/ttyUSB0\"."
"Requested device state change from ShutDown to: Initialized"
"* Port \"/dev/ttyUSB0\" opened."
"> ccTalk request: SimplePoll, address: 2, data: (empty)"
"Starting poll timer."
"> Request: 020001feff"
"Polling......"
"< Full response: 020001feff01000200fd"
"< ccTalk response from address 2, data: (empty)"
"* Device is alive (answered to simple poll)"
QObject::startTimer: Timers cannot be started from another thread
"Polling......"
"Polling......"
"Polling......"
"- The last message was repeated 40 times total"
"- The last message was repeated 80 times total"
"- The last message was repeated 120 times total"
"- The last message was repeated 160 times total"

JonB

@Damian7546
At this point I doubt that anyone can reliably know what exactly your code reads. You clearly still have a thread issue.

I am unsure: have you identified where the startTimer() call is issued from in code?

Pl45m4

@Damian7546

Since you probably used the code from the original GitHub repo without modifying it, you should reach out to the owner of the repo via GitHub issues.
Maybe he can identify the problem as a whole faster than a bunch of people in the forum here, who have to dive into the project first and dig through the logic to understand how everything is put together.
As @J-Hilk mentioned above, there are a couple of things that could have been done cleaner...
(creating QObject derived worker without actually calling the QObject base implementation... etc...)

Damian7546

@JonB
My opinion it call only once, in the CctalkDevice::requestSwitchDeviceState (cctalk_device.cpp) , when the device goes to Initialized.
241 linie: https://github.com/ienecode/cctalk/blob/main/cctalk/cctalk_device.cpp

@Pl45m4
I contacted the author, and he tells me that this code was developed for a project that was stopped before there was a chance to properly test the code,
so this is the state it was left in.

I interesed in this code, because I think that the structure of this code for SERIAL communication seems very reasonable and developmental to me.

So I would be really grateful if anyone of forumers could be help me run this code , especially to run cyclic polling , in way is realized in this code.

Christian Ehrlicher

I don't see a reason to use threads for simple serial communication with QSerialPort at all. It just complicates things, esp. for people without proper knowledge about threads. Don't use it - simply use QSerialPort and proper signals & slots.

Damian7546

@Christian-Ehrlicher I expected such answers: "simply use QSerialPort and proper signals & slots." . I have project realized in this way.
But as part of learning (main goal of the project) programming and uing threads I would like to use threads this time to serial communication, and run example considered in this topic.

JonB

@Damian7546
Then by choosing to use threads where they are probably not needed you (or the original author) have ended up with a problem using threads, possibly because the original code was not well written for threads. Which tells you a lot... ! Just saying.

Pl45m4

@Damian7546 said in Problem with running timer in another thread:

But as part of learning (main goal of the project) programming and uing threads I would like to use threads this time to serial communication, and run example considered in this topic.

Then you can look, for example, at Qt's Blocking Serial Port Examples:

(Sender)

https://doc.qt.io/qt-6/qtserialport-blockingsender-example.html

(Receiver)

https://doc.qt.io/qt-6/qtserialport-blockingreceiver-example.html

Or you use some non-Qt implementation of serial port communication, such as serial_port from Boost.ASIO library.
(makes little sense, since you are already using Qt which provides all that stuff... but for learning purposes it's fine to look at different techniques)

https://www.boost.org/doc/libs/1_84_0/doc/html/boost_asio/overview/serial_ports.html

Christian Ehrlicher

@Damian7546 said in Problem with running timer in another thread:

But as part of learning (main goal of the project) programming and uing threads I would like to use threads this time to serial

Then look at the Qt documentation for Qt threads and it's examples instead some obscure git projects.

Damian7546

Problem with QObject::startTimer: Timers cannot be started from another thread Solved!

I changed:

-All entries of the connect function, from:

connect(this, &CctalkLinkController::openPortInWorker, serial_worker_.data(), &SerialWorker::openPort, Qt::QueuedConnection);

to:

connect(this, &CctalkLinkController::openPortInWorker, serial_worker_, &SerialWorker::openPort, Qt::QueuedConnection);

Because I don't have any idea what is data() ?

And like suggested @J.Hilk in CctalkLinkController I changed:

CctalkLinkController::CctalkLinkController()
{
    serial_worker_ = new SerialWorker();

to

CctalkLinkController::CctalkLinkController(QObject *parent) : QObject(parent), serial_worker_{new SerialWorker(this)}
{