scpi-pps: Reimplemention of switching channel groups (PSU channels)

Acquisition won't work correctly in a multi-threaded environment, when
config_set() and config_get() are called with a channel group.
The channel switching itself has moved to scpi/scpi.c, to be able to
handle switching in a thread safe way.

src/device.c

@@ -135,10 +135,18 @@ SR_API int sr_dev_channel_enable(struct sr_channel *channel, gboolean state)
 	return SR_OK;
 }
 
-/* Returns the next enabled channel, wrapping around if necessary. */
-/** @private */
+/**
+ * Returns the next enabled channel, wrapping around if necessary.
+ *
+ * @param[in] sdi The device instance the channel is connected to.
+ *                Must not be NULL.
+ * @param[in] cur_channel The current channel.
+ *
+ * @return A pointer to the next enabled channel of this device.
+ *
+ * @private
+ */
 SR_PRIV struct sr_channel *sr_next_enabled_channel(const struct sr_dev_inst *sdi,
-
 		struct sr_channel *cur_channel)
 {
 	struct sr_channel *next_channel;

src/hardware/scpi-pps/api.c

@@ -164,7 +164,7 @@ static struct sr_dev_inst *probe_device(struct sr_scpi_dev_inst *scpi,
 	sr_scpi_hw_info_free(hw_info);
 	hw_info = NULL;
 
-	sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_LOCAL);
+	sr_scpi_cmd(sdi, devc->device->commands, 0, NULL, SCPI_CMD_LOCAL);
 
 	return sdi;
 }
@@ -256,13 +256,14 @@ static int dev_open(struct sr_dev_inst *sdi)
 		return SR_ERR;
 
 	devc = sdi->priv;
-	sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_REMOTE);
+	sr_scpi_cmd(sdi, devc->device->commands, 0, NULL, SCPI_CMD_REMOTE);
 	devc->beeper_was_set = FALSE;
-	if (sr_scpi_cmd_resp(sdi, devc->device->commands, &beeper,
-			G_VARIANT_TYPE_BOOLEAN, SCPI_CMD_BEEPER) == SR_OK) {
+	if (sr_scpi_cmd_resp(sdi, devc->device->commands, 0, NULL,
+			&beeper, G_VARIANT_TYPE_BOOLEAN, SCPI_CMD_BEEPER) == SR_OK) {
 		if (g_variant_get_boolean(beeper)) {
 			devc->beeper_was_set = TRUE;
-			sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_BEEPER_DISABLE);
+			sr_scpi_cmd(sdi, devc->device->commands,
+				0, NULL, SCPI_CMD_BEEPER_DISABLE);
 		}
 		g_variant_unref(beeper);
 	}
@@ -282,8 +283,9 @@ static int dev_close(struct sr_dev_inst *sdi)
 		return SR_ERR_BUG;
 
 	if (devc->beeper_was_set)
-		sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_BEEPER_ENABLE);
-	sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_LOCAL);
+		sr_scpi_cmd(sdi, devc->device->commands,
+			0, NULL, SCPI_CMD_BEEPER_ENABLE);
+	sr_scpi_cmd(sdi, devc->device->commands, 0, NULL, SCPI_CMD_LOCAL);
 
 	return sr_scpi_close(scpi);
 }
