Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block TrikScriptRunnerWorker thread when reading values from sensors #1805

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,18 @@ void TrikLineSensorAdapter::detect()

QVector<int> TrikLineSensorAdapter::read()
{
QMetaObject::invokeMethod(mLineSensor, "read");
/* Sensor values are calculated by timer in the UI Thread
* (subscription in plugins/robots/interpreters/trikKitInterpreterCommon/src/TrikBrick)
* and are also forced through TrikScriptRunner when calculating sensor values (except for Gyroscope::read).
* This heavily loads the UI thread, and in incorrect code like
* while (true) { brick.lineSensor("video1").read(); }
* the UI thread gets blocked by sensor reading tasks, making the user interface non-functional.
* As a hot-fix, it is proposed to calculate values using Qt::BlockingQueuedConnection,
* which will force waiting for the sensor value calculation instead of queuing new tasks
* in the UI thread every few milliseconds (this is critical for reading from sensors
* whose handlers take a relatively long time, as the queue contains many reading tasks
* while returning old (already calculated) sensor values, which forces new tasks to be queued). */
QMetaObject::invokeMethod(mLineSensor, "read", Qt::BlockingQueuedConnection);
return mLineSensor->lastData(); // hopefully the same format
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ TrikSensorEmu::TrikSensorEmu(kitBase::robotModel::robotParts::ScalarSensor *sens
int TrikSensorEmu::read()
{
//mSensor->read(); test crash fixes
QMetaObject::invokeMethod(mSensor, "read");

/* Sensor values are calculated by timer in the UI Thread
* (subscription in plugins/robots/interpreters/trikKitInterpreterCommon/src/TrikBrick)
* and are also forced through TrikScriptRunner when calculating sensor values (except for Gyroscope::read).
* This heavily loads the UI thread, and in incorrect code like
* while (true) { brick.sensor("A1").read() }
* the UI thread gets blocked by sensor reading tasks, making the user interface non-functional.
* As a hot-fix, it is proposed to calculate values using Qt::BlockingQueuedConnection,
* which will force waiting for the sensor value calculation instead of queuing new tasks
* in the UI thread every few milliseconds (this is critical for reading from sensors
* whose handlers take a relatively long time, as the queue contains many reading tasks
* while returning old (already calculated) sensor values, which forces new tasks to be queued). */
QMetaObject::invokeMethod(mSensor, "read", Qt::BlockingQueuedConnection);
return mSensor->lastData(); // not best, race conditions?
}
Loading