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

Enhancements #971

Merged
merged 19 commits into from
Dec 21, 2020
Merged

Enhancements #971

merged 19 commits into from
Dec 21, 2020

Conversation

DanielGuramulta
Copy link
Contributor

This pull request implements enhancements for the Logic tools as described in the following issues:

Some exceptions would be handled in a thread. Using this macro that creates a QMessageBox would cause undefined behaviour due to the fact that we are creating GUI elements from another thread. If we can identify that this thread does not correspond to the main thread ie the qapplication's thread we invoke the message box. Due to the fact that "msg.exec()" blocks the main thread we also need to block the current thread, to further prevent more undefined behaviour from being introduced, thus the usage of Qt::BlockingQueuedConnection. If we are on the main thread we can simply create the message box like before

Signed-off-by: DanielGuramulta <[email protected]>
This is usefull when printing the plot

Signed-off-by: DanielGuramulta <[email protected]>
@DanielGuramulta DanielGuramulta requested a review from a team December 2, 2020 08:42
Copy link
Contributor

@AlexandraTrifan AlexandraTrifan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@DanielGuramulta
Copy link
Contributor Author

I have pushed some few new changes, @AlexandraTrifan could you review this new changes? Thank you!

@DanielGuramulta DanielGuramulta requested review from adisuciu and a team December 8, 2020 15:26
@AlexandraTrifan
Copy link
Contributor

AlexandraTrifan commented Dec 9, 2020

I have pushed some few new changes, @AlexandraTrifan could you review this new changes? Thank you!

Looks good! I will give it a test run once the appveyor build is finished.

@DanielGuramulta
Copy link
Contributor Author

Should be in final form with all the enhancements. @AlexandraTrifan could you take another look at this. Thank you!

@AlexandraTrifan
Copy link
Contributor

Looks good!

@adisuciu adisuciu merged commit 5fa4964 into master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants