korad-kaxxxxp: Synchronize read and write operations.

src/hardware/korad-kaxxxxp/api.c

@@ -2,6 +2,7 @@
  * This file is part of the libsigrok project.
  *
  * Copyright (C) 2015 Hannu Vuolasaho <vuokkosetae@gmail.com>
+ * Copyright (C) 2018 Frank Stettner <frank-stettner@gmx.net>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -139,8 +140,8 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
 
 	devc = g_malloc0(sizeof(struct dev_context));
 	sr_sw_limits_init(&devc->limits);
+	g_mutex_init(&devc->rw_mutex);
 	devc->model = &models[model_id];
-	devc->reply[5] = 0;
 	devc->req_sent_at = 0;
 	sdi->priv = devc;
 
@@ -177,28 +178,36 @@ static int config_get(uint32_t key, GVariant **data,
 	case SR_CONF_LIMIT_MSEC:
 		return sr_sw_limits_config_get(&devc->limits, key, data);
 	case SR_CONF_VOLTAGE:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_VOLTAGE, devc);
 		*data = g_variant_new_double(devc->voltage);
 		break;
 	case SR_CONF_VOLTAGE_TARGET:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_VOLTAGE_MAX, devc);
 		*data = g_variant_new_double(devc->voltage_max);
 		break;
 	case SR_CONF_CURRENT:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_CURRENT, devc);
 		*data = g_variant_new_double(devc->current);
 		break;
 	case SR_CONF_CURRENT_LIMIT:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_CURRENT_MAX, devc);
 		*data = g_variant_new_double(devc->current_max);
 		break;
 	case SR_CONF_ENABLED:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_OUTPUT, devc);
 		*data = g_variant_new_boolean(devc->output_enabled);
 		break;
 	case SR_CONF_REGULATION:
 		/* Dual channel not supported. */
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_STATUS, devc);
 		*data = g_variant_new_string((devc->cc_mode[0]) ? "CC" : "CV");
 		break;
 	case SR_CONF_OVER_CURRENT_PROTECTION_ENABLED:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_OCP, devc);
 		*data = g_variant_new_boolean(devc->ocp_enabled);
 		break;
 	case SR_CONF_OVER_VOLTAGE_PROTECTION_ENABLED:
+		korad_kaxxxxp_get_value(sdi->conn, KAXXXXP_OVP, devc);
 		*data = g_variant_new_boolean(devc->ovp_enabled);
 		break;
 	default:
