Refactor Picoscope implementation

[vimpostor]

As discussed, this implements all of the following items:

• Restrict Picoscope value types to int16_t, float and gr::UncertainValue<float>
• Remove the custom work() implementation and port Picoscope to use processBulk()
• BlockingIO is now chosen at compile-time, based on whether the acquisition mode is streaming or rapidBlock
• Generalize and extend the Picoscope Block so that it can be used for all Picoscope variants.
• Incorporate timing messages received over the message port, when a trigger arrives
• Update to latest GR version

[vimpostor]

Note that the CI will probably fail, as the following patch is needed for graph-prototype:

diff --git a/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp b/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp
index 89da05f..4304fb6 100644
--- a/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp
+++ b/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp
@@ -370,9 +370,9 @@ struct TagSink : public Block<TagSink<T, UseProcessVariant>> {

 ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, gr::testing::ProcessFunction b), (gr::testing::TagSource<T, b>), out, n_samples_max, sample_rate, signal_name, signal_unit, signal_min, signal_max, verbose_console, mark_tag, values);
 ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, gr::testing::ProcessFunction b), (gr::testing::TagMonitor<T, b>), in, out, n_samples_expected, sample_rate, signal_name, n_samples_produced, log_tags,
-                                    log_samples, verbose_console, samples);
+                                    log_samples, verbose_console);
 ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, gr::testing::ProcessFunction b), (gr::testing::TagSink<T, b>), in, n_samples_expected, sample_rate, signal_name, n_samples_produced, log_tags,
-                                    log_samples, verbose_console, samples);
+                                    log_samples, verbose_console);

 auto registerTagSource = gr::registerBlock<gr::testing::TagSource, gr::testing::ProcessFunction::USE_PROCESS_ONE, float, double>(gr::globalBlockRegistry())
                        | gr::registerBlock<gr::testing::TagSource, gr::testing::ProcessFunction::USE_PROCESS_BULK, float, double>(gr::globalBlockRegistry());

Alternatively one could copy over the TagSink implementation, but that would be unnecessary code duplication, as it is questionable if the samples member needs to be changeable over the Settings interface (it can still be accessed as a member of the class by tests of course).

[vimpostor]

I have updated to latest graph-prototype, but it looks like there is still an unnecessary copy-constructor that causes the build to fail. The following patch should work:

diff --git a/core/include/gnuradio-4.0/BlockModel.hpp b/core/include/gnuradio-4.0/BlockModel.hpp
index 44f209d..d0a2aff 100644
--- a/core/include/gnuradio-4.0/BlockModel.hpp
+++ b/core/include/gnuradio-4.0/BlockModel.hpp
@@ -353,7 +353,7 @@ public:

     ~BlockWrapper() override = default;

-    explicit BlockWrapper(property_map initParameter = {}) : _block(std::move(initParameter)) {
+    explicit BlockWrapper(property_map initParameter = {}) : _block{{std::move(initParameter)}} {
         initMessagePorts();
         _dynamicPortsLoader = std::bind(&BlockWrapper::dynamicPortLoader, this);
     }

[RalphSteinhagen]

The syntax is a bit weird:

_block{{std::move(initParameter)}}

because this implies that you create a map in a map... :thinking:

[vimpostor]

because this implies that you create a map in a map... 🤔

Not quite, I agree the syntax is a bit confusing, but this is to force list-initialization instead of copy-initialization according to [dcl.init.aggr] section 8.5.1 clause 2:

Each member is copy-initialized from the corresponding initializer-clause.
(...)
If an initializer-clause is itself an initializer list, the member is list-initialized.

So the extra {} is there to make the initializer clause itself an initializer-list, see also https://stackoverflow.com/a/60609943.

I was also a bit confused by this behaviour, but you can test it empirically: Without the patch applied the build fails, because the constructor is ill-formed. With the patch applied, everything works.

I hope that explains it. :)

[vimpostor]

@RalphSteinhagen I have pushed an alternative commit f61526696d8c30e0689206f0a4b7310d735223c7, that works around this fishy behaviour by redefining the property_map constructor. Let me know if you prefer that over the patch described above.

[sonarcloud[bot]]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud