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

grc: Fix crash when using function probe with BokehGUI #3301

Merged
merged 1 commit into from May 29, 2020

Conversation

Notou
Copy link
Contributor

@Notou Notou commented Apr 2, 2020

The function probe block creates a new worker thread to do it's thing, this means that the callback to update the value of the block is executed in that separate thread.
The bokeh framework doesn't allow that, so using a probe block value in a BokehGUI block causes a crash.
This PR solves it by giving the callback function to the bokeh framework so it can execute it in its own thread.
I did not know how to check whether QT or bokeh is active from the yaml file, so I made a nested try.

This can be backported to maint-3.8.

@nickoe nickoe added GRC master Specific to master labels Apr 3, 2020
@mormj mormj requested a review from 777arc April 3, 2020 13:00
Copy link
Contributor

@michaelld michaelld left a comment

Choose a reason for hiding this comment

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

small tweak, but otherwise LGTM

grc/blocks/variable_function_probe.block.yml Outdated Show resolved Hide resolved
@michaelld michaelld requested review from mormj and haakov April 19, 2020 20:47
@michaelld
Copy link
Contributor

I tagged a few other GR devs to see what they think ...

Copy link
Contributor

@michaelld michaelld left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelld
Copy link
Contributor

@mormj you around to check out this PR? Should be quick ...

@mormj
Copy link
Contributor

mormj commented Apr 26, 2020

I've actually never tried out the bokeh framework, so this one I'll have to give a try later

@michaelld
Copy link
Contributor

@mormj ping ... any progress? I've ever tried the boken framework either ... but the code looks good & if the author says it does the trick I'm willing to go with that.

@mormj
Copy link
Contributor

mormj commented May 5, 2020

Trying to get bokeh set up, but ran into this (among others):
gnuradio/gr-bokehgui#19

Also, gr-bokeh does not currently compile with master branch gr. Not sure the depth of changes needed

@Notou
Copy link
Contributor Author

Notou commented May 6, 2020

I have not yet pulled the 3.8 port to the gnuradio gr-bokehgui repo.
I the mean time, you can use this fork: https://github.com/Inria-Maracas/gr-bokehgui

@Notou
Copy link
Contributor Author

Notou commented May 6, 2020

And now it's done. You should be able to build with maint-3.8. Master is not yet supported since the change in shared pointers.

@Notou
Copy link
Contributor Author

Notou commented May 29, 2020

@mormj ... Any news on this part? It should even work on master (at least related to the shared pointer changes).

@mormj
Copy link
Contributor

mormj commented May 29, 2020

I still haven't gotten gr-bokeh to work yet, but I pulled in this change and verified it didn't break the function probe, so I go ahead and merge.

@mormj mormj merged commit 5a29835 into gnuradio:master May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GRC master Specific to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants