hantek-dso: eliminate the "forced trigger" option value

Commit c93f113879a0 introduced a hack on top of an unfortunate choice.
It seems counter intuitive to open code a default trigger source on an
arbitrary channel, and have users override it with the "forced" value
to _not_ use a trigger.

The normal approach would be to specify a trigger when a trigger should
be used, and to not specify a trigger when no trigger should be used.

Eliminate the "forced" choice in the trigger_sources[] list. Start with
a not-set value. Accept when a value is specified (a "genuine" choice).
Don't bother adding support to "un-set" the previously specified choice.

This leaves the intuitive support for trigger specs that shall be used,
and only uses triggers when they were specified. Does not provide means
to "un-set" an earlier specified trigger config as the implementation
did before the 2023-09 change, which would be the 2021-02 status.

If the approach implemented here should be considered undesirable, then
a "none" option value should be introduced, something that much better
communicates to users what's happening and what the consequence of a
config choice would be.

This commit also adjusts the "ugh" code path which differed from the
"capture empty" code path. Fixes a resource leak in the config set path.
The "set trigger source" and "set trigger and samplerate" commands don't
agree on the phrase for the trigger source in the wire format. But have
been doing that before the forced trigger introduction, behaviour
remains unchanged here.

diff --git a/src/hardware/hantek-dso/api.c b/src/hardware/hantek-dso/api.c
index d10c58d139ef3dcd7f8a2fd1c25e685bb703a218..bfad5f7135f07750901e04a9fda5eb4fe1a87fbd 100644 (file)
--- a/src/hardware/hantek-dso/api.c
+++ b/src/hardware/hantek-dso/api.c
@@ -168,7 +168,7 @@ static const uint64_t vdivs[][2] = {
 };
 
 static const char *trigger_sources[] = {
-       "CH1", "CH2", "EXT", "forced"
+       "CH1", "CH2", "EXT",
 };
 
 static const char *trigger_slopes[] = {
@@ -218,7 +218,7 @@ static struct sr_dev_inst *dso_dev_new(const struct dso_profile *prof)
        devc->voffset_trigger = DEFAULT_VERT_TRIGGERPOS;
        devc->framesize = DEFAULT_FRAMESIZE;
        devc->triggerslope = SLOPE_POSITIVE;
-       devc->triggersource = g_strdup(DEFAULT_TRIGGER_SOURCE);
+       devc->triggersource = NULL;
        devc->capture_ratio = DEFAULT_CAPTURE_RATIO;
        sdi->priv = devc;
 
@@ -465,6 +465,8 @@ static int config_get(uint32_t key, GVariant **data,
                        *data = g_variant_new_uint64(devc->framesize);
                        break;
                case SR_CONF_TRIGGER_SOURCE:
+                       if (!devc->triggersource)
+                               return SR_ERR_NA;
                        *data = g_variant_new_string(devc->triggersource);
                        break;
                case SR_CONF_TRIGGER_SLOPE:
@@ -555,6 +557,7 @@ static int config_set(uint32_t key, GVariant *data,
                case SR_CONF_TRIGGER_SOURCE:
                        if ((idx = std_str_idx(data, ARRAY_AND_SIZE(trigger_sources))) < 0)
                                return SR_ERR_ARG;
+                       g_free(devc->triggersource);
                        devc->triggersource = g_strdup(trigger_sources[idx]);
                        break;
                default:
@@ -836,8 +839,10 @@ static int handle_event(int fd, int revents, void *cb_data)
                        return TRUE;
                if (dso_enable_trigger(sdi) != SR_OK)
                        return TRUE;
-//             if (dso_force_trigger(sdi) != SR_OK)
-//                     return TRUE;
+               if (!devc->triggersource) {
+                       if (dso_force_trigger(sdi) != SR_OK)
+                               return TRUE;
+               }
                sr_dbg("Successfully requested next chunk.");
                devc->dev_state = CAPTURE;
                return TRUE;
@@ -858,7 +863,7 @@ static int handle_event(int fd, int revents, void *cb_data)
                                break;
                        if (dso_enable_trigger(sdi) != SR_OK)
                                break;
-                       if (!strcmp("forced", devc->triggersource)) {
+                       if (!devc->triggersource) {
                                if (dso_force_trigger(sdi) != SR_OK)
                                        break;
                        }

diff --git a/src/hardware/hantek-dso/protocol.c b/src/hardware/hantek-dso/protocol.c
index 408f61122a07dad0bd606c0bbddc4871ed11f168..34a3935b9fd4fa6e36f3ceab8197652f2629e625 100644 (file)
--- a/src/hardware/hantek-dso/protocol.c
+++ b/src/hardware/hantek-dso/protocol.c
@@ -275,12 +275,14 @@ static int dso2250_set_trigger_samplerate(const struct sr_dev_inst *sdi)
        memset(cmdstring, 0, sizeof(cmdstring));
        /* Command */
        cmdstring[0] = CMD_2250_SET_TRIGGERSOURCE;
-       sr_dbg("Trigger source %s.", devc->triggersource);
-       if (!strcmp("CH2", devc->triggersource))
+       sr_dbg("Trigger source %s.", devc->triggersource ? : "<none>");
+       if (!devc->triggersource)
+               tmp = 0;
+       else if (!strcmp("CH2", devc->triggersource))
                tmp = 3;
        else if (!strcmp("CH1", devc->triggersource))
                tmp = 2;
-       else if (!strcmp("EXT", devc->triggersource) || !strcmp("forced", devc->triggersource))
+       else if (!strcmp("EXT", devc->triggersource))
                tmp = 0;
        else {
                sr_err("Invalid trigger source: '%s'.", devc->triggersource);
@@ -421,12 +423,14 @@ SR_PRIV int dso_set_trigger_samplerate(const struct sr_dev_inst *sdi)
        cmdstring[0] = CMD_SET_TRIGGER_SAMPLERATE;
 
        /* Trigger source */
-       sr_dbg("Trigger source %s.", devc->triggersource);
-       if (!strcmp("CH2", devc->triggersource))
+       sr_dbg("Trigger source %s.", devc->triggersource ? : "<none>");
+       if (!devc->triggersource)
+               tmp = 2;
+       else if (!strcmp("CH2", devc->triggersource))
                tmp = 0;
        else if (!strcmp("CH1", devc->triggersource))
                tmp = 1;
-       else if (!strcmp("EXT", devc->triggersource) || !strcmp("forced", devc->triggersource))
+       else if (!strcmp("EXT", devc->triggersource))
                tmp = 2;
        else {
                sr_err("Invalid trigger source: '%s'.", devc->triggersource);
@@ -668,7 +672,7 @@ static int dso_set_relays(const struct sr_dev_inst *sdi)
        if (devc->coupling[1] != COUPLING_AC)
                relays[6] = ~relays[6];
 
-       if (!strcmp(devc->triggersource, "EXT"))
+       if (devc->triggersource && strcmp(devc->triggersource, "EXT") == 0)
                relays[7] = ~relays[7];
 
        if (sr_log_loglevel_get() >= SR_LOG_DBG) {