doc: update sigrok-cli(1) for channel assignment to decoder inputs

Extend the sigrok-cli manpage section which discusses the -P option.
Mention the assignment of logic channels to decoder inputs by index
or by name.

Automatic assignment by position in the absence of user specs seems
to not have been recent implementations' default behaviour. Remove
that outdated or incorrect comment.

diff --git a/HACKING b/HACKING
index c9b876b1df7dc57a1df85cfe0aaaec6c815337e3..36c6756faaccd9228019d84a17f5aee8eeec9f0b 100644 (file)
--- a/HACKING
+++ b/HACKING
@@ -29,18 +29,27 @@ Contributions
 Random notes
 ------------
 
 Random notes
 ------------
 

- - Consistently use g_try_malloc() / g_try_malloc0(). Do not use standard
+ - Don't do variable declarations in compound statements, only at the
+   beginning of a function.
+
+ - Generally avoid assigning values to variables at declaration time,
+   especially so for complex and/or run-time dependent values.
+
+ - Consistently use g_*malloc() / g_*malloc0(). Do not use standard

    malloc()/calloc() if it can be avoided (sometimes other libs such
    as libftdi can return malloc()'d memory, for example).
 
  - Always properly match allocations with the proper *free() functions. If
    malloc()/calloc() if it can be avoided (sometimes other libs such
    as libftdi can return malloc()'d memory, for example).
 
  - Always properly match allocations with the proper *free() functions. If

-   glib's g_try_malloc()/g_try_malloc0() was used, use g_free() to free the
+   glib's g_*malloc()/g_*malloc0() was used, use g_free() to free the

    memory. Otherwise use standard free(). Never use the wrong function!
 
    memory. Otherwise use standard free(). Never use the wrong function!
 

- - Never use g_malloc() or g_malloc0(). These functions do not return NULL
-   if not enough memory is available but rather lead to an exit() or segfault
-   instead. This behaviour is not acceptable.
-   Use g_try_malloc()/g_try_malloc0() instead and check the return value.
+ - We assume that "small" memory allocations (< 1MB) will always succeed.
+   Thus, it's fine to use g_malloc() or g_malloc0() for allocations of
+   simple/small structs and such (instead of using g_try_malloc()), and
+   there's no need to check the return value.
+
+   Do use g_try_malloc() or g_try_malloc0() for large (>= 1MB) allocations
+   and check the return value.

 
  - You should never print any messages (neither to stdout nor stderr nor
    elsewhere) "manually" via e.g. printf() or g_log() or similar functions.
 
  - You should never print any messages (neither to stdout nor stderr nor
    elsewhere) "manually" via e.g. printf() or g_log() or similar functions.

diff --git a/decode.c b/decode.c
index 4ddf34afbbaf00db414a041f4ef2e61ddc27bc0f..12083a1707f5d0171096b188db6e87176f6c1dd5 100644 (file)
--- a/decode.c
+++ b/decode.c
@@ -33,6 +33,10 @@ uint64_t pd_samplerate = 0;
 
 extern struct srd_session *srd_sess;
 
 
 extern struct srd_session *srd_sess;
 

+static const char *keyword_assign = "assign_channels";
+static const char *assign_by_index = "auto_index";
+static const char *assign_by_name = "auto_names";
+

 static int opts_to_gvar(struct srd_decoder *dec, GHashTable *hash,
                GHashTable **options)
 {
 static int opts_to_gvar(struct srd_decoder *dec, GHashTable *hash,
                GHashTable **options)
 {


@@ -89,7 +93,7 @@ static int opts_to_gvar(struct srd_decoder *dec, GHashTable *hash,
        return ret;
 }
 
        return ret;
 }
 

-static int move_hash_element(GHashTable *src, GHashTable *dest, void *key)
+static int move_hash_element(GHashTable *src, GHashTable *dest, const void *key)

 {
        void *orig_key, *value;
 
 {
        void *orig_key, *value;
 


@@ -111,6 +115,7 @@ static GHashTable *extract_channel_map(struct srd_decoder *dec, GHashTable *hash
        channel_map = g_hash_table_new_full(g_str_hash, g_str_equal,
                                          g_free, g_free);
 
        channel_map = g_hash_table_new_full(g_str_hash, g_str_equal,
                                          g_free, g_free);
 

+       move_hash_element(hash, channel_map, keyword_assign);

        for (l = dec->channels; l; l = l->next) {
                pdch = l->data;
                move_hash_element(hash, channel_map, pdch->id);
        for (l = dec->channels; l; l = l->next) {
                pdch = l->data;
                move_hash_element(hash, channel_map, pdch->id);


@@ -253,11 +258,21 @@ static void map_pd_inst_channels(void *key, void *value, void *user_data)
        GHashTable *channel_indices;
        GSList *channel_list;
        struct srd_decoder_inst *di;
        GHashTable *channel_indices;
        GSList *channel_list;
        struct srd_decoder_inst *di;

+       struct srd_decoder *pd;

        GVariant *var;
        void *channel_id;
        void *channel_target;
        struct sr_channel *ch;
        GHashTableIter iter;
        GVariant *var;
        void *channel_id;
        void *channel_target;
        struct sr_channel *ch;
        GHashTableIter iter;

+       enum assign_t {
+               ASSIGN_UNKNOWN,
+               ASSIGN_USER_SPEC,
+               ASSIGN_BY_INDEX,
+               ASSIGN_BY_NAMES,
+       } assign;
+       const char *keyword;
+       GSList *l_pd, *l_pdo, *l_ch;
+       struct srd_channel *pdch;

 
        channel_map = value;
        channel_list = user_data;
 
        channel_map = value;
        channel_list = user_data;


@@ -271,14 +286,108 @@ static void map_pd_inst_channels(void *key, void *value, void *user_data)
        channel_indices = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
                                              (GDestroyNotify)g_variant_unref);
 
        channel_indices = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
                                              (GDestroyNotify)g_variant_unref);
 

+       /*
+        * The typical mode of operation is to apply a user specified
+        * mapping of sigrok channels to decoder inputs. Lack of mapping
+        * specs will assign no signals (compatible behaviour to earlier
+        * implementations).
+        *
+        * Alternatively we can assign sigrok logic channels to decoder
+        * inputs in the strict order of channel indices, or when their
+        * names match. Users need to request this automatic assignment
+        * though, because this behaviour differs from earlier versions.
+        *
+        * Even more sophisticated heuristics of mapping sigrok channels
+        * to decoder inputs are not implemented here. Later versions
+        * could translate the Pulseview approach to the C language, but
+        * it's better to stabilize that logic first before porting it.
+        */
+       assign = ASSIGN_USER_SPEC;
+       keyword = g_hash_table_lookup(channel_map, keyword_assign);
+       if (g_hash_table_size(channel_map) != 1) {
+               assign = ASSIGN_USER_SPEC;
+       } else if (!keyword) {
+               assign = ASSIGN_USER_SPEC;
+       } else if (strcmp(keyword, assign_by_index) == 0) {
+               assign = ASSIGN_BY_INDEX;
+       } else if (strcmp(keyword, assign_by_name) == 0) {
+               assign = ASSIGN_BY_NAMES;
+       } else {
+               g_critical("Unknown type of decoder channel assignment: %s.",
+                       keyword);
+               return;
+       }
+
+       pd = di->decoder;
+       if (assign == ASSIGN_BY_INDEX) {
+               /*
+                * Iterate the protocol decoder's list of input signals
+                * (mandatory and optional, never more than the decoder's
+                * total channels count). Assign sigrok logic channels
+                * until either is exhausted. Use sigrok channels in the
+                * very order of their declaration in the input stream.
+                */
+               l_ch = channel_list;
+               l_pd = pd->channels;
+               while (l_ch && l_pd) {
+                       ch = l_ch->data;
+                       l_ch = l_ch->next;
+                       if (ch->type != SR_CHANNEL_LOGIC)
+                               break;
+                       if (ch->index >= di->dec_num_channels)
+                               break;
+                       pdch = l_pd->data;
+                       l_pd = l_pd->next;
+                       if (!l_pd)
+                               l_pd = pd->opt_channels;
+                       /* TODO Emit an INFO message. */
+                       g_hash_table_insert(channel_map,
+                               g_strdup(pdch->id), g_strdup(ch->name));
+               }
+       } else if (assign == ASSIGN_BY_NAMES) {
+               /*
+                * Iterate the protocol decoder's list of input signals.
+                * Search for sigrok logic channels that have matching
+                * names (case insensitive comparison). Not finding a
+                * sigrok channel of a given name is non-fatal (could be
+                * an optional channel, the decoder will check later for
+                * all mandatory channels to be assigned).
+                */
+               l_pd = pd->channels;
+               l_pdo = pd->opt_channels;
+               while (l_pd) {
+                       pdch = l_pd->data;
+                       l_pd = l_pd->next;
+                       if (!l_pd) {
+                               l_pd = l_pdo;
+                               l_pdo = NULL;
+                       }
+                       ch = find_channel(channel_list, pdch->id, FALSE);
+                       if (!ch) {
+                               /* TODO Emit a WARN message. */
+                               /* if (l_pdo) ...; */
+                               continue;
+                       }
+                       if (ch->type != SR_CHANNEL_LOGIC) {
+                               /* TODO Emit a WARN message. */
+                               continue;
+                       }
+                       /* TODO Emit an INFO message. */
+                       g_hash_table_insert(channel_map,
+                               g_strdup(pdch->id), g_strdup(ch->name));
+               }
+       }
+

        g_hash_table_iter_init(&iter, channel_map);
        while (g_hash_table_iter_next(&iter, &channel_id, &channel_target)) {
        g_hash_table_iter_init(&iter, channel_map);
        while (g_hash_table_iter_next(&iter, &channel_id, &channel_target)) {

+               if (strcmp(channel_id, keyword_assign) == 0)
+                       continue;

                if (!channel_target) {
                        g_printerr("cli: Channel name for \"%s\" missing.\n",
                                   (char *)channel_id);
                        continue;
                }
                if (!channel_target) {
                        g_printerr("cli: Channel name for \"%s\" missing.\n",
                                   (char *)channel_id);
                        continue;
                }

-               ch = find_channel(channel_list, channel_target);
+               ch = find_channel(channel_list, channel_target, TRUE);

                if (!ch) {
                        g_printerr("cli: No channel with name \"%s\" found.\n",
                                   (char *)channel_target);
                if (!ch) {
                        g_printerr("cli: No channel with name \"%s\" found.\n",
                                   (char *)channel_target);


@@ -684,6 +793,8 @@ void show_pd_annotations(struct srd_proto_data *pdata, void *cb_data)
                show_class = TRUE;
                show_abbrev = TRUE;
        }
                show_class = TRUE;
                show_abbrev = TRUE;
        }

+       if (opt_pd_ann_class)
+               show_class = TRUE;

 
        /*
         * Display the annotation's fields after the layout was
 
        /*
         * Display the annotation's fields after the layout was

diff --git a/doc/sigrok-cli.1 b/doc/sigrok-cli.1
index b333bee93de02f34357a80cc0929d6a1a5db58e9..33174cdbe4a6ec545e48a370bb4efd7cb52d0ee5 100644 (file)
--- a/doc/sigrok-cli.1
+++ b/doc/sigrok-cli.1
@@ -1,4 +1,4 @@
-.TH SIGROK\-CLI 1 "March 28, 2019"
+.TH SIGROK\-CLI 1 "April 07, 2023"

 .SH "NAME"
 sigrok\-cli \- Command-line client for the sigrok software
 .SH "SYNOPSIS"
 .SH "NAME"
 sigrok\-cli \- Command-line client for the sigrok software
 .SH "SYNOPSIS"


@@ -311,15 +311,27 @@ In this example,
 .B wordsize
 is an option supported by the
 .B spi
 .B wordsize
 is an option supported by the
 .B spi

-protocol decoder. Additionally, the user tells sigrok to decode the SPI
+protocol decoder. Additionally, the user requests sigrok to decode the SPI

 protocol using channel 1 as MISO signal for SPI, channel 5 as MOSI, channel 3
 as CLK, and channel 0 as CS# signal.
 .sp
 protocol using channel 1 as MISO signal for SPI, channel 5 as MOSI, channel 3
 as CLK, and channel 0 as CS# signal.
 .sp

-Notice that the
+The

 .B sigrok\-cli
 .B sigrok\-cli

-application does not support "name matching". Instead it's assumed that the
-traces in the input stream match the order of the decoder's input signals,
-or that users explicitly specify the input channel to decoder signal mapping.
+application can automatically assign logic channels to decoder inputs
+in their strict channel index order (by means of the
+.B auto_index
+keyword), or can automatically assign logic channels to decoder inputs
+when their names match (case insensitive match,
+.B auto_names
+keyword). Users need to explicitly request this assignment, the default
+behaviour is to not automatically assign any signals.
+.sp
+Example:
+.sp
+ $
+.B "sigrok\-cli \-i <file.sr>"
+.br
+.B "              \-P ieee488:assign_channels=auto_names"

 .br
 .sp
 When multiple decoders are specified in the same
 .br
 .sp
 When multiple decoders are specified in the same


@@ -426,6 +438,13 @@ Not every decoder generates binary output.
 When given, decoder annotations will include sample numbers, too.
 This allows consumers to receive machine readable timing information.
 .TP
 When given, decoder annotations will include sample numbers, too.
 This allows consumers to receive machine readable timing information.
 .TP

+.BR "\-\-protocol\-decoder\-ann\-class
+When given, decoder annotations will include annotation class names.
+.TP
+.BR "\-\-protocol\-decoder\-jsontrace
+When given, decoder output uses the Google Trace Event format (JSON).
+Which can be inspected in web browsers or other viewers.
+.TP

 .BR "\-l, \-\-loglevel " <level>
 Set the libsigrok and libsigrokdecode loglevel. At the moment \fBsigrok\-cli\fP
 doesn't support setting the two loglevels independently. The higher the
 .BR "\-l, \-\-loglevel " <level>
 Set the libsigrok and libsigrokdecode loglevel. At the moment \fBsigrok\-cli\fP
 doesn't support setting the two loglevels independently. The higher the


@@ -581,6 +600,25 @@ To turn on internal logging on a Lascar EL-USB series device:
 .SH "EXIT STATUS"
 .B sigrok\-cli
 exits with 0 on success, 1 on most failures.
 .SH "EXIT STATUS"
 .B sigrok\-cli
 exits with 0 on success, 1 on most failures.

+.SH "ENVIRONMENT"
+.TP
+.B SIGROK_FIRMWARE_DIR
+A single path where to search for firmware images, in addition to a
+builtin list of locations.
+.TP
+.B SIGROK_FIRMWARE_PATH
+Multiple path entries where to search for firmware images, in addition
+to builtin locations.
+.TP
+When decoder support was enabled in the application's configuration:
+.TP
+.B SIGROKDECODE_DIR
+A single path where to search for protocol decoders, in addition to
+a builtin list of locations.
+.TP
+.B SIGROKDECODE_PATH
+Multiple path entries where to search for protocol decoders, in addition
+to builtin locations.

 .SH "SEE ALSO"
 \fBpulseview\fP(1)
 .SH "BUGS"
 .SH "SEE ALSO"
 \fBpulseview\fP(1)
 .SH "BUGS"

diff --git a/options.c b/options.c
index 2573bd4c17b6cb861be37239070ff6e7bc8e07ef..e47b6804e96eb7635f9874b3090cb02110526dae 100644 (file)
--- a/options.c
+++ b/options.c
@@ -40,6 +40,7 @@ gchar **opt_pds = NULL;
 gchar *opt_pd_annotations = NULL;
 gchar *opt_pd_meta = NULL;
 gchar *opt_pd_binary = NULL;
 gchar *opt_pd_annotations = NULL;
 gchar *opt_pd_meta = NULL;
 gchar *opt_pd_binary = NULL;

+gboolean opt_pd_ann_class = FALSE;

 gboolean opt_pd_samplenum = FALSE;
 gboolean opt_pd_jsontrace = FALSE;
 #endif
 gboolean opt_pd_samplenum = FALSE;
 gboolean opt_pd_jsontrace = FALSE;
 #endif


@@ -139,6 +140,8 @@ static const GOptionEntry optargs[] = {
                        "Protocol decoder meta output to show", NULL},
        {"protocol-decoder-binary", 'B', 0, G_OPTION_ARG_CALLBACK, &check_opt_pd_binary,
                        "Protocol decoder binary output to show", NULL},
                        "Protocol decoder meta output to show", NULL},
        {"protocol-decoder-binary", 'B', 0, G_OPTION_ARG_CALLBACK, &check_opt_pd_binary,
                        "Protocol decoder binary output to show", NULL},

+       {"protocol-decoder-ann-class", 0, 0, G_OPTION_ARG_NONE, &opt_pd_ann_class,
+                       "Show annotation class in decoder output", NULL},

        {"protocol-decoder-samplenum", 0, 0, G_OPTION_ARG_NONE, &opt_pd_samplenum,
                        "Show sample numbers in decoder output", NULL},
        {"protocol-decoder-jsontrace", 0, 0, G_OPTION_ARG_NONE, &opt_pd_jsontrace,
        {"protocol-decoder-samplenum", 0, 0, G_OPTION_ARG_NONE, &opt_pd_samplenum,
                        "Show sample numbers in decoder output", NULL},
        {"protocol-decoder-jsontrace", 0, 0, G_OPTION_ARG_NONE, &opt_pd_jsontrace,

diff --git a/parsers.c b/parsers.c
index b74d74fe98792ac314094be4bf927b1e11b51fb3..2aacefb089f184eeb18dfb8d5be8855dc5623811 100644 (file)
--- a/parsers.c
+++ b/parsers.c
@@ -25,7 +25,8 @@
 #include <glib.h>
 #include "sigrok-cli.h"
 
 #include <glib.h>
 #include "sigrok-cli.h"
 

-struct sr_channel *find_channel(GSList *channellist, const char *channelname)
+struct sr_channel *find_channel(GSList *channellist, const char *channelname,
+       gboolean exact_case)

 {
        struct sr_channel *ch;
        GSList *l;
 {
        struct sr_channel *ch;
        GSList *l;


@@ -33,8 +34,13 @@ struct sr_channel *find_channel(GSList *channellist, const char *channelname)
        ch = NULL;
        for (l = channellist; l; l = l->next) {
                ch = l->data;
        ch = NULL;
        for (l = channellist; l; l = l->next) {
                ch = l->data;

-               if (!strcmp(ch->name, channelname))
-                       break;
+               if (exact_case) {
+                       if (strcmp(ch->name, channelname) == 0)
+                               break;
+               } else {
+                       if (g_ascii_strcasecmp(ch->name, channelname) == 0)
+                               break;
+               }

        }
        ch = l ? l->data : NULL;
 
        }
        ch = l ? l->data : NULL;
 


@@ -105,7 +111,7 @@ GSList *parse_channelstring(struct sr_dev_inst *sdi, const char *channelstring)
                                        ret = SR_ERR;
                                        break;
                                }
                                        ret = SR_ERR;
                                        break;
                                }

-                               ch = find_channel(channels, str);
+                               ch = find_channel(channels, str, TRUE);

                                if (!ch) {
                                        g_critical("unknown channel '%d'.", b);
                                        ret = SR_ERR;
                                if (!ch) {
                                        g_critical("unknown channel '%d'.", b);
                                        ret = SR_ERR;


@@ -130,7 +136,7 @@ range_fail:
                                break;
                        }
 
                                break;
                        }
 

-                       ch = find_channel(channels, names[0]);
+                       ch = find_channel(channels, names[0], TRUE);

                        if (!ch) {
                                g_critical("unknown channel '%s'.", names[0]);
                                g_strfreev(names);
                        if (!ch) {
                                g_critical("unknown channel '%s'.", names[0]);
                                g_strfreev(names);


@@ -364,6 +370,8 @@ GHashTable *parse_generic_arg(const char *arg,
                i++;
        }
        for (; elements[i]; i++) {
                i++;
        }
        for (; elements[i]; i++) {

+               if (!elements[i][0])
+                       continue;

                split_key_value(elements[i], &k, &v);
                k = g_strdup(k);
                v = v ? g_strdup(v) : NULL;
                split_key_value(elements[i], &k, &v);
                k = g_strdup(k);
                v = v ? g_strdup(v) : NULL;

diff --git a/sigrok-cli.h b/sigrok-cli.h
index 2c0fbdcf57d2358b9864cce8676b39e1e7bba1f0..b33b076bd1f82824d284395ed7ca4267fd3b516c 100644 (file)
--- a/sigrok-cli.h
+++ b/sigrok-cli.h
@@ -103,7 +103,8 @@ void map_pd_channels(struct sr_dev_inst *sdi);
 #endif
 
 /* parsers.c */
 #endif
 
 /* parsers.c */

-struct sr_channel *find_channel(GSList *channellist, const char *channelname);
+struct sr_channel *find_channel(GSList *channellist, const char *channelname,
+       gboolean exact_case);

 GSList *parse_channelstring(struct sr_dev_inst *sdi, const char *channelstring);
 int parse_triggerstring(const struct sr_dev_inst *sdi, const char *s,
                struct sr_trigger **trigger);
 GSList *parse_channelstring(struct sr_dev_inst *sdi, const char *channelstring);
 int parse_triggerstring(const struct sr_dev_inst *sdi, const char *s,
                struct sr_trigger **trigger);


@@ -140,6 +141,7 @@ extern gchar **opt_pds;
 extern gchar *opt_pd_annotations;
 extern gchar *opt_pd_meta;
 extern gchar *opt_pd_binary;
 extern gchar *opt_pd_annotations;
 extern gchar *opt_pd_meta;
 extern gchar *opt_pd_binary;

+extern gboolean opt_pd_ann_class;

 extern gboolean opt_pd_samplenum;
 extern gboolean opt_pd_jsontrace;
 #endif
 extern gboolean opt_pd_samplenum;
 extern gboolean opt_pd_jsontrace;
 #endif