]> sigrok.org Git - libsigrok.git/blobdiff - src/output/csv.c
output/csv: use intermediate time_t var, silence compiler warning
[libsigrok.git] / src / output / csv.c
index 296d9a06b0ab541037b588b63a42f7d62b13a619..c4801ef7f984bca96b432fda2eb37c55fd9da730 100644 (file)
@@ -101,6 +101,11 @@ struct context {
        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;
 };
 
 /*
@@ -247,11 +252,13 @@ static GString *gen_header(const struct sr_output *o,
        /* Some metadata */
        if (ctx->header && !ctx->did_header) {
                /* save_gnuplot knows how many lines we print. */
+               time_t secs;
+               secs = hdr->starttime.tv_sec;
                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_get(), ctx->comment,
-                       ctx->title, ctime(&hdr->starttime.tv_sec));
+                       ctx->title, ctime(&secs));
 
                /* Columns / channels */
                channels = o->sdi ? o->sdi->channels : NULL;
@@ -415,7 +422,8 @@ static void dump_saved_values(struct context *ctx, GString **out)
        } else {
                sr_info("Dumping %u samples", ctx->num_samples);
 
-               *out = g_string_sized_new(512);
+               if (!*out)
+                       *out = g_string_sized_new(512);
                num_channels =
                    ctx->num_logic_channels + ctx->num_analog_channels;
 
@@ -569,10 +577,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)
@@ -583,18 +672,32 @@ 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);
+               *out = g_string_sized_new(512);
+               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);
+               *out = g_string_sized_new(512);
+               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: