Passing segment instances fails because this creates a race condition.
When a long conversion is taking place, the SignalBase::samples_added
signal is called often but since it's in a separate thread, the calls
are queued and aren't executed immediately. Now if the conversion is
restarted - for example as a result of a changed conversion threshold -
then the segments holding the converted data are destroyed, rendering
the pointers submitted as parameters to samples_added invalid.
Once the signal queue is processed, those invalid pointers will be
accessed and PV segfaults.
Since the signal queue can neither be emptied nor flushed, this
leaves only two sensible choices:
1) Signal samples_added less often, thereby reducing the chance of
signals being queued
2) Supply the segment ID instead of the segment instance as that's
essentially the only thing we currently care about - in fact, the
only user of samples_added (ViewBase::on_samples_added) uses the
instance to query only this
As #1 is only a band-aid and not a waterproof solution, I chose
to go with #2.
Up to now, registered callbacks could not be unregistered
because std::function does not permit comparing for equality.
Using an interface class removes the need for std::function,
making the mechanism a little less elegant but at least fully
functional.
Use case is as follows:
- Capture 20+ segments with ~500kS each
- Afterwards, enable conversion for a channel
Without this change, the converted logic will be repainted
20++ times because we are only told that new samples were
added but not which segment.
With this change, the logic trace is only painted when we
see that samples were added to the segment we're showing.
This change is primarily needed because before, newly
created decode signals had a name assigned to them at time
of the constructor call. This changed, and now the name
is empty upon creation, breaking the previously working
header size adjustment.
Instead of using a patterned region on top of the trace,
show the thresholds by using different background colors
for the individual areas: green/grey/red.
If we don't do this, the ruler is out-of-sync with the rest
of the view until the session is loaded and goes into stopped
state. At that point, the ruler is updated, which is too late.
This way, we can use the same mechanism for changing
min/max as well, preventing multiple successive starts
of the conversion algorithm.
Preventing this is necessary because it makes the UI
stop updating for a significant amount of time, which
we obviously don't want.
Two issues here:
1) it's bad style to have an event handler for an event
that was triggered in the same class
2) supplying the current hover point along with the event
is a sensible thing to do