X-Git-Url: https://sigrok.org/gitweb/?a=blobdiff_plain;f=src%2Foutput%2Fcsv.c;h=848595f9d3096b9570fcd0e5505e2d1026ea1250;hb=084d6b927163fb0881941ad13014e07a6a5e7a77;hp=296d9a06b0ab541037b588b63a42f7d62b13a619;hpb=4feb6ec9a2c818cb7cad508ee4284ce2386ccd41;p=libsigrok.git

diff --git a/src/output/csv.c b/src/output/csv.c
index 296d9a06..848595f9 100644
--- a/src/output/csv.c
+++ b/src/output/csv.c
@@ -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;
 };
 
 /*
@@ -415,7 +420,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 +575,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 +670,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: