| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
In theory support for PD reset is optional, applications are not
required to make use of it. But it's essential to receive correct
decoding results when used with the popular Pulseview mainline
application.
So let's consider the absence of the reset() method fatal. All mainline
decoders have it. Out-of-tree decoders are easy to adjust, and very
probably should support reset() as well. Thus this change is considered
beneficial, and not harmful.
|
|
When the decoder and session unload functions are called they
remove themselves from the list. The code walking the list
must be careful to avoid accessing the next pointer which
might now be invalid. The g_slist_foreach() takes care of
this.
Reports from Valgrind before fix:
==175436== Invalid read of size 8
--
==175436== Address 0xe3f2598 is 8 bytes inside a block of size 16 free'd
==175436== at 0x4C2FDAC: free (vg_replace_malloc.c:530)
==175436== by 0x563C541: g_free (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x5654783: g_slice_free1 (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x56552A2: g_slist_remove (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x4E3FEFF: srd_session_destroy (session.c:343)
==175436== by 0x4E3F5C7: srd_exit (srd.c:311)
==175436== by 0x40336F: test_inst_new (inst.c:40)
==175436== by 0x53E51D5: srunner_run_tagged (in /usr/lib64/libcheck.so.0.0.0)
==175436== by 0x401237: main (main.c:51)
==175436== Block was alloc'd at
==175436== at 0x4C2EBAB: malloc (vg_replace_malloc.c:299)
==175436== by 0x563C435: g_malloc (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x5654056: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x5655797: g_slist_append (in /usr/lib64/libglib-2.0.so.0.5600.3)
==175436== by 0x4E3FC75: srd_session_new (session.c:71)
==175436== by 0x403345: test_inst_new (inst.c:37)
==175436== by 0x53E51D5: srunner_run_tagged (in /usr/lib64/libcheck.so.0.0.0)
==175436== by 0x401237: main (main.c:51)
|
|
decoder.c: In function ‘srd_decoder_unload_all’:
decoder.c:1080:27: warning: cast between incompatible function types from ‘int (*)(struct srd_decoder *)’ to ‘void (*)(void *, void *)’ [-Wcast-function-type]
g_slist_foreach(pd_list, (GFunc)srd_decoder_unload, NULL);
^
|
|
|
|
|
|
decoder.c:343:9: warning: Access to field 'ob_type' results in a dereference of a null pointer (loaded from variable 'py_default')
if (Py_TYPE(py_default) != Py_TYPE(py_item)) {
^~~~~~~~~~~~~~~~~~~
/usr/include/python3.6m/object.h:118:33: note: expanded from macro 'Py_TYPE'
#define Py_TYPE(ob) (((PyObject*)(ob))->ob_type)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
For the converted calls there's no requirement for all struct fields
being memset()'d to zero, or all struct fields are explicitly set
later anyway.
|
|
|
|
|
|
With these additions, frontends can now call libsigrokdecode API
functions from different threads without running into threading issues.
The backend releases the GIL when it is performing tasks that might take
a while and it doesn't need to run Python/C API calls during that time.
This allows frontends to run multiple PD stacks (in multiple frontend
threads) "at the same time" in a time-sharing, "interlocked" manner.
Whenever one of the decoders is inside e.g. self.wait() it releases the
GIL and thus allows other decoders to do some work in the mean time.
The user-visible effect is that for use-cases such as running 3 different
decoder stacks at the same time for an acquisition, the user will not
have to wait for PD 1 to finish decoding, then wait for PD 2 to finish
decoding, and only *then* being able to see annotations from PD 3.
Instead, all three PDs will decode some chunks of data from time to
time, thus the user is able to inspect annotations from all 3 PDs while
the acquisition and decoding is still going on.
|
|
All decoders must be of PD API version 3 now.
|
|
|
|
|
|
Some Python versions will lead to the following message on stdout currently:
srd: Attribute Error: Failed to load decoder common: no 'Decoder' \
attribute in imported module: 'module' object has no attribute 'Decoder'
This is a harmless (though confusing for users) warning, since "common"
is not an actual PD and it shouldn't be loaded as PD (it just has to
be present).
|
|
In 'except_out' the fail_txt variable is always non-NULL.
|
|
srd_inst_new() used the decoder ID as the instance ID, preventing the use
of multiple instances of the same decoder in the same session. Simply
append a numerical suffix to later instances to allow more.
Required changes to cleanup to reliably free all memory. Valgrind checked.
This fixes parts of bug #868.
Based on original work by: Soeren Apel <soeren@apelpie.net>
Signed-off-by: Karl Palsson <karlp@etactica.com>
|
|
For the time being, both APIs (2 and 3) will remain supported until all
decoders have been converted to API version 3. Then, support for API
version 2 will be dropped.
Decoders using PD v3 API can benefit from both readability improvements
as well as performance improvements. Up to 10x speedup has been measured
in some situations (depends a lot on the decoder, the amount of data,
the amount of edges in the signals, the amount of oversampling etc. etc.).
This is only the first set of (basic) performance improvements for
libsigrokdecode, there are various additional opportunities for further
changes to improve performance.
This changeset has been tested to survive a run of all the test-cases in
the sigrok-test repo without issues (for the converted PDs), however it is
not very well-tested yet, so there might be regressions that need to be
addressed.
|
|
Old versions triggered valgrind errors as the API method to remove an
entry modifies the list that was being iterated.
Signed-off-by: Karl Palsson <karlp@etactica.com>
|
|
Several checks get applied when loading externally provided protocol
decoders. Print error messages when checks fail, so that developers can
better resolve issues.
The sequence of tests terminates upon the first failed check, while
decoders may suffer from several issues at the same time. This is
considered acceptable, as it reduces the commit's size and the code's
complexity. This commit only adds error messages, and does not change
logic/behaviour.
Failed API version checks result in two messages: One specific message
which reflects the decoder's version and what's supported by the loader,
and a generic message that the API version check has failed. This is
done to simplify the logic of a rarely used error code path.
This commit addresses libsigrokdecode Bug 704, and allows to identify
decoders from parallel installations of differing version.
This fixes bug #704.
Signed-off-by: Gerhard Sittig <gerhard.sittig@gmx.net>
|
|
|
|
This issue was discovered via the newly added unit tests.
|
|
Create a utility function for loading a Python module by its name
in UTF-8.
|
|
|
|
Limit usage of the Python C API to the stable ABI subset as defined
by PEP 384. This removes some type definitions and functions which
libsigrokdecode made use of. Convert all affected code to suitable
API alternatives. Also fix a few leaks that became apparent while
working on the code.
The most visible change is that PyTypeObject is now an opaque type.
Thus, the custom Decoder and srd_logic types are now created on the
heap via an alternative API. Unfortunately, since tp_name is now
inaccessible, type names had to be removed from the log output.
Stack traces after Python exceptions are now formatted by calling
into Python, since the trace object C API is no longer available.
|
|
Since Autoconf places some important feature flags only into the
configuration header, it is necessary to include it globally to
guarantee a consistent build.
|
|
|
|
Use g_malloc*() for small allocations and assume they always
succeed. Simplify error handling in a few places accordingly.
Document the rules in the README file.
|
|
This fixes bug #378.
|
|
This prevents Python.h from being included into client code, where
it can mess things up by e.g. redefining _POSIX_C_SOURCE.
|
|
srd_decoder_load() works perfectly with decoders stored inside zip files.
However, srd_decoder_load_all{_path,}() did not, because it assumed that
normal directory listings can be used to enumerate decoders, which is not
the case for zipped decoders. Fix this by providing a fallback based on
the zipimporter Python class.
|
|
Decoders that implement any other PD API version are per definition not
compatible and cannot work with this library version.
|
|
|
|
|
|
The function srd_check_init() is only used in decoder.c.
|
|
|
|
Variables of type 'struct srd_channel *' are consistently named 'pdch' to
make them easily distinguishable from libsigrok's 'struct sr_channel *'
variables that are consistently named 'ch'.
|
|
|
|
Annotation entries also consist of a tuple, not a list.
|
|
Each option consists of a dictionary with the following keys:
id The option id, which is passed in when setting a value.
desc A description of the option, suitable for display.
def The default value for this option.
values (optional) If present, a tuple containing values the option
may take. They must be of the same type as the default.
Valid types for the options are UTF-8-encoded strings, integers, and
floating point values.
|
|
An annotation row is a list of annotation classes that should all be
displayed in the same "row" in GUIs.
For example, the UART decoder would have at least two rows (for decoded
RX and TX data/startbits/stopbits/paritybits), the SPI decoder would have
at least two rows (for decoded MISO and MOSI data), and so on.
Each annotation row has a short ID string (mostly for use in
command-line frontends), a description string (mostly for use by GUIs),
and a tuple/list of annotation class IDs belonging to this row.
If no annotation rows are provided by the decoder, frontends can
consider this as the "trivial" case of all annotation classes being
displayed on the same (only) row.
|
|
|
|
|
|
srd_decoder_load_all() was really only getting a list of decoders
from the default (installation) path, so could not find uninstalled
decoders, or those in a custom added search path.
This broke development of new PDs when using the SIGROKDECODE_DIR
environment variable, and broke many of the unit tests in the tests/
directory -- those actually tested against the already-installed
decoders, not the ones about to be installed.
|
|
|
|
|
|
Python silently uses the existing module anyway, but the library
was counting it as an extra module. This was exposed by a test
case in the test suite.
|
|
This fixes bug 177.
|
