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: add python snippets to GRC #3169

Merged
merged 1 commit into from Feb 10, 2020
Merged

Conversation

mormj
Copy link
Contributor

@mormj mormj commented Feb 4, 2020

This feature adds the ability to insert arbitrary code into the python
flowgraph. It gives a little more low-level flexibility for quickly
modifying flowgraphs and adding custom bits of code rather than having
to go and edit the generated py file

One example is synchronizing multiple USRP objects - sometimes you want
different sync than what is offered in the multi-usrp object, so you can
put a bit of code in the snippet block to do the custom synchronization


This supercedes #2779 and #2814 - hopefully the issues identified on those PRs have been sufficiently resolved, as the details of last iteration were worked out with @dkozel and @marcusmueller at the ESA hackfest.

  • Went back to a single snippet block. This way they can be easily arranged and copied/pasted
  • Priority was added to the block so multiple snippets added to the same section can be sorted by priority
  • Reduce the possible placements in the rendered flowgraph to inside the main function (can add more later if requested)
  • Cleaned up the rendered flowgraph to put the snippet code within their own functions

image

image

Copy link
Member

@marcusmueller marcusmueller left a comment

Choose a reason for hiding this comment

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

This is an adequate, good implementation!

It balances two interests:

  1. ease of use – the snippet block is as easy as users would want it to have, and it's non-singular
  2. sanity of use – the snippets are aggregated in dicts, have order and are inserted into the generated Python on demand.

I think we should let this balloon launch and see how it flies.

grc/blocks/snippet.block.yml Show resolved Hide resolved
Copy link
Member

@devnulling devnulling left a comment

Choose a reason for hiding this comment

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

Looks great! I'm going to test out this branch shortly, will merge if no issues found.

@devnulling
Copy link
Member

@mormj Can you please add py_snippets_demo.grc to the install list for cmake here: https://github.com/gnuradio/gnuradio/blob/master/gr-blocks/examples/CMakeLists.txt

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.

I haven't tested / verified functionality, but this PR reads well & I love the concept of inserting snippets ... how many times have I just edited the resulting Python to do the snippets by hand? I'm with Marcus: let's get this merged & see how it flies!

Copy link
Member

@devnulling devnulling left a comment

Choose a reason for hiding this comment

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

With the cmake fix mentioned in the comment above, this should be good to go.

This feature adds the ability to insert arbitrary code into the python
flowgraph.  It gives a little more low-level flexibility for quickly
modifying flowgraphs and adding custom bits of code rather than having
to go and edit the generated py file

One example is synchronizing multiple USRP objects - sometimes you want
different sync than what is offered in the multi-usrp object, so you can
put a bit of code in the snippet block to do the custom synchronization
@mormj
Copy link
Contributor Author

mormj commented Feb 10, 2020

@devnulling thanks for staying on top of the examples - too easy to forget. Should be good to go now.

@michaelld
Copy link
Contributor

@devnulling please go ahead and merge if this meets your change request

Copy link
Member

@devnulling devnulling left a comment

Choose a reason for hiding this comment

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

Thanks!

@devnulling devnulling merged commit 3232eeb into gnuradio:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants