kingst-la2016: implement alternative simpler threshold voltage config

The complex setup with three config keys that heavily interact with each
other did not work from the user's perspective. It's unfortunate how the
configuration of one item changed another item's value. The GUI would
not provide labels which reflect the currently configured voltage value,
the list of presets was never presented in the GUI.

Implement something that is both simpler and works with sigrok-cli as
well as pulseview. Reduce the set of choices and only provide a discrete
list of voltages. Yet try to cover most logic families and typical use
cases. This alternative approach exists in addition to the previously
implemented approach, and is selected by compile time switches. Ideally
the logic input threshold voltage would be a property of the "Logic"
channel group, but the GUI then won't display the option. That's why it
currently remains a global device option.

Pass the user specified voltage as a value from api.c to protocol.c
which eliminates intimate knowledge of the config API's internal details
(especially with the compile time options in api.c).

diff --git a/src/hardware/kingst-la2016/api.c b/src/hardware/kingst-la2016/api.c
index 2d90a9f9caf6029b38d7b1ff87ce069a50761374..0e8f9fe03c6378e4b6bcd51bbaf7fb37a7e7e003 100644 (file)
--- a/src/hardware/kingst-la2016/api.c
+++ b/src/hardware/kingst-la2016/api.c
@@ -48,13 +48,23 @@ static const uint32_t devopts[] = {
        SR_CONF_SAMPLERATE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
        SR_CONF_LIMIT_SAMPLES | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
        SR_CONF_LIMIT_MSEC | SR_CONF_GET | SR_CONF_SET,
+#if WITH_THRESHOLD_DEVCFG
        SR_CONF_VOLTAGE_THRESHOLD | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
+#if !WITH_THRESHOLD_SIMPLE
        SR_CONF_LOGIC_THRESHOLD | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
        SR_CONF_LOGIC_THRESHOLD_CUSTOM | SR_CONF_GET | SR_CONF_SET,
+#endif
+#endif
        SR_CONF_TRIGGER_MATCH | SR_CONF_LIST,
        SR_CONF_CAPTURE_RATIO | SR_CONF_GET | SR_CONF_SET,
 };
 
