Skip to content

Conversation

@wsakernel
Copy link

I found these things while reviewing your patches for libsigrok. What do you think?

v1ne and others added 29 commits November 18, 2020 23:58
While the raw sample is the actual received data, the expanded sample is the
crucial data I as a developer am interested in seeing. At least if I assume that
the sample expansion works.
This fixes a bug where the channel group setting would not be overwritten in a
new run, but instead the channel group settings of both runs would be merged
together.
... and thus selectable in PulseView.
This copies the code verbatim from one place to another in preparation to adding
support for complex triggers.
It just means "number of compressed samples", but it's still highly useful
to decice how much data to capture. If this is unavailable, PulseView
provides a weird list of up to 1 TSmpl, which makes even less sense.
This further reduces clutter. An exception is where an error message is
logged. In those cases, I left the old pattern in place.
On my OLS with Demon Core (v3.07), the following command produces only low
levels on channels 8-15:
sigrok-cli -d ols:conn=/dev/ttyACM0 --output-format ascii:width=128 \
  --config='samplerate=200M:pattern=Internal:captureratio=50' \
  --channels 0-15 --samples 32 --triggers "12=1"

This doesn't make sense since it only triggers if channel 12 is high.
By not overwriting the channel data, I get the desired output. The data
is processed in groups of 2 bytes, so there's no need to fold back "input"
from devc->sample[2..3] because these bytes are always zero.
The additional stage prevented OLS from using 4 stages.
On my unit with Demon Core v3.07, I don't see a stage-dependent delay. Thus,
set the delay so that in my tests, the trigger point lines up with the
edge of a test signal.
Because in RLE mode, we don't know how many samples we could read. The
limit that we tell the hardware is only the limit in number of compressed
samples, not the number of uncompressed samples.
The number of samples to be received is pretty clear if no RLE is being used.
But if RLE is being used, the number of raw samples must be counted. For now,
read samples until we time out. This should work for RLE and non-RLE, but
may break if the device is connected via a high-latency network connection.
If the devices times out during a transfer, it can be that num_samples
is smaller than trigger_at_smpl.
In my experiments, this helped when not all data was read from the device.
Otherwise, sigrok-cli would just hang.

Also, be nice and reset the device when an acquisition ends. This is not
strictly necessary, but is a nice touch, I think.
cnt_samples_rle was larger than num_samples in RLE mode. Instead of
correcting that, use num_samples instead.
…of 4

The exact number must be known when reading data so that we can read the
right number of samples without incuring a timeout penalty.
Even with RLE enabled, we know how many samples to expect: Exactly the
configured number, no less, no more. Thus, when that number is received,
begin processing immediately. Also, put a warning so that we can get to
know if the calculation is off.

Since this works, the timeout can be bumped to provide for better
usability if the logic sniffer is used over a slow network connection.
We're talking about 20 kB, so there's little need to go back to the event
loop in between. But there's also no reason why we should go back to the
event loop if we can read more data immediately.
Why just read a single byte when you can read a whole sample?
Due to the nature of RLE, the number of samples from the start to the
trigger point is unknown. What is known is that the hardware triggers still
records a given number of raw samples to its sample memory. When reading
and expanding these raw samples from the back (remember, OLS sends the
recording back-to-front), remember the expanded sample position from the
back and then map it back to the right position from the front once the
total number of expanded samples is known.
This mode can double the space available for RLE messages because unchanged
values don't have to be repeated.
This adds code from http://web.archive.org/web/20190317154112/
http://mygizmos.org/ols/Logic-Sniffer-FPGA-Spec.pdf (GPL2 with the option
to relicense it to any later version of that license) with reformatting
and without typos to set up the LUT bits.

The trigger setup starts with a delay to collect the required number of
pre-trigger samples. Afterwards, the remaining samples are captured or
further trigger stages follow.

Each of these extra stages mirrors what the user has defined as trigger
pattern: Level and edge triggers are combined and the state machine only
advances to the next stage if all levels and at least one edge meets the
conditions. Contrary to level triggers, edge triggers are ORed together.
This is an undocumented property of the Demon Core.
If the number of basic triger stages is too large, it is still copied to
the device struct. Use a temporary variable and populate the struct only
if it is valid.
More efficient but as readable.
@wsakernel wsakernel force-pushed the ols/num_triggers_on_failure branch from 701c53c to 52c302d Compare January 17, 2021 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants