From: Gerhard Sittig Date: Mon, 27 Nov 2023 18:12:11 +0000 (+0100) Subject: hantek-dso: eliminate the "forced trigger" option value X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=5f50d6da1bc5cc5b48fdc6d4f1775313e97fb1d2;p=libsigrok.git 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 d10c58d1..bfad5f71 100644 --- 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 408f6112..34a3935b 100644 --- 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 ? : ""); + 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 ? : ""); + 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) {