From: Gerhard Sittig Date: Mon, 31 Jan 2022 17:41:55 +0000 (+0100) Subject: kingst-la2016: implement alternative simpler threshold voltage config X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=9270f8f437581605ab4e3e808e2bd62f41ab7d35 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 2d90a9f9..0e8f9fe0 100644 --- 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 dc1a24a5..86c90ea9 100644 --- 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 c865a13b..4e16b372 100644 --- 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);