@@ -305,6 +307,8 @@ static int config_get(uint32_t key, GVariant **data,
 	struct dev_context *devc;
 	const GVariantType *gvtype;
 	unsigned int i;
+	int channel_group_cmd;
+	char *channel_group_name;
 	int cmd, ret;
 	const char *s;
 
@@ -399,9 +403,16 @@ static int config_get(uint32_t key, GVariant **data,
 	if (!gvtype)
 		return SR_ERR_NA;
 
-	if (cg)
-		select_channel(sdi, cg->channels->data);
-	ret = sr_scpi_cmd_resp(sdi, devc->device->commands, data, gvtype, cmd);
+	channel_group_cmd = 0;
+	channel_group_name = NULL;
+	if (cg) {
+		channel_group_cmd = SCPI_CMD_SELECT_CHANNEL;
+		channel_group_name = g_strdup(cg->name);
+	}
+
+	ret = sr_scpi_cmd_resp(sdi, devc->device->commands,
+		channel_group_cmd, channel_group_name, data, gvtype, cmd);
+	g_free(channel_group_name);
 
 	if (cmd == SCPI_CMD_GET_OUTPUT_REGULATION) {
 		/*
@@ -433,78 +444,100 @@ static int config_set(uint32_t key, GVariant *data,
 {
 	struct dev_context *devc;
 	double d;
+	int channel_group_cmd;
+	char *channel_group_name;
+	int ret;
 
 	if (!sdi)
 		return SR_ERR_ARG;
 
-	if (cg)
-		select_channel(sdi, cg->channels->data);
+	channel_group_cmd = 0;
+	channel_group_name = NULL;
+	if (cg) {
+		channel_group_cmd = SCPI_CMD_SELECT_CHANNEL;
+		channel_group_name = g_strdup(cg->name);
+	}
 
 	devc = sdi->priv;
 
 	switch (key) {
 	case SR_CONF_ENABLED:
 		if (g_variant_get_boolean(data))
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OUTPUT_ENABLE);
 		else
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OUTPUT_DISABLE);
 		break;
 	case SR_CONF_VOLTAGE_TARGET:
 		d = g_variant_get_double(data);
-		return sr_scpi_cmd(sdi, devc->device->commands,
+		ret = sr_scpi_cmd(sdi, devc->device->commands,
+				channel_group_cmd, channel_group_name,
 				SCPI_CMD_SET_VOLTAGE_TARGET, d);
 		break;
 	case SR_CONF_OUTPUT_FREQUENCY_TARGET:
 		d = g_variant_get_double(data);
-		return sr_scpi_cmd(sdi, devc->device->commands,
+		ret = sr_scpi_cmd(sdi, devc->device->commands,
+				channel_group_cmd, channel_group_name,
 				SCPI_CMD_SET_FREQUENCY_TARGET, d);
 		break;
 	case SR_CONF_CURRENT_LIMIT:
 		d = g_variant_get_double(data);
-		return sr_scpi_cmd(sdi, devc->device->commands,
+		ret = sr_scpi_cmd(sdi, devc->device->commands,
+				channel_group_cmd, channel_group_name,
 				SCPI_CMD_SET_CURRENT_LIMIT, d);
 		break;
 	case SR_CONF_OVER_VOLTAGE_PROTECTION_ENABLED:
 		if (g_variant_get_boolean(data))
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_VOLTAGE_PROTECTION_ENABLE);
 		else
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_VOLTAGE_PROTECTION_DISABLE);
 		break;
 	case SR_CONF_OVER_VOLTAGE_PROTECTION_THRESHOLD:
 		d = g_variant_get_double(data);
-		return sr_scpi_cmd(sdi, devc->device->commands,
+		ret = sr_scpi_cmd(sdi, devc->device->commands,
+				channel_group_cmd, channel_group_name,
 				SCPI_CMD_SET_OVER_VOLTAGE_PROTECTION_THRESHOLD, d);
 		break;
 	case SR_CONF_OVER_CURRENT_PROTECTION_ENABLED:
 		if (g_variant_get_boolean(data))
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_CURRENT_PROTECTION_ENABLE);
 		else
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_CURRENT_PROTECTION_DISABLE);
 		break;
 	case SR_CONF_OVER_CURRENT_PROTECTION_THRESHOLD:
 		d = g_variant_get_double(data);
-		return sr_scpi_cmd(sdi, devc->device->commands,
+		ret = sr_scpi_cmd(sdi, devc->device->commands,
+				channel_group_cmd, channel_group_name,
 				SCPI_CMD_SET_OVER_CURRENT_PROTECTION_THRESHOLD, d);
 		break;
 	case SR_CONF_OVER_TEMPERATURE_PROTECTION:
 		if (g_variant_get_boolean(data))
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_TEMPERATURE_PROTECTION_ENABLE);
 		else
-			return sr_scpi_cmd(sdi, devc->device->commands,
+			ret = sr_scpi_cmd(sdi, devc->device->commands,
+					channel_group_cmd, channel_group_name,
 					SCPI_CMD_SET_OVER_TEMPERATURE_PROTECTION_DISABLE);
 		break;
 	default:
-		return SR_ERR_NA;
+		ret = SR_ERR_NA;
 	}
 
-	return SR_OK;
+	g_free(channel_group_name);
+
+	return ret;
 }
 
 static int config_list(uint32_t key, GVariant **data,
@@ -588,24 +621,19 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 {
 	struct dev_context *devc;
 	struct sr_scpi_dev_inst *scpi;
-	struct sr_channel *ch;
-	struct pps_channel *pch;
 	int ret;
 
 	devc = sdi->priv;
 	scpi = sdi->conn;
 
+	/* Prime the pipe with the first channel. */
+	devc->cur_acquisition_channel = sr_next_enabled_channel(sdi, NULL);
+
 	if ((ret = sr_scpi_source_add(sdi->session, scpi, G_IO_IN, 10,
 			scpi_pps_receive_data, (void *)sdi)) != SR_OK)
 		return ret;
 	std_session_send_df_header(sdi);
 
-	/* Prime the pipe with the first channel's fetch. */
-	ch = sr_next_enabled_channel(sdi, NULL);
-	pch = ch->priv;
-	if ((ret = select_channel(sdi, ch)) < 0)
-		return ret;
-
 	return SR_OK;
 }
 

src/hardware/scpi-pps/protocol.c

@@ -24,36 +24,6 @@
 #include "scpi.h"
 #include "protocol.h"
 
-SR_PRIV int select_channel(const struct sr_dev_inst *sdi, struct sr_channel *ch)
-{
-	struct dev_context *devc;
-	struct pps_channel *cur_pch, *new_pch;
-	int ret;
-
-	if (g_slist_length(sdi->channels) == 1)
-		return SR_OK;
-
-	devc = sdi->priv;
-	if (ch == devc->cur_channel)
-		return SR_OK;
-
-	new_pch = ch->priv;
-	if (devc->cur_channel) {
-		cur_pch = devc->cur_channel->priv;
-		if (cur_pch->hw_output_idx == new_pch->hw_output_idx) {
-			/* Same underlying output channel. */
-			devc->cur_channel = ch;
-			return SR_OK;
-		}
-	}
-
-	if ((ret = sr_scpi_cmd(sdi, devc->device->commands, SCPI_CMD_SELECT_CHANNEL,
-			new_pch->hwname)) >= 0)
-		devc->cur_channel = ch;
-
-	return ret;
-}
-
 SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 {
 	struct dev_context *devc;
@@ -63,7 +33,8 @@ SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 	struct sr_analog_meaning meaning;
 	struct sr_analog_spec spec;
 	const struct sr_dev_inst *sdi;
-	struct sr_channel *next_channel;
+	int channel_group_cmd;
+	char *channel_group_name;
 	struct pps_channel *pch;
 	const struct channel_spec *ch_spec;
 	int ret;
@@ -81,7 +52,14 @@ SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 	if (!(devc = sdi->priv))
 		return TRUE;
 
-	pch = devc->cur_channel->priv;
+	channel_group_cmd = 0;
+	channel_group_name = NULL;
+	if (g_slist_length(sdi->channel_groups) > 1) {
+		channel_group_cmd = SCPI_CMD_SELECT_CHANNEL;
+		channel_group_name = g_strdup(devc->cur_acquisition_channel->name);
+	}
+
+	pch = devc->cur_acquisition_channel->priv;
 	if (pch->mq == SR_MQ_VOLTAGE) {
 		gvtype = G_VARIANT_TYPE_DOUBLE;
 		cmd = SCPI_CMD_GET_MEAS_VOLTAGE;
@@ -98,9 +76,10 @@ SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 		return SR_ERR;
 	}
 
-	//sr_scpi_cmd(sdi, devc->device->commands, cmd, pch->hwname); <- api.c 1x called
-	//sr_scpi_cmd(sdi, devc->device->commands, cmd); <- protocol.c xx called
-	ret = sr_scpi_cmd_resp(sdi, devc->device->commands, &gvdata, gvtype, cmd);
+	ret = sr_scpi_cmd_resp(sdi, devc->device->commands,
+		channel_group_cmd, channel_group_name, &gvdata, gvtype, cmd);
+	g_free(channel_group_name);
+
 	if (ret != SR_OK)
 		return ret;
 
@@ -109,7 +88,7 @@ SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 	packet.payload = &analog;
 	/* Note: digits/spec_digits will be overridden later. */
 	sr_analog_init(&analog, &encoding, &meaning, &spec, 0);
-	analog.meaning->channels = g_slist_append(NULL, devc->cur_channel);
+	analog.meaning->channels = g_slist_append(NULL, devc->cur_acquisition_channel);
 	analog.num_samples = 1;
 	analog.meaning->mq = pch->mq;
 	if (pch->mq == SR_MQ_VOLTAGE) {
@@ -131,12 +110,10 @@ SR_PRIV int scpi_pps_receive_data(int fd, int revents, void *cb_data)
 	sr_session_send(sdi, &packet);
 	g_slist_free(analog.meaning->channels);
 
+	/* Next channel. */
 	if (g_slist_length(sdi->channels) > 1) {
-		next_channel = sr_next_enabled_channel(sdi, devc->cur_channel);
-		if (select_channel(sdi, next_channel) != SR_OK) {
-			sr_err("Failed to select channel %s", next_channel->name);
-			return FALSE;
-		}
+		devc->cur_acquisition_channel =
+			sr_next_enabled_channel(sdi, devc->cur_acquisition_channel);
 	}
 
 	return TRUE;

src/hardware/scpi-pps/protocol.h

@@ -29,7 +29,7 @@
 #define LOG_PREFIX "scpi-pps"
 
 enum pps_scpi_cmds {
-	SCPI_CMD_REMOTE,
+	SCPI_CMD_REMOTE = 1,
 	SCPI_CMD_LOCAL,
 	SCPI_CMD_BEEPER,
 	SCPI_CMD_BEEPER_ENABLE,
@@ -143,7 +143,7 @@ struct dev_context {
 	struct channel_spec *channels;
 	struct channel_group_spec *channel_groups;
 
-	struct sr_channel *cur_channel;
+	struct sr_channel *cur_acquisition_channel;
 };
 
 SR_PRIV extern unsigned int num_pps_profiles;

src/scpi.h

@@ -100,6 +100,7 @@ struct sr_scpi_dev_inst {
 	/* Only used for quirk workarounds, notably the Rigol DS1000 series. */
 	uint64_t firmware_version;
 	GMutex scpi_mutex;
+	const char *actual_channel_name;
 };
 
 SR_PRIV GSList *sr_scpi_scan(struct drv_context *drvc, GSList *options,
@@ -149,11 +150,15 @@ SR_PRIV int sr_scpi_get_hw_id(struct sr_scpi_dev_inst *scpi,
 SR_PRIV void sr_scpi_hw_info_free(struct sr_scpi_hw_info *hw_info);
 
 SR_PRIV const char *sr_vendor_alias(const char *raw_vendor);
-SR_PRIV const char *sr_scpi_cmd_get(const struct scpi_command *cmdtable, int command);
+SR_PRIV const char *sr_scpi_cmd_get(const struct scpi_command *cmdtable,
+		int command);
 SR_PRIV int sr_scpi_cmd(const struct sr_dev_inst *sdi,
-		const struct scpi_command *cmdtable, int command, ...);
+		const struct scpi_command *cmdtable,
+		int channel_command, const char *channel_name,
+		int command, ...);
 SR_PRIV int sr_scpi_cmd_resp(const struct sr_dev_inst *sdi,
 		const struct scpi_command *cmdtable,
+		int channel_command, const char *channel_name,
 		GVariant **gvar, const GVariantType *gvtype, int command, ...);
 
 #endif