PublishablePortSpan #415
PublishablePortSpan #415
Conversation
| @@ -345,6 +345,12 @@ struct DummyPublishableSpan { | |||
| }; | |||
| static_assert(PublishableSpan<DummyPublishableSpan<int>>); | |||
|
|
|||
| template<typename T> | |||
| struct DummyPublishablePortSpan: public DummyPublishableSpan<T> { | |||
There was a problem hiding this comment.
@drslebedev @wirew0rm I am wondering whether we should merge both concepts to create basically only one PublishableSpan. 🤔
There was a problem hiding this comment.
Since we don't need the Dummy Type anywhere else than in the Block, we don't really need the Base Type DummyPublishableSpan<T> and it could be removed.
There was a problem hiding this comment.
Thanks for the update.
Some minor comments/questions:
- do we need both concept definitions (see inline comments)?
- the use of the
ProducableSpan::publishTag(...)should be changed in the few uses in the block lib (e.g. in: BasicFileIo.hpp, clock_source.hpp, TagMonitors.hpp). - Rename the parameter
indextotagOffsetand remove the '0' since this would be the default parameter value.
This should make the changes/naming more expressive. Once these minor changes are in, the PR could be merged as-is without further review.
P.S. please do not forget to 'check' this tasks in the corresponding issue. Thanks in advance.
| @@ -268,6 +268,11 @@ concept ConsumablePortSpan = ConsumableSpan<T> && requires(T span) { | |||
| { *span.tags.begin() } -> std::same_as<const gr::Tag&>; | |||
| }; | |||
|
|
|||
| template<typename T> | |||
| concept PublishablePortSpan = PublishableSpan<T> && requires(T span, property_map& tagData, Tag::signed_index_type index) { | |||
There was a problem hiding this comment.
Merge with PublishableSpan? Are there use-cases where we need to keep them separate?
There was a problem hiding this comment.
This one is specific to Port, while the other applies to every buffer. If we would merge them you could not use the Span API on a "bare" ringbuffer.
Note: For the Consumable Span it is not possible, since the ConsumablePortSpan<CircularBufferMemberType> contains a ConsumableSpan<Tag>, by merging them this would become recursive. So there would be some "symmetry" argument to keep them seperated also in the PublishableSpan case.
There was a problem hiding this comment.
@wirew0rm the question is about
a) naming/proliferation of concepts (similar dependent definitions)
b) we need this named concept since users -- with this PR -- do not usually interact directly with the buffer-related concept.
core/include/gnuradio-4.0/Port.hpp
Outdated
|
|
||
| ~PublishablePortOutputRange() = default; | ||
|
|
||
| void publishTag(property_map& tagData, Tag::signed_index_type index) { _port->publishTag(tagData, index); } |
There was a problem hiding this comment.
Maybe rename index to tagOffset to better express the intention that this is the index/offset w.r.t. the initial sample that is presented to the user.
drslebedev
force-pushed
the
out_port_range
branch
from
2fc2796
to
272cadd
Compare
…rt tags. Additionally, the following issues were addressed: * Renamed Block::input_tags_present() to inputTagPresent(). * Fixed a bug in the parameter name for the Soapy block. * Rename some CircularBuffer/Reader/Writer method to be more consistent * Add convenience methods to iterate through tuple: for_each_publishable_span, for_each_consumable_span * Remove obsolete Block::availableInputSamples and Block::availableOutputSamples methods Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
drslebedev
force-pushed
the
out_port_range
branch
from
272cadd
to
ebf8401
Compare
Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
drslebedev
force-pushed
the
out_port_range
branch
from
bf74c5a
to
cd13e2c
Compare
| @@ -345,6 +345,12 @@ struct DummyPublishableSpan { | |||
| }; | |||
| static_assert(PublishableSpan<DummyPublishableSpan<int>>); | |||
|
|
|||
| template<typename T> | |||
| struct DummyPublishablePortSpan: public DummyPublishableSpan<T> { | |||
There was a problem hiding this comment.
Since we don't need the Dummy Type anywhere else than in the Block, we don't really need the Base Type DummyPublishableSpan<T> and it could be removed.
More details in corresponding issue #407
This PR introduces
PublishablePortSpan, which extendsPublishableSpanby adding a convenientpublishTagmethod.Additionally, the following issues were addressed in this PR:
Block::input_tags_present()toinputTagPresent().Soapyblock.for_each_publishable_span,for_each_consumable_spanBlock::availableInputSamplesandBlock::availableOutputSamplesmethods