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

feature/ZeroMQ #9

Merged
merged 6 commits into from Sep 2, 2021
Merged

feature/ZeroMQ #9

merged 6 commits into from Sep 2, 2021

Conversation

loic-fejoz
Copy link
Contributor

Add ZMQSubSource and SMQPubSink for streaming over ZeroMQ protocol which is also by default in GNU Radio.
I think I have made it as a feature activated by default.
It also come with two binaries (one receiver, one emitter) that can talk to each other and/or with GNU Radio.

Apparently the topics is not settable in GNU Radio thus I have not provided way to set it. Maybe I should?

Close #3

Working example of an emitter and a receiver over ZeroMQ.
Tested also against a GNU Radio ZMQ PUB/SUB.
Also declaring blocks as blocking.
Use log crate.
Clean comments.
@bastibl
Copy link
Member

bastibl commented Sep 2, 2021

Awesome! With example and even feature-gated. Really cool :-)

I'll merge this as is and do some minor follow-up patches (mainly because I'll be afk the next days and want to avoid stalling this through a lot of back-and-forth).

Regarding error handling: in the functions that return a Result<X>, you can use the ? operator to deal with errors. In case of an Option<X>, you could use .context("option was none")? instead of expect() or assert!(....is_ok()).

There are also some related questions, I thought about. Maybe you have some thoughts here as well:

  • I'm not sure if such blocks should be typed, i.e., ZmqSubSource(4, ...) or ZmqSubSource<u32>(...). I think at some point, we might have to switch to typed blocks, since the alignment of data in the buffer cannot only be determined by the size of the data (see align_of). I'll also have adapt Copy and NullSource etc.
  • I also prefer ZMQSubSource but it seems to be idiomatic Rust to write ZmqSubSource. It's also why I called PMTs Pmts. But in this case, we could anyway get around this by putting the blocks in a submodule/subfolder and have zmq::SubSource. This also eases feature-gating and (IIRC) there are other transfer modes, so we might add more blocks in the future.
  • I'm not sure what to do with default features. Since compile times tend to get large, I thought about keeping them minimal and also remove soapy and vulkan. The binaries and examples can be annotated with required-features so one will get a nice message, telling the user to enable the feature. Unfortunately, there doesn't seem to be a user config to overwrite default features, for example, if you work on the ZMQ stuff. So the only option would be to edit Cargo.toml temporary and add the feature.

Thanks again!

@bastibl bastibl merged commit 0d73d9a into FutureSDR:master Sep 2, 2021
@loic-fejoz loic-fejoz deleted the feature/ZeroMQ branch September 2, 2021 19:08
@loic-fejoz
Copy link
Contributor Author

Concerning having typed blocks, I have mix feeling. Somehow I think the runtime could do the transformation and thus we can have "agnostic" blocks. But on the other side, It would probably increase overhead and thus may not be a good idea.
So maybe we should have transformation blocks moreover with typed blocks indeed.

NB: Thanks for the tip about context (I am a rust newbie).

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.

ZeroMQ PubSink & SubSource
2 participants