From: Gerhard Sittig <redacted>
Date: Wed, 27 Jun 2018 21:57:45 +0000 (+0200)
Subject: output: fixup trigger marker position in ascii/bits/hex output modules
X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=67b345b981a581731435f490dfbf7b13d8b1d5a9;p=libsigrok.git

output: fixup trigger marker position in ascii/bits/hex output modules

Adjust the calculation of the '^' marker's position in T: lines of the
-O ascii/bits/hex output modules such that it matches the sample data
lines' layout. Add comments which discuss the motivation of the marker
position's calculation, which differs among each of those modules.

Strictly speaking -O bits was already correct. But I chose to adjust and
comment the logic such that multiple output modules follow a common
pattern. If performance is an issue, the bits.c change might be worth
reverting.

This commit fixes bug #1238.
---

diff --git a/src/output/ascii.c b/src/output/ascii.c
index d0d40b6f..3962fe9d 100644
--- a/src/output/ascii.c
+++ b/src/output/ascii.c
@@ -199,7 +199,13 @@ static int receive(const struct sr_output *o, const struct sr_datafeed_packet *p
 					g_string_append_len(*out, ctx->lines[j]->str, ctx->lines[j]->len);
 					g_string_append_c(*out, '\n');
 					if (j == ctx->num_enabled_channels - 1 && ctx->trigger > -1) {
-						offset = ctx->trigger + ctx->trigger / 8;
+						/*
+						 * Each group of 8 bits occupies 8 bit positions
+						 * and no separator. With this dense presentation
+						 * the "calculation" of the trigger position is
+						 * rather straight forward.
+						 */
+						offset = ctx->trigger;
 						g_string_append_printf(*out, "T:%*s^ %d\n", offset, "", ctx->trigger);
 						ctx->trigger = -1;
 					}
diff --git a/src/output/bits.c b/src/output/bits.c
index 768a985e..379f019b 100644
--- a/src/output/bits.c
+++ b/src/output/bits.c
@@ -168,7 +168,14 @@ static int receive(const struct sr_output *o, const struct sr_datafeed_packet *p
 					g_string_append_len(*out, ctx->lines[j]->str, ctx->lines[j]->len);
 					g_string_append_c(*out, '\n');
 					if (j == ctx->num_enabled_channels - 1 && ctx->trigger > -1) {
-						offset = ctx->trigger + ctx->trigger / 8;
+						/*
+						 * Each group of 8 bits occupies 8 bit positions
+						 * plus 1 separator. Calculate the position of the
+						 * byte which contains the trigger, then adjust for
+						 * the trigger's bit position within that byte.
+						 */
+						offset = ctx->trigger / 8 * (8 + 1);
+						offset += ctx->trigger % 8;
 						g_string_append_printf(*out, "T:%*s^ %d\n", offset, "", ctx->trigger);
 						ctx->trigger = -1;
 					}
diff --git a/src/output/hex.c b/src/output/hex.c
index 8fcfcea4..89111446 100644
--- a/src/output/hex.c
+++ b/src/output/hex.c
@@ -181,7 +181,14 @@ static int receive(const struct sr_output *o, const struct sr_datafeed_packet *p
 					g_string_append_len(*out, ctx->lines[j]->str, ctx->lines[j]->len);
 					g_string_append_c(*out, '\n');
 					if (j == ctx->num_enabled_channels - 1 && ctx->trigger > -1) {
-						offset = ctx->trigger + ctx->trigger / 8;
+						/*
+						 * Each group of 8 bits occupies 2 hex digits plus
+						 * 1 separator. Calculate the position of the byte
+						 * which contains the trigger, then adjust for the
+						 * trigger's bit position within that byte.
+						 */
+						offset = ctx->trigger / 8 * (2 + 1);
+						offset += (ctx->trigger % 8) / 4;
 						g_string_append_printf(*out, "T:%*s^ %d\n", offset, "", ctx->trigger);
 						ctx->trigger = -1;
 					}