]> sigrok.org Git - libsigrok.git/blobdiff - src/output/csv.c
output/csv: check unsupported/untested input signal conditions
[libsigrok.git] / src / output / csv.c
index c2450e9ce1115f3faebf8c0e5a8b09108dc4debe..49d6eb6ec457ea2b211aaab4e897cbd72ce4566c 100644 (file)
  *          Values are "channel", "units", "off". Defaults to "units".
  *
  * time:    Whether or not the first column should include the time the sample
- *          was taken. Defaults to TRUE.
+ *          was taken. Defaults to FALSE.
  *
  * trigger: Whether or not to add a "trigger" column as the last column.
  *          Defaults to FALSE.
  *
- * dedup:   Don't output duplicate rows. Defaults to TRUE. If time is off, then
+ * dedup:   Don't output duplicate rows. Defaults to FALSE. If time is off, then
  *          this is forced to be off.
  */
 
-#include <math.h>
 #include <config.h>
+#include <math.h>
 #include <stdlib.h>
 #include <string.h>
 #include <glib.h>
@@ -93,13 +93,19 @@ struct context {
        uint32_t num_samples;
        uint32_t channel_count, logic_channel_count;
        uint32_t channels_seen;
-       uint64_t period;
-       uint64_t sample_time;
+       uint64_t sample_rate;
+       uint64_t sample_scale;
+       uint64_t out_sample_count;
        uint8_t *previous_sample;
        float *analog_samples;
        uint8_t *logic_samples;
        const char *xlabel;     /* Don't free: will point to a static string. */
        const char *title;      /* Don't free: will point into the driver struct. */
+
+       /* Input data constraints check. */
+       gboolean have_checked;
+       gboolean have_frames;
+       uint64_t pkt_snums;
 };
 
 /*
@@ -216,33 +222,30 @@ static GString *gen_header(const struct sr_output *o,
        struct sr_channel *ch;
        GVariant *gvar;
        GString *header;
-       GSList *l;
+       GSList *channels, *l;
        unsigned int num_channels, i;
-       uint64_t samplerate = 0, sr;
        char *samplerate_s;
 
        ctx = o->priv;
        header = g_string_sized_new(512);
 
-       if (ctx->period == 0) {
+       if (ctx->sample_rate == 0) {
                if (sr_config_get(o->sdi->driver, o->sdi, NULL,
                                  SR_CONF_SAMPLERATE, &gvar) == SR_OK) {
-                       samplerate = g_variant_get_uint64(gvar);
+                       ctx->sample_rate = g_variant_get_uint64(gvar);
                        g_variant_unref(gvar);
                }
 
                i = 0;
-               sr = 1;
-               while (sr < samplerate) {
+               ctx->sample_scale = 1;
+               while (ctx->sample_scale < ctx->sample_rate) {
                        i++;
-                       sr *= 1000;
+                       ctx->sample_scale *= 1000;
                }
-               if (samplerate)
-                       ctx->period = sr / samplerate;
                if (i < ARRAY_SIZE(xlabels))
                        ctx->xlabel = xlabels[i];
-               sr_info("Set sample period to %" PRIu64 " %s",
-                       ctx->period, ctx->xlabel);
+               sr_info("Set sample rate, scale to %" PRIu64 ", %" PRIu64 " %s",
+                       ctx->sample_rate, ctx->sample_scale, ctx->xlabel);
        }
        ctx->title = (o->sdi && o->sdi->driver) ? o->sdi->driver->longname : "unknown";
 
@@ -252,25 +255,27 @@ static GString *gen_header(const struct sr_output *o,
                g_string_append_printf(header,
                        "%s CSV generated by %s %s\n%s from %s on %s",
                        ctx->comment, PACKAGE_NAME,
-                       SR_PACKAGE_VERSION_STRING, ctx->comment,
+                       sr_package_version_string_get(), ctx->comment,
                        ctx->title, ctime(&hdr->starttime.tv_sec));
 
                /* Columns / channels */
-               num_channels = g_slist_length(o->sdi->channels);
+               channels = o->sdi ? o->sdi->channels : NULL;
+               num_channels = g_slist_length(channels);
                g_string_append_printf(header, "%s Channels (%d/%d):",
                        ctx->comment, ctx->num_analog_channels +
                        ctx->num_logic_channels, num_channels);
-               for (l = o->sdi->channels; l; l = l->next) {
+               for (l = channels; l; l = l->next) {
                        ch = l->data;
                        if (ch->enabled)
                                g_string_append_printf(header, " %s,", ch->name);
                }
-               if (o->sdi->channels)
+               if (channels) {
                        /* Drop last separator. */
                        g_string_truncate(header, header->len - 1);
+               }
                g_string_append_printf(header, "\n");
-               if (samplerate != 0) {
-                       samplerate_s = sr_samplerate_string(samplerate);
+               if (ctx->sample_rate != 0) {
+                       samplerate_s = sr_samplerate_string(ctx->sample_rate);
                        g_string_append_printf(header, "%s Samplerate: %s\n",
                                               ctx->comment, samplerate_s);
                        g_free(samplerate_s);
@@ -278,6 +283,10 @@ static GString *gen_header(const struct sr_output *o,
                ctx->did_header = TRUE;
        }
 
+       /* Time column requested but samplerate unknown. Emit a warning. */
+       if (ctx->time && !ctx->sample_rate)
+               sr_warn("Samplerate unknown, cannot provide timestamps.");
+
        return header;
 }
 
@@ -308,10 +317,13 @@ static void process_analog(struct context *ctx,
                           const struct sr_datafeed_analog *analog)
 {
        int ret;
-       unsigned int i, j, c, num_channels;
+       size_t num_rcvd_ch, num_have_ch;
+       size_t idx_have, idx_smpl, idx_rcvd;
+       size_t idx_send;
        struct sr_analog_meaning *meaning;
        GSList *l;
        float *fdata = NULL;
+       struct sr_channel *ch;
 
        if (!ctx->analog_samples) {
                ctx->analog_samples = g_malloc(analog->num_samples
@@ -324,31 +336,34 @@ static void process_analog(struct context *ctx,
                        ctx->num_samples, analog->num_samples);
 
        meaning = analog->meaning;
-       num_channels = g_slist_length(meaning->channels);
-       ctx->channels_seen += num_channels;
-       sr_dbg("Processing packet of %u analog channels", num_channels);
-       fdata = g_malloc(analog->num_samples * num_channels * sizeof(float));
+       num_rcvd_ch = g_slist_length(meaning->channels);
+       ctx->channels_seen += num_rcvd_ch;
+       sr_dbg("Processing packet of %zu analog channels", num_rcvd_ch);
+       fdata = g_malloc(analog->num_samples * num_rcvd_ch * sizeof(float));
        if ((ret = sr_analog_to_float(analog, fdata)) != SR_OK)
                sr_warn("Problems converting data to floating point values.");
 
-       for (i = 0; i < ctx->num_analog_channels + ctx->num_logic_channels; i++) {
-               if (ctx->channels[i].ch->type == SR_CHANNEL_ANALOG) {
-                       sr_dbg("Looking for channel %s",
-                              ctx->channels[i].ch->name);
-                       for (l = meaning->channels, c = 0; l; l = l->next, c++) {
-                               struct sr_channel *ch = l->data;
-                               sr_dbg("Checking %s", ch->name);
-                               if (ctx->channels[i].ch == l->data) {
-                                       if (ctx->label_do && !ctx->label_names) {
-                                               sr_analog_unit_to_string(analog,
-                                                       &ctx->channels[i].label);
-                                       }
-                                       for (j = 0; j < analog->num_samples; j++)
-                                               ctx->analog_samples[j * ctx->num_analog_channels + i] = fdata[j * num_channels + c];
-                                       break;
-                               }
+       num_have_ch = ctx->num_analog_channels + ctx->num_logic_channels;
+       idx_send = 0;
+       for (idx_have = 0; idx_have < num_have_ch; idx_have++) {
+               if (ctx->channels[idx_have].ch->type != SR_CHANNEL_ANALOG)
+                       continue;
+               sr_dbg("Looking for channel %s",
+                      ctx->channels[idx_have].ch->name);
+               for (l = meaning->channels, idx_rcvd = 0; l; l = l->next, idx_rcvd++) {
+                       ch = l->data;
+                       sr_dbg("Checking %s", ch->name);
+                       if (ctx->channels[idx_have].ch != ch)
+                               continue;
+                       if (ctx->label_do && !ctx->label_names) {
+                               sr_analog_unit_to_string(analog,
+                                       &ctx->channels[idx_have].label);
                        }
+                       for (idx_smpl = 0; idx_smpl < analog->num_samples; idx_smpl++)
+                               ctx->analog_samples[idx_smpl * ctx->num_analog_channels + idx_send] = fdata[idx_smpl * num_rcvd_ch + idx_rcvd];
+                       break;
                }
+               idx_send++;
        }
        g_free(fdata);
 }
@@ -393,6 +408,8 @@ static void process_logic(struct context *ctx,
 static void dump_saved_values(struct context *ctx, GString **out)
 {
        unsigned int i, j, analog_size, num_channels;
+       double sample_time_dbl;
+       uint64_t sample_time_u64;
        float *analog_sample, value;
        uint8_t *logic_sample;
 
@@ -410,8 +427,8 @@ static void dump_saved_values(struct context *ctx, GString **out)
                if (ctx->label_do) {
                        if (ctx->time)
                                g_string_append_printf(*out, "%s%s",
-                                       ctx->label_names ? "Time" :
-                                       ctx->xlabel, ctx->value);
+                                       ctx->label_names ? "Time" : ctx->xlabel,
+                                       ctx->value);
                        for (i = 0; i < num_channels; i++) {
                                g_string_append_printf(*out, "%s%s",
                                        ctx->channels[i].label, ctx->value);
@@ -434,7 +451,6 @@ static void dump_saved_values(struct context *ctx, GString **out)
                        ctx->previous_sample = g_malloc0(analog_size + ctx->num_logic_channels);
 
                for (i = 0; i < ctx->num_samples; i++) {
-                       ctx->sample_time += ctx->period;
                        analog_sample =
                            &ctx->analog_samples[i * ctx->num_analog_channels];
                        logic_sample =
@@ -456,9 +472,16 @@ static void dump_saved_values(struct context *ctx, GString **out)
                                       analog_sample, analog_size);
                        }
 
-                       if (ctx->time)
+                       if (ctx->time && !ctx->sample_rate) {
+                               g_string_append_printf(*out, "0%s", ctx->value);
+                       } else if (ctx->time) {
+                               sample_time_dbl = ctx->out_sample_count++;
+                               sample_time_dbl /= ctx->sample_rate;
+                               sample_time_dbl *= ctx->sample_scale;
+                               sample_time_u64 = sample_time_dbl;
                                g_string_append_printf(*out, "%" PRIu64 "%s",
-                                       ctx->sample_time, ctx->value);
+                                       sample_time_u64, ctx->value);
+                       }
 
                        for (j = 0; j < num_channels; j++) {
                                if (ctx->channels[j].ch->type == SR_CHANNEL_ANALOG) {
@@ -551,10 +574,91 @@ static void save_gnuplot(struct context *ctx)
        g_string_free(script, TRUE);
 }
 
+static void check_input_constraints(struct context *ctx)
+{
+       size_t snum_count, snum_warn_limit;
+       size_t logic, analog;
+       gboolean has_frames, is_short, is_mixed, is_multi_analog;
+       gboolean do_warn;
+
+       /*
+        * Check and conditionally warn exactly once during execution
+        * of the output module on a set of input data.
+        */
+       if (ctx->have_checked)
+               return;
+       ctx->have_checked = TRUE;
+
+       /*
+        * This implementation of the CSV output module assumes some
+        * constraints which need not be met in reality. Emit warnings
+        * until a better version becomes available. Letting users know
+        * that their request may not get processed correctly is the
+        * only thing we can do for now except for complete refusal to
+        * process the input data.
+        *
+        * What the implementation appears to assume (unverified, this
+        * interpretation may be incorrect and/or incomplete):
+        * - Multi-channel analog data, or mixed signal input, always
+        *   is enclosed in frame markers.
+        * - Data which gets received across several packets spans a
+        *   consistent sample number range. All samples of one frame
+        *   and channel number or data type fit into a single packet.
+        *   Arbitrary chunking seems to not be supported.
+        * - A specific order of analog data packets is assumed.
+        *
+        * With these assumptions encoded in the implementation, and
+        * not being met at runtime, incorrect and unexpected results
+        * were seen for these configurations:
+        * - More than one analog channel.
+        * - The combination of logic and analog channel types.
+        *
+        * The condition of frames with large sample counts is a wild
+        * guess, the limit is a totally arbitrary choice. It assumes
+        * typical scope frames with at most a few thousand samples per
+        * frame, and assumes that a channel's data gets sent in large
+        * enough packets. The absence of a warning message does not
+        * necessarily translate to correct output, it's more of a rate
+        * limiting approach to not scare users too much.
+        */
+       snum_count = ctx->pkt_snums;
+       snum_warn_limit = 1 * 1000 * 1000;
+       logic = ctx->num_logic_channels;
+       analog = ctx->num_analog_channels;
+       has_frames = ctx->have_frames;
+       is_short = snum_count < snum_warn_limit;
+       is_mixed = logic && analog;
+       is_multi_analog = analog > 1;
+
+       if (has_frames && is_short) {
+               sr_info("Assuming consistent framed input data.");
+               return;
+       }
+
+       do_warn = FALSE;
+       if (has_frames) {
+               sr_warn("Untested configuration: large frame content.");
+               do_warn = TRUE;
+       }
+       if (is_mixed) {
+               sr_warn("Untested configuration: mixed signal input data.");
+               do_warn = TRUE;
+       }
+       if (is_multi_analog) {
+               sr_warn("Untested configuration: multi-channel analog data.");
+               do_warn = TRUE;
+       }
+       if (!do_warn)
+               return;
+       sr_warn("Resulting CSV output data may be incomplete or incorrect.");
+}
+
 static int receive(const struct sr_output *o,
                   const struct sr_datafeed_packet *packet, GString **out)
 {
        struct context *ctx;
+       const struct sr_datafeed_logic *logic;
+       const struct sr_datafeed_analog *analog;
 
        *out = NULL;
        if (!o || !o->sdi)
@@ -565,18 +669,30 @@ static int receive(const struct sr_output *o,
        sr_dbg("Got packet of type %d", packet->type);
        switch (packet->type) {
        case SR_DF_HEADER:
+               ctx->have_checked = FALSE;
+               ctx->have_frames = FALSE;
+               ctx->pkt_snums = FALSE;
                *out = gen_header(o, packet->payload);
                break;
        case SR_DF_TRIGGER:
                ctx->trigger = TRUE;
                break;
        case SR_DF_LOGIC:
-               process_logic(ctx, packet->payload);
+               logic = packet->payload;
+               ctx->pkt_snums = logic->length;
+               ctx->pkt_snums /= logic->length;
+               check_input_constraints(ctx);
+               process_logic(ctx, logic);
                break;
        case SR_DF_ANALOG:
-               process_analog(ctx, packet->payload);
+               analog = packet->payload;
+               ctx->pkt_snums = analog->num_samples;
+               ctx->pkt_snums /= g_slist_length(analog->meaning->channels);
+               check_input_constraints(ctx);
+               process_analog(ctx, analog);
                break;
        case SR_DF_FRAME_BEGIN:
+               ctx->have_frames = TRUE;
                *out = g_string_new(ctx->frame);
                /* Fallthrough */
        case SR_DF_END:
@@ -635,6 +751,8 @@ static struct sr_option options[] = {
 
 static const struct sr_option *get_options(void)
 {
+       GSList *l = NULL;
+
        if (!options[0].def) {
                options[0].def = g_variant_ref_sink(g_variant_new_string(""));
                options[1].def = g_variant_ref_sink(g_variant_new_boolean(TRUE));
@@ -644,9 +762,13 @@ static const struct sr_option *get_options(void)
                options[5].def = g_variant_ref_sink(g_variant_new_string(";"));
                options[6].def = g_variant_ref_sink(g_variant_new_boolean(TRUE));
                options[7].def = g_variant_ref_sink(g_variant_new_string("units"));
-               options[8].def = g_variant_ref_sink(g_variant_new_boolean(TRUE));
+               l = g_slist_append(l, g_variant_ref_sink(g_variant_new_string("units")));
+               l = g_slist_append(l, g_variant_ref_sink(g_variant_new_string("channel")));
+               l = g_slist_append(l, g_variant_ref_sink(g_variant_new_string("off")));
+               options[7].values = l;
+               options[8].def = g_variant_ref_sink(g_variant_new_boolean(FALSE));
                options[9].def = g_variant_ref_sink(g_variant_new_boolean(FALSE));
-               options[10].def = g_variant_ref_sink(g_variant_new_boolean(TRUE));
+               options[10].def = g_variant_ref_sink(g_variant_new_boolean(FALSE));
        }
 
        return options;