refactored port<T> interface

[RalphSteinhagen]

... as outlined by GR Architecture WG and #148

tackled items:

• refactored port structure (mandatory enum NTTPs vs. optional type-wrapped arguments)

  class template<typename T, fixed_string PortName, port_type_t PortType, port_direction_t PortDirection, 
                             typename...  Arguments>
  struct Port { /* ... */ };

• added optional domain argument
• added default init value (needed for cyclic graphs)
• add isOptional() annotation
• fixed repeated_port name -> name0, name1, name2, ...
• fixed 0 -> 1 for the required min port samples in various tests & code examples (now ensured via a static_assert(..))
• added 'Async' port annotation and 'isSynchronous()' function
• renamed IN,OUT,... short-hand aliases to more explicit/hopefully descriptive PortIn, PortOut names

  template<typename T, typename... Arguments>
  using PortIn = Port<T, "", port_type_t::STREAM, port_direction_t::INPUT, Arguments...>;
  template<typename T, typename... Arguments>
  using PortOut = Port<T, "", port_type_t::STREAM, port_direction_t::OUTPUT, Arguments...>;
  template<typename... Arguments>
  using MsgPortIn = Port<property_map, "", port_type_t::MESSAGE, port_direction_t::INPUT, Arguments...>;
  template<typename... Arguments>
  using MsgPortOut = Port<property_map, "", port_type_t::MESSAGE, port_direction_t::OUTPUT, Arguments...>;
  // ...

• updated the developer Readme.md and code documentation where necessary (-> there's more work for technical writers):
  Some of the possible optional port annotation attributes are:
  • RequiredSamples<size_t, size_t> to describe the min/max number of samples required from this port before invoking the blocks work function,
  • Optional informing the graph/scheduler that a given port does not require to be connected,
  • PortDomain<fixed_string> described whether the port can be handled within the same scheduling domain (e.g. CPU or GPU),
  • StreamBufferType and TagBufferType to inject specific user-provided buffer implementations to the port, or
  • Async for making a port asynchronous in a signal flow-graph block.
• changed to capitalised class naming following C++ Core guideline item and Bjarne Stroustrup's style naming convention where ISO-C++/STL-style classes (or in our case classes that could be) are lower-case but own declared types being declared capitalised. N.B. this helps to distinguish whether classes are standard ISO-C++ (and should perhaps be known by the user) and/or are defined in the GNU Radio context only.

Thanks for the in-person and on-line pre-comments and feedbacks by @ivan-cukic, @drslebedev, and @wirew0rm.

One of the follow-ups would be to implement the Async function in the node/block code.

[pr-explainer-bot]

Pull Request Review

Hey there! 👋 I've summarized the previous results for you. Here's a markdown document for your pull request review:

Changes

1. In bm_case1.cpp:

   • Changed the inheritance of math_op struct from fg::node<math_op<T, op>, fg::IN<T, 0, N_MAX, "in">, fg::OUT<T, 0, N_MAX, "out">> to fg::node<math_op<T, op>, fg::PortInNamed<T, "in">, fg::PortOutNamed<T, "out">>.
   • Changed the inheritance of math_bulk_op class from fg::node<math_bulk_op<T, op>, fg::IN<T, 0, N_MAX, "in">, fg::OUT<T, 0, N_MAX, "out">> to fg::node<math_bulk_op<T, op>, fg::PortInNamed<T, "in", fg::RequiredSamples<1, N_MAX>>, fg::PortOutNamed<T, "out", fg::RequiredSamples<1, N_MAX>>>.
   • Changed the inheritance of converting_multiply class from fg::node<converting_multiply<T, R>> to fg::node<converting_multiply<T, R>, fg::PortIn<T>, fg::PortOut<R>>.
   • Changed the inheritance of add class from fg::node<add<T, addend>> to fg::node<add<T, addend>, fg::PortIn<T>, fg::PortOut<T>>.
   • Changed the inheritance of gen_operation_SIMD class from fg::node<gen_operation_SIMD<T, op>, fg::IN<T, 0, N_MAX, "in">, fg::OUT<T, 0, N_MAX, "out">> to fg::node<gen_operation_SIMD<T, op>, fg::PortInNamed<T, "in">, fg::PortOutNamed<T, "out">>.

2. In bm_test_helper.hpp:

   • The source class has the following major changes:
     • The out member variable has been changed from fg::OUT<T> to fg::PortOut<T>.
     • The source class now inherits from fg::node<source<T, min, count>>.

Suggestions

In README.md:

1. In the Port class template, the Arguments parameter should be variadic (typename... Arguments) to allow for multiple arguments.
2. The Port class template should have a default value for the BufferType parameter, such as gr::circular_buffer<T>.
3. The Port class template should have a default value for the PortType parameter, such as port_type_t::Generic.
4. The Port class template should have a default value for the PortDirection parameter, such as port_direction_t::Input.
5. The Port class template should have a default value for the PortName parameter, such as an empty string.
6. The Port class template should have a default value for the MIN_SAMPLES parameter, such as std::dynamic_extent.
7. The Port class template should have a default value for the MAX_SAMPLES parameter, such as std::dynamic_extent.

Bugs

1. Potential bugs in the code:
   • In bm_test_helper.hpp, the sink class has a member variable named in of type fg::PortIn<T, fg::RequiredSamples<N_MIN, N_MAX>>. However, the RequiredSamples template argument is not provided, which may lead to compilation errors or unexpected behavior.
   • In README.md, the Port class template is defined as class template, which is incorrect syntax. It should be template<typename T, fixed_string PortName, port_type_t PortType, port_direction_t PortDirection, typename... Arguments> struct Port { /* ... */ };.

Improvements

A place in the code that could be refactored for better readability is in README.md. The function signatures of get_trigger_poller, get_multiplexed_poller, get_snapshot_poller, register_streaming_callback, register_trigger_callback, register_multiplexed_callback, and register_snapshot_callback can be improved by adding spaces around the && operator.

Rating

The code has been rated on a scale of 0 to 10 based on the following criteria:

• Readability: 8
• Performance: 9
• Security: 7

That's it! Feel free to make any necessary changes and let me know if you need any further assistance. Good luck! 🚀

[wirew0rm]

Looks good and makes the port definitions nicer. Thanks also for taking care of updating the documentation 👍

[wirew0rm]

Should these math helper functions fall back to a non recursive implementation if they are evaluated at runtime? Or be consteval or an assertion to prevent accidental runtime use?

[wirew0rm]

I assume Port is capitalized because you refer to the actual type and not the general concept? I it is supposed to be code I would expect it to be surrounded by single ticks (Port). Not super important but if we decide on one style there we can make the effort to keep it consistent everywhere.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

47.7% Coverage
1.4% Duplication

The version of Java (11.0.17) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Catch issues before they fail your Quality Gate with our IDE extension SonarLint