+static const uint32_t devopts_cg_logic[] = {
+#if !WITH_THRESHOLD_DEVCFG
+       SR_CONF_VOLTAGE_THRESHOLD | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
+#endif
+};
+
 static const uint32_t devopts_cg_pwm[] = {
        SR_CONF_ENABLED | SR_CONF_GET | SR_CONF_SET,
        SR_CONF_OUTPUT_FREQUENCY | SR_CONF_GET | SR_CONF_SET,
@@ -143,6 +153,43 @@ static const uint64_t rates_100mhz[] = {
        SR_MHZ(100),
 };
 
+#if WITH_THRESHOLD_SIMPLE
+
+/*
+ * Only list a few discrete voltages, to form a useful set which covers
+ * most logic families. Too many choices can make some applications use
+ * a slider again. Which may lack a scale for the current value, and
+ * leave users without feedback what the currently used value might be.
+ */
+static const double threshold_ranges[][2] = {
+       { 0.4, 0.4, },
+       { 0.6, 0.6, },
+       { 0.9, 0.9, },
+       { 1.2, 1.2, },
+       { 1.4, 1.4, }, /* Default, 1.4V, index 4. */
+       { 2.0, 2.0, },
+       { 2.5, 2.5, },
+       { 4.0, 4.0, },
+};
+#define LOGIC_THRESHOLD_IDX_DFLT       4
+
+static double threshold_voltage(const struct sr_dev_inst *sdi, double *high)
+{
+       struct dev_context *devc;
+       size_t idx;
+       double voltage;
+
+       devc = sdi->priv;
+       idx = devc->threshold_voltage_idx;
+       voltage = threshold_ranges[idx][0];
+       if (high)
+               *high = threshold_ranges[idx][1];
+
+       return voltage;
+}
+
+#else /* WITH_THRESHOLD_SIMPLE */
+
 static const float logic_threshold_value[] = {
        1.58,
        2.5,
@@ -170,6 +217,26 @@ static const char *logic_threshold[] = {
 
 #define LOGIC_THRESHOLD_IDX_USER       (ARRAY_SIZE(logic_threshold) - 1)
 
+static double threshold_voltage(const struct sr_dev_inst *sdi, double *high)
+{
+       struct dev_context *devc;
+       size_t idx;
+       double voltage;
+
+       devc = sdi->priv;
+       idx = devc->threshold_voltage_idx;
+       if (idx == LOGIC_THRESHOLD_IDX_USER)
+               voltage = devc->threshold_voltage;
+       else
+               voltage = logic_threshold_value[idx];
+       if (high)
+               *high = voltage;
+
+       return voltage;
+}
+
+#endif /* WITH_THRESHOLD_SIMPLE */
+
 /* Convenience. Release an allocated devc from error paths. */
 static void kingst_la2016_free_devc(struct dev_context *devc)
 {
@@ -606,8 +673,12 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
                devc->sw_limits.limit_samples = 0;
                devc->capture_ratio = 50;
                devc->cur_samplerate = devc->model->samplerate;
+#if WITH_THRESHOLD_SIMPLE
+               devc->threshold_voltage_idx = LOGIC_THRESHOLD_IDX_DFLT;
+#else /* WITH_THRESHOLD_SIMPLE */
                devc->threshold_voltage_idx = 0;
                devc->threshold_voltage = logic_threshold_value[devc->threshold_voltage_idx];
+#endif /* WITH_THRESHOLD_SIMPLE */
                if  (ARRAY_SIZE(devc->pwm_setting) >= 1) {
                        devc->pwm_setting[0].enabled = FALSE;
                        devc->pwm_setting[0].freq = SR_KHZ(1);
@@ -731,9 +802,12 @@ static int config_get(uint32_t key, GVariant **data,
        size_t logic_idx, analog_idx;
        struct pwm_setting *pwm;
        struct sr_usb_dev_inst *usb;
-       double rounded;
+       double voltage, rounded;
        const char *label;
 
+       (void)rounded;
+       (void)voltage;
+
        if (!sdi)
                return SR_ERR_ARG;
        devc = sdi->priv;
@@ -745,8 +819,19 @@ static int config_get(uint32_t key, GVariant **data,
 
        /* Handle requests for the "Logic" channel group. */
        if (cg && cg_type == SR_CHANNEL_LOGIC) {
-               /* TODO */
-               return SR_ERR_NA;
+               switch (key) {
+#if !WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+               case SR_CONF_VOLTAGE_THRESHOLD:
+                       voltage = threshold_voltage(sdi, NULL);
+                       *data = std_gvar_tuple_double(voltage, voltage);
+                       break;
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
+               default:
+                       return SR_ERR_NA;
+               }
+               return SR_OK;
        }
 
        /* Handle requests for the "PWMx" channel groups. */
@@ -782,10 +867,17 @@ static int config_get(uint32_t key, GVariant **data,
        case SR_CONF_CAPTURE_RATIO:
                *data = g_variant_new_uint64(devc->capture_ratio);
                break;
+#if WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+       case SR_CONF_VOLTAGE_THRESHOLD:
+               voltage = threshold_voltage(sdi, NULL);
+               *data = std_gvar_tuple_double(voltage, voltage);
+               break;
+#else /* WITH_THRESHOLD_SIMPLE */
        case SR_CONF_VOLTAGE_THRESHOLD:
                rounded = (int)(devc->threshold_voltage / 0.1) * 0.1;
                *data = std_gvar_tuple_double(rounded, rounded + 0.1);
-               return SR_OK;
+               break;
        case SR_CONF_LOGIC_THRESHOLD:
                label = logic_threshold[devc->threshold_voltage_idx];
                *data = g_variant_new_string(label);
@@ -793,7 +885,8 @@ static int config_get(uint32_t key, GVariant **data,
        case SR_CONF_LOGIC_THRESHOLD_CUSTOM:
                *data = g_variant_new_double(devc->threshold_voltage);
                break;
-
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
        default:
                return SR_ERR_NA;
        }
@@ -809,7 +902,7 @@ static int config_set(uint32_t key, GVariant *data,
        size_t logic_idx, analog_idx;
        struct pwm_setting *pwm;
        double value_f;
-       double low, high;
+       double low, high, voltage;
        int idx;
 
        devc = sdi->priv;
@@ -821,8 +914,22 @@ static int config_set(uint32_t key, GVariant *data,
 
        /* Handle requests for the "Logic" channel group. */
        if (cg && cg_type == SR_CHANNEL_LOGIC) {
-               /* TODO */
-               return SR_ERR_NA;
+               switch (key) {
+#if !WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+               case SR_CONF_LOGIC_THRESHOLD:
+                       idx = std_double_tuple_idx(data,
+                               ARRAY_AND_SIZE(threshold_ranges));
+                       if (idx < 0)
+                               return SR_ERR_ARG;
+                       devc->threshold_voltage_idx = idx;
+                       break;
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
+               default:
+                       return SR_ERR_NA;
+               }
+               return SR_OK;
        }
 
        /* Handle requests for the "PWMx" channel groups. */
@@ -869,6 +976,16 @@ static int config_set(uint32_t key, GVariant *data,
        case SR_CONF_CAPTURE_RATIO:
                devc->capture_ratio = g_variant_get_uint64(data);
                break;
+#if WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+       case SR_CONF_VOLTAGE_THRESHOLD:
+               idx = std_double_tuple_idx(data,
+                       ARRAY_AND_SIZE(threshold_ranges));
+               if (idx < 0)
+                       return SR_ERR_ARG;
+               devc->threshold_voltage_idx = idx;
+               break;
+#else /* WITH_THRESHOLD_SIMPLE */
        case SR_CONF_VOLTAGE_THRESHOLD:
                g_variant_get(data, "(dd)", &low, &high);
                devc->threshold_voltage = (low + high) / 2.0;
@@ -887,6 +1004,8 @@ static int config_set(uint32_t key, GVariant *data,
        case SR_CONF_LOGIC_THRESHOLD_CUSTOM:
                devc->threshold_voltage = g_variant_get_double(data);
                break;
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
        default:
                return SR_ERR_NA;
        }
@@ -910,8 +1029,25 @@ static int config_list(uint32_t key, GVariant **data,
 
        /* Handle requests for the "Logic" channel group. */
        if (cg && cg_type == SR_CHANNEL_LOGIC) {
-               /* TODO */
-               return SR_ERR_NA;
+               switch (key) {
+               case SR_CONF_DEVICE_OPTIONS:
+                       if (ARRAY_SIZE(devopts_cg_logic) == 0)
+                               return SR_ERR_NA;
+                       *data = g_variant_new_fixed_array(G_VARIANT_TYPE_UINT32,
+                               devopts_cg_logic, ARRAY_SIZE(devopts_cg_logic),
+                               sizeof(devopts_cg_logic[0]));
+                       break;
+#if !WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+               case SR_CONF_VOLTAGE_THRESHOLD:
+                       *data = std_gvar_thresholds(ARRAY_AND_SIZE(threshold_ranges));
+                       break;
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
+               default:
+                       return SR_ERR_NA;
+               }
+               return SR_OK;
        }
 
        /* Handle requests for the "PWMx" channel groups. */
@@ -946,17 +1082,27 @@ static int config_list(uint32_t key, GVariant **data,
        case SR_CONF_LIMIT_SAMPLES:
                *data = std_gvar_tuple_u64(0, LA2016_NUM_SAMPLES_MAX);
                break;
+#if WITH_THRESHOLD_DEVCFG
+#if WITH_THRESHOLD_SIMPLE
+       case SR_CONF_VOLTAGE_THRESHOLD:
+               *data = std_gvar_thresholds(ARRAY_AND_SIZE(threshold_ranges));
+               break;
+#else /* WITH_THRESHOLD_SIMPLE */
        case SR_CONF_VOLTAGE_THRESHOLD:
                *data = std_gvar_min_max_step_thresholds(
                        LA2016_THR_VOLTAGE_MIN,
                        LA2016_THR_VOLTAGE_MAX, 0.1);
                break;
+#endif /* WITH_THRESHOLD_SIMPLE */
+#endif /* WITH_THRESHOLD_DEVCFG */
        case SR_CONF_TRIGGER_MATCH:
                *data = std_gvar_array_i32(ARRAY_AND_SIZE(trigger_matches));
                break;
+#if WITH_THRESHOLD_DEVCFG && !WITH_THRESHOLD_SIMPLE
        case SR_CONF_LOGIC_THRESHOLD:
                *data = g_variant_new_strv(ARRAY_AND_SIZE(logic_threshold));
                break;
+#endif
        default:
                return SR_ERR_NA;
        }
@@ -970,6 +1116,7 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
        struct drv_context *drvc;
        struct sr_context *ctx;
        struct dev_context *devc;
+       double voltage;
        int ret;
 
        di = sdi->driver;
@@ -988,7 +1135,8 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 
        sr_sw_limits_acquisition_start(&devc->sw_limits);
 
-       ret = la2016_setup_acquisition(sdi);
+       voltage = threshold_voltage(sdi, NULL);
+       ret = la2016_setup_acquisition(sdi, voltage);
        if (ret != SR_OK) {
                feed_queue_logic_free(devc->feed_queue);
                devc->feed_queue = NULL;

diff --git a/src/hardware/kingst-la2016/protocol.c b/src/hardware/kingst-la2016/protocol.c
index dc1a24a5193b6f0a96933e9f15443480b0eb2c5b..86c90ea99ad164e077218a4b1485cde38be05948 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.c
+++ b/src/hardware/kingst-la2016/protocol.c
@@ -335,14 +335,11 @@ static int enable_fpga_bitstream(const struct sr_dev_inst *sdi)
 
 static int set_threshold_voltage(const struct sr_dev_inst *sdi, float voltage)
 {
-       struct dev_context *devc;
        int ret;
        uint16_t duty_R79, duty_R56;
        uint8_t buf[2 * sizeof(uint16_t)];
        uint8_t *wrptr;
 
-       devc = sdi->priv;
-
        /* Clamp threshold setting to valid range for LA2016. */
        if (voltage > LA2016_THR_VOLTAGE_MAX) {
                voltage = LA2016_THR_VOLTAGE_MAX;
@@ -391,7 +388,6 @@ static int set_threshold_voltage(const struct sr_dev_inst *sdi, float voltage)
                sr_err("Cannot set threshold voltage %.2fV.", voltage);
                return ret;
        }
-       devc->threshold_voltage = voltage;
 
        return SR_OK;
 }
@@ -893,15 +889,13 @@ SR_PRIV int la2016_upload_firmware(const struct sr_dev_inst *sdi,
        return SR_OK;
 }
 
-SR_PRIV int la2016_setup_acquisition(const struct sr_dev_inst *sdi)
+SR_PRIV int la2016_setup_acquisition(const struct sr_dev_inst *sdi,
+       double voltage)
 {
-       struct dev_context *devc;
        int ret;
        uint8_t cmd;
 
-       devc = sdi->priv;
-
-       ret = set_threshold_voltage(sdi, devc->threshold_voltage);
+       ret = set_threshold_voltage(sdi, voltage);
        if (ret != SR_OK)
                return ret;
 

diff --git a/src/hardware/kingst-la2016/protocol.h b/src/hardware/kingst-la2016/protocol.h
index c865a13b9acfa6b0c412e6c4c5f6f79f5d09f0fd..4e16b37227736110cc6b76467c19060d5d6147f1 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.h
+++ b/src/hardware/kingst-la2016/protocol.h
@@ -66,6 +66,25 @@
  */
 #define LA2016_EP2_PADDING     2048
 
+/*
+ * The complex logic input threshold voltage support with a custom level
+ * is not operational yet. Ideally we could support the set of pre-made
+ * voltages with their pretty text labels, and one of them referencing
+ * a voltage which is user specified. But not all applications support
+ * this setup equally well, or present it most appropriately to users.
+ * So let's implement something simpler for the moment until the more
+ * complex approach becomes accessible in all applications.
+ *
+ * Strictly speaking the logic input threshold voltage is a property of
+ * the "Logic" channel group. Again not all applications support such
+ * an approach, and like to see them as global device properties.
+ */
+#define WITH_THRESHOLD_DEVCFG  1
+#define WITH_THRESHOLD_SIMPLE  1
+#if !WITH_THRESHOLD_DEVCFG && !WITH_THRESHOLD_SIMPLE
+#  error "Custom threshold in Logic group is not implemented."
+#endif
+
 #define LA2016_THR_VOLTAGE_MIN 0.40
 #define LA2016_THR_VOLTAGE_MAX 4.00
 
@@ -136,7 +155,8 @@ SR_PRIV int la2016_identify_device(const struct sr_dev_inst *sdi,
 SR_PRIV int la2016_init_hardware(const struct sr_dev_inst *sdi);
 SR_PRIV int la2016_deinit_hardware(const struct sr_dev_inst *sdi);
 SR_PRIV int la2016_write_pwm_config(const struct sr_dev_inst *sdi, size_t idx);
-SR_PRIV int la2016_setup_acquisition(const struct sr_dev_inst *sdi);
+SR_PRIV int la2016_setup_acquisition(const struct sr_dev_inst *sdi,
+       double voltage);
 SR_PRIV int la2016_start_acquisition(const struct sr_dev_inst *sdi);
 SR_PRIV int la2016_abort_acquisition(const struct sr_dev_inst *sdi);
 SR_PRIV int la2016_receive_data(int fd, int revents, void *cb_data);