recently there has been some discussion regarding the peak_detector2
both in the github/gnuradio (pull request 404) as well as in the issue
tracker (issue 783).
It is now well accepted that this block is buggy: there are cases the
function returns -1, which is a bug (see issue 783 on how to recreate
I believe however that there is a DEEPER misconception about how this
works/should work that has resulted in some frustration on what an
appropriate fix should be.
In particular there is an insistence that an appropriate bug fix should
pass the qa_test of this block and it should be [in the spirit] of the
In the following I will explain why passing the qa_test is a
of the buggy behaviour of this block and NOT its feature.
In addition I will suggest what a proper behaviour of this block should
so that others who may want to write their own version of a peak
find it useful.
So the peak_detector block is very reasonable in its conception and its
name is very informative and appropriate. It works as follows:
Reads the input and keeps track of a running average (through a
single-pole iir filter)
When the current input crosses a threshold (= average * a user-defined
factor) upwards the block enters a search state, where it looks for the
maximum value of the input over a window of user-defined length.
This is clearly the intended behaviour of the block according to the
documentation (I don’t know who the original author is…).
It should be obvious that this block is intended for a scenario where
inputs are coming continuously and the block is determining the peaks.
A prototypical application is finding the peak after a MF with a
sequence in order to determine the start of a packet
(BTW, there is some effort on creating a correlate and sync block, which
very relevant to this discussion).
It should also be obvious that both the pole of the IIR filter, the
threshold, AND the window should be properly selected per application
(eg, for a training sequence of length N, it is appropriate to set the
window ~N) otherwise the block will simply not work according to its
So here are some questions and my suggested answers:
Q1: How should this block behave when it has already crossed the
upwards and is still inside the user-defined search window but there is
enough input data to process?
A1: Clearly the work function should return (consuming all
up to the most current peak–but not more) and hopefully the next time
is called, the scheduler would provide more samples so that the search
the remaining window is exhausted and the peak is reported.
Q2: And then, what if the scheduler does not give this block more input
data? Maybe because (as in the qa_test) this block is connected to a
that outputs a finite (=21 !) number of samples?
A2: Since the intended use of this block is a continuous stream of data,
the above situation is an inappropriate use of this block, and thus it
cannot be considered a test case. This means that an appropriate test
should make sure that enough tail data is in the finite input so that
search window is exhausted.
Q3: What if the user insists in using the block in a way that is not
intended, ie, by providing it with a finite (and small compared to the
search window) number of inputs. Should there be a way that the block
A3: The current implementation has this behaviour but it is a BUG not a
feature: ERRONEOUSLY it decrements the window even if it does not return
any data, so eventually the window becomes smaller than the available
and the block exits!
This results in the block passing an ill-conceived qa_test (where the
window is set to 1000 for a peak occurring within 10 samples of the
threshold crossing). However the intended use of the block was never
(according to the documentation).
From the above it should be clear that any fix of this block should NOT
be in the spirit of the original one, nor not passing the existing
is a measure of its difference from the original block (at least from
stated intended behaviour of the original block).
I attach here a clean modification of this block that has the intended
behaviour. It DOES NOT pass (correctly) the inappropriate qa_test,
but it does pass more appropriately designed test(s) (also attached).