@@ -228,8 +237,7 @@ static int config_set(uint32_t key, GVariant *data,
 		if (dval < devc->model->voltage[0] || dval > devc->model->voltage[1])
 			return SR_ERR_ARG;
 		devc->voltage_max = dval;
-		devc->target = KAXXXXP_VOLTAGE_MAX;
+		if (korad_kaxxxxp_set_value(sdi->conn, KAXXXXP_VOLTAGE_MAX, devc) < 0)
-		if (korad_kaxxxxp_set_value(sdi->conn, devc) < 0)
 			return SR_ERR;
 		break;
 	case SR_CONF_CURRENT_LIMIT:
@@ -237,30 +245,26 @@ static int config_set(uint32_t key, GVariant *data,
 		if (dval < devc->model->current[0] || dval > devc->model->current[1])
 			return SR_ERR_ARG;
 		devc->current_max = dval;
-		devc->target = KAXXXXP_CURRENT_MAX;
+		if (korad_kaxxxxp_set_value(sdi->conn, KAXXXXP_CURRENT_MAX, devc) < 0)
-		if (korad_kaxxxxp_set_value(sdi->conn, devc) < 0)
 			return SR_ERR;
 		break;
 	case SR_CONF_ENABLED:
 		bval = g_variant_get_boolean(data);
 		/* Set always so it is possible turn off with sigrok-cli. */
 		devc->output_enabled = bval;
-		devc->target = KAXXXXP_OUTPUT;
+		if (korad_kaxxxxp_set_value(sdi->conn, KAXXXXP_OUTPUT, devc) < 0)
-		if (korad_kaxxxxp_set_value(sdi->conn, devc) < 0)
 			return SR_ERR;
 		break;
 	case SR_CONF_OVER_CURRENT_PROTECTION_ENABLED:
 		bval = g_variant_get_boolean(data);
 		devc->ocp_enabled = bval;
-		devc->target = KAXXXXP_OCP;
+		if (korad_kaxxxxp_set_value(sdi->conn, KAXXXXP_OCP, devc) < 0)
-		if (korad_kaxxxxp_set_value(sdi->conn, devc) < 0)
 			return SR_ERR;
 		break;
 	case SR_CONF_OVER_VOLTAGE_PROTECTION_ENABLED:
 		bval = g_variant_get_boolean(data);
 		devc->ovp_enabled = bval;
-		devc->target = KAXXXXP_OVP;
+		if (korad_kaxxxxp_set_value(sdi->conn, KAXXXXP_OVP, devc) < 0)
-		if (korad_kaxxxxp_set_value(sdi->conn, devc) < 0)
 			return SR_ERR;
 		break;
 	default:
@@ -298,6 +302,17 @@ static int config_list(uint32_t key, GVariant **data,
 	return SR_OK;
 }
 
+static int dev_close(struct sr_dev_inst *sdi)
+{
+	struct dev_context *devc;
+
+	devc = (sdi) ? sdi->priv : NULL;
+	if (devc)
+		g_mutex_clear(&devc->rw_mutex);
+
+	return std_serial_dev_close(sdi);
+}
+
 static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 {
 	struct dev_context *devc;
@@ -308,7 +323,6 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 	sr_sw_limits_acquisition_start(&devc->limits);
 	std_session_send_df_header(sdi);
 
-	devc->reply_pending = FALSE;
 	devc->req_sent_at = 0;
 	serial = sdi->conn;
 	serial_source_add(sdi->session, serial, G_IO_IN,
@@ -331,7 +345,7 @@ static struct sr_dev_driver korad_kaxxxxp_driver_info = {
 	.config_set = config_set,
 	.config_list = config_list,
 	.dev_open = std_serial_dev_open,
-	.dev_close = std_serial_dev_close,
+	.dev_close = dev_close,
 	.dev_acquisition_start = dev_acquisition_start,
 	.dev_acquisition_stop = std_serial_dev_acquisition_stop,
 	.context = NULL,

src/hardware/korad-kaxxxxp/protocol.c

@@ -2,6 +2,7 @@
  * This file is part of the libsigrok project.
  *
  * Copyright (C) 2015 Hannu Vuolasaho <vuokkosetae@gmail.com>
+ * Copyright (C) 2018 Frank Stettner <frank-stettner@gmx.net>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -78,21 +79,23 @@ static void give_device_time_to_process(struct dev_context *devc)
 }
 
 SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
-				struct dev_context *devc)
+				int target, struct dev_context *devc)
 {
 	char msg[21];
 	const char *cmd;
 	float value;
 	int ret;
 
+	g_mutex_lock(&devc->rw_mutex);
 	give_device_time_to_process(devc);
 
 	msg[20] = 0;
-	switch (devc->target) {
+	switch (target) {
 	case KAXXXXP_CURRENT:
 	case KAXXXXP_VOLTAGE:
 	case KAXXXXP_STATUS:
-		sr_err("Can't set measurable parameter.");
+		sr_err("Can't set measurable parameter %d.", target);
+		g_mutex_unlock(&devc->rw_mutex);
 		return SR_ERR;
 	case KAXXXXP_CURRENT_MAX:
 		cmd = "ISET1:%05.3f";
@@ -123,6 +126,7 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
 		if (devc->program < 1 || devc->program > 5) {
 			sr_err("Only programs 1-5 supported and %d isn't "
 			       "between them.", devc->program);
+			g_mutex_unlock(&devc->rw_mutex);
 			return SR_ERR;
 		}
 		value = devc->program;
@@ -132,12 +136,14 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
 		if (devc->program < 1 || devc->program > 5) {
 			sr_err("Only programs 1-5 supported and %d isn't "
 			       "between them.", devc->program);
+			g_mutex_unlock(&devc->rw_mutex);
 			return SR_ERR;
 		}
 		value = devc->program;
 		break;
 	default:
-		sr_err("Don't know how to set %d.", devc->target);
+		sr_err("Don't know how to set %d.", target);
+		g_mutex_unlock(&devc->rw_mutex);
 		return SR_ERR;
 	}
 
@@ -146,116 +152,76 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
 
 	ret = korad_kaxxxxp_send_cmd(serial, msg);
 	devc->req_sent_at = g_get_monotonic_time();
-	devc->reply_pending = FALSE;
+
+	g_mutex_unlock(&devc->rw_mutex);
 
 	return ret;
 }
 
-SR_PRIV int korad_kaxxxxp_query_value(struct sr_serial_dev_inst *serial,
+SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial,
-				struct dev_context *devc)
+				int target, struct dev_context *devc)
 {
-	int ret;
+	int ret, count;
+	char reply[6];
+	float *value;
+	char status_byte;
 
+	g_mutex_lock(&devc->rw_mutex);
 	give_device_time_to_process(devc);
 
-	switch (devc->target) {
+	value = NULL;
+	count = 5;
+
+	switch (target) {
 	case KAXXXXP_CURRENT:
 		/* Read current from device. */
 		ret = korad_kaxxxxp_send_cmd(serial, "IOUT1?");
+		value = &(devc->current);
 		break;
 	case KAXXXXP_CURRENT_MAX:
 		/* Read set current from device. */
 		ret = korad_kaxxxxp_send_cmd(serial, "ISET1?");
+		value = &(devc->current_max);
 		break;
 	case KAXXXXP_VOLTAGE:
 		/* Read voltage from device. */
 		ret = korad_kaxxxxp_send_cmd(serial, "VOUT1?");
+		value = &(devc->voltage);
 		break;
 	case KAXXXXP_VOLTAGE_MAX:
 		/* Read set voltage from device. */
 		ret = korad_kaxxxxp_send_cmd(serial, "VSET1?");
+		value = &(devc->voltage_max);
 		break;
 	case KAXXXXP_STATUS:
 	case KAXXXXP_OUTPUT:
+	case KAXXXXP_OCP:
+	case KAXXXXP_OVP:
 		/* Read status from device. */
 		ret = korad_kaxxxxp_send_cmd(serial, "STATUS?");
+		count = 1;
 		break;
 	default:
-		sr_err("Don't know how to query %d.", devc->target);
+		sr_err("Don't know how to query %d.", target);
+		g_mutex_unlock(&devc->rw_mutex);
 		return SR_ERR;
 	}
 
 	devc->req_sent_at = g_get_monotonic_time();
-	devc->reply_pending = TRUE;
 
-	return ret;
+	if ((ret = korad_kaxxxxp_read_chars(serial, count, reply)) < 0) {
-}
+		g_mutex_unlock(&devc->rw_mutex);
-
-SR_PRIV int korad_kaxxxxp_get_all_values(struct sr_serial_dev_inst *serial,
-				struct dev_context *devc)
-{
-	int ret;
-
-	for (devc->target = KAXXXXP_CURRENT;
-			devc->target <= KAXXXXP_STATUS; devc->target++) {
-		if ((ret = korad_kaxxxxp_query_value(serial, devc)) < 0)
-			return ret;
-		if ((ret = korad_kaxxxxp_get_reply(serial, devc)) < 0)
-			return ret;
-	}
-
-	return ret;
-}
-
-SR_PRIV int korad_kaxxxxp_get_reply(struct sr_serial_dev_inst *serial,
-				struct dev_context *devc)
-{
-	double value;
-	int count, ret;
-	float *target;
-	char status_byte;
-
-	target = NULL;
-	count = 5;
-
-	switch (devc->target) {
-	case KAXXXXP_CURRENT:
-		/* Read current from device. */
-		target = &(devc->current);
-		break;
-	case KAXXXXP_CURRENT_MAX:
-		/* Read set current from device. */
-		target = &(devc->current_max);
-		break;
-	case KAXXXXP_VOLTAGE:
-		/* Read voltage from device. */
-		target = &(devc->voltage);
-		break;
-	case KAXXXXP_VOLTAGE_MAX:
-		/* Read set voltage from device. */
-		target = &(devc->voltage_max);
-		break;
-	case KAXXXXP_STATUS:
-	case KAXXXXP_OUTPUT:
-		/* Read status from device. */
-		count = 1;
-		break;
-	default:
-		sr_err("Don't know where to put repply %d.", devc->target);
-	}
-
-	if ((ret = korad_kaxxxxp_read_chars(serial, count, devc->reply)) < 0)
 		return ret;
+	}
 
-	devc->reply[count] = 0;
+	reply[count] = 0;
 
-	if (target) {
+	if (value) {
-		value = g_ascii_strtod(devc->reply, NULL);
+		sr_atof_ascii((const char *)&reply, value);
-		*target = (float)value;
+		sr_dbg("value: %f", *value);
-		sr_dbg("value: %f",value);
 	} else {
 		/* We have status reply. */
-		status_byte = devc->reply[0];
+		status_byte = reply[0];
 		/* Constant current */
 		devc->cc_mode[0] = !(status_byte & (1 << 0)); /* Channel one */
 		devc->cc_mode[1] = !(status_byte & (1 << 1)); /* Channel two */
@@ -284,41 +250,45 @@ SR_PRIV int korad_kaxxxxp_get_reply(struct sr_serial_dev_inst *serial,
 			(status_byte & (1 << 6)) ? "enabled" : "disabled",
 			(status_byte & (1 << 7)) ? "true" : "false");
 	}
+
 	/* Read the sixth byte from ISET? BUG workaround. */
-	if (devc->target == KAXXXXP_CURRENT_MAX)
+	if (target == KAXXXXP_CURRENT_MAX)
 		serial_read_blocking(serial, &status_byte, 1, 10);
-	devc->reply_pending = FALSE;
+
+	g_mutex_unlock(&devc->rw_mutex);
+
+	return ret;
+}
+
+SR_PRIV int korad_kaxxxxp_get_all_values(struct sr_serial_dev_inst *serial,
+				struct dev_context *devc)
+{
+	int ret, target;
+
+	for (target = KAXXXXP_CURRENT;
+			target <= KAXXXXP_STATUS; target++) {
+		if ((ret = korad_kaxxxxp_get_value(serial, target, devc)) < 0)
+			return ret;
+	}
 
 	return ret;
 }
 
 static void next_measurement(struct dev_context *devc)
 {
-	switch (devc->target) {
+	switch (devc->acquisition_target) {
 	case KAXXXXP_CURRENT:
-		devc->target = KAXXXXP_VOLTAGE;
+		devc->acquisition_target = KAXXXXP_VOLTAGE;
-		break;
-	case KAXXXXP_CURRENT_MAX:
-		devc->target = KAXXXXP_CURRENT;
 		break;
 	case KAXXXXP_VOLTAGE:
-		devc->target = KAXXXXP_STATUS;
+		devc->acquisition_target = KAXXXXP_STATUS;
-		break;
-	case KAXXXXP_VOLTAGE_MAX:
-		devc->target = KAXXXXP_CURRENT;
-		break;
-	/* Read back what was set. */
-	case KAXXXXP_BEEP:
-	case KAXXXXP_OCP:
-	case KAXXXXP_OVP:
-	case KAXXXXP_OUTPUT:
-		devc->target = KAXXXXP_STATUS;
 		break;
 	case KAXXXXP_STATUS:
-		devc->target = KAXXXXP_CURRENT;
+		devc->acquisition_target = KAXXXXP_CURRENT;
 		break;
 	default:
-		devc->target = KAXXXXP_CURRENT;
+		devc->acquisition_target = KAXXXXP_CURRENT;
+		sr_err("Invalid target for next acquisition.");
 	}
 }
 
@@ -332,7 +302,6 @@ SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data)
 	struct sr_analog_encoding encoding;
 	struct sr_analog_meaning meaning;
 	struct sr_analog_spec spec;
-	uint64_t elapsed_us;
 	GSList *l;
 
 	(void)fd;
@@ -347,7 +316,7 @@ SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data)
 
 	if (revents == G_IO_IN) {
 		/* Get the value. */
-		korad_kaxxxxp_get_reply(serial, devc);
+		korad_kaxxxxp_get_value(serial, devc->acquisition_target, devc);
 
 		/* Note: digits/spec_digits will be overridden later. */
 		sr_analog_init(&analog, &encoding, &meaning, &spec, 0);
@@ -357,7 +326,7 @@ SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data)
 		packet.payload = &analog;
 		analog.num_samples = 1;
 		l = g_slist_copy(sdi->channels);
-		if (devc->target == KAXXXXP_CURRENT) {
+		if (devc->acquisition_target == KAXXXXP_CURRENT) {
 			l = g_slist_remove_link(l, g_slist_nth(l, 0));
 			analog.meaning->channels = l;
 			analog.meaning->mq = SR_MQ_CURRENT;
@@ -368,7 +337,7 @@ SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data)
 			analog.data = &devc->current;
 			sr_session_send(sdi, &packet);
 		}
-		else if (devc->target == KAXXXXP_VOLTAGE) {
+		else if (devc->acquisition_target == KAXXXXP_VOLTAGE) {
 			l = g_slist_remove_link(l, g_slist_nth(l, 1));
 			analog.meaning->channels = l;
 			analog.meaning->mq = SR_MQ_VOLTAGE;
@@ -381,31 +350,10 @@ SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data)
 			sr_sw_limits_update_samples_read(&devc->limits, 1);
 		}
 		next_measurement(devc);
-	} else {
-		/* Time out */
-		if (!devc->reply_pending) {
-			if (korad_kaxxxxp_query_value(serial, devc) < 0)
-				return TRUE;
-			devc->req_sent_at = g_get_monotonic_time();
-			devc->reply_pending = TRUE;
-		}
 	}
 
-	if (sr_sw_limits_check(&devc->limits)) {
+	if (sr_sw_limits_check(&devc->limits))
 		sr_dev_acquisition_stop(sdi);
-		return TRUE;
-	}
-
-	/* Request next packet, if required. */
-	if (sdi->status == SR_ST_ACTIVE) {
-		if (devc->reply_pending) {
-			elapsed_us = g_get_monotonic_time() - devc->req_sent_at;
-			if (elapsed_us > (REQ_TIMEOUT_MS * 1000))
-				devc->reply_pending = FALSE;
-			return TRUE;
-		}
-
-	}
 
 	return TRUE;
 }

src/hardware/korad-kaxxxxp/protocol.h

@@ -2,6 +2,7 @@
  * This file is part of the libsigrok project.
  *
  * Copyright (C) 2015 Hannu Vuolasaho <vuokkosetae@gmail.com>
+ * Copyright (C) 2018 Frank Stettner <frank-stettner@gmx.net>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -70,7 +71,7 @@ struct dev_context {
 
 	struct sr_sw_limits limits;
 	int64_t req_sent_at;
-	gboolean reply_pending;
+	GMutex rw_mutex;
 
 	float current;          /**< Last current value [A] read from device. */
 	float current_max;      /**< Output current set. */
@@ -83,9 +84,8 @@ struct dev_context {
 	gboolean ocp_enabled;    /**< Output current protection enabled. */
 	gboolean ovp_enabled;    /**< Output voltage protection enabled. */
 
-	int target;              /**< What reply to expect. */
+	int acquisition_target;  /**< What reply to expect. */
 	int program;             /**< Program to store or recall. */
-	char reply[6];
 };
 
 SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial,
@@ -93,11 +93,9 @@ SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial,
 SR_PRIV int korad_kaxxxxp_read_chars(struct sr_serial_dev_inst *serial,
 					int count, char *buf);
 SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
-					struct dev_context *devc);
+					int target, struct dev_context *devc);
-SR_PRIV int korad_kaxxxxp_query_value(struct sr_serial_dev_inst *serial,
+SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial,
-					struct dev_context *devc);
+					int target, struct dev_context *devc);
-SR_PRIV int korad_kaxxxxp_get_reply(struct sr_serial_dev_inst *serial,
-					struct dev_context *devc);
 SR_PRIV int korad_kaxxxxp_get_all_values(struct sr_serial_dev_inst *serial,
 					struct dev_context *devc);
 SR_PRIV int korad_kaxxxxp_receive_data(int fd, int revents, void *cb_data);