ostrosco / comms-rs

A library for building DSP communication pipelines.
MIT License
13 stars 1 forks source link

Node Re-architecture. #18

Closed ostrosco closed 5 years ago

ostrosco commented 5 years ago
Alex-Addy commented 5 years ago

Changes look good. My only final question would be for an example node that uses those fields.

ostrosco commented 5 years ago

@rfdoell I'm not sure where your comment went (I hope I didn't accidentally delete it), but here's the answer to your question.

In node/mod.rs, test_throughput, the Node3 instantiation uses the fields. I'll add an example to the docs along the lines of what @Alex-Addy was asking for with a node that uses all of the options in the macro.

rfdoell commented 5 years ago

Generally looks good to me. Regarding your previous comment, I found the docs after I commented, so I deleted it (forgetting that GH does push notifications on all activity :P) - I'll try and be a little more thorough in the future.

If I were to add anything, it'd me more tests and examples, but the ones that you have should be enough to get started and start making stuff happen. For @garbagetrash , I had the same confusion until I saw the doc on line 53:

/// - func: The function this node executes upon executing `call()`. The
///   function must accept a mutable reference to the node being constructed as
///   its first parameter, but if the function doesn't need to access state the
///   parameter can be safely be ignored.

After playing with it a little, it makes sense to me - this is what I got to: https://play.rust-lang.org/?gist=081728c67f1122c9da72626d66e25576&version=stable&mode=debug&edition=2015 and I think this is how we'll access member fields for nodes which maintain state.