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 d0d40b6f25089efb35b42e4464131a0bdb65afcc..3962fe9d80215fcde193e5556d78c3ab70f3ded1 100644 (file)
--- 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 768a985ed84efe134cfedd091cd2810f3fb633a7..379f019be4ed038db70e514afecc8890d379ae3b 100644 (file)
--- 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 8fcfcea456357c149fa80967356ef772b611383e..89111446c65001c503329796802582cf40be0d8e 100644 (file)
--- 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;
                                        }