From: Frank Stettner Date: Sat, 9 Dec 2017 18:45:02 +0000 (+0100) Subject: scpi-pps: Reimplemention of switching channel groups (PSU channels) X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=17a82e83ff4c2a7dc1bcc4f5ab6197c16a3b72de;p=libsigrok.git 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. --- diff --git a/src/device.c b/src/device.c index 519a1726..99ccd42f 100644 --- a/src/device.c +++ b/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; diff --git a/src/hardware/scpi-pps/api.c b/src/hardware/scpi-pps/api.c index 880468b4..a35052df 100644 --- a/src/hardware/scpi-pps/api.c +++ b/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; } diff --git a/src/hardware/scpi-pps/protocol.c b/src/hardware/scpi-pps/protocol.c index ecdf40e5..8dd66972 100644 --- a/src/hardware/scpi-pps/protocol.c +++ b/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; diff --git a/src/hardware/scpi-pps/protocol.h b/src/hardware/scpi-pps/protocol.h index e9365820..aee7fcc5 100644 --- a/src/hardware/scpi-pps/protocol.h +++ b/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; diff --git a/src/scpi.h b/src/scpi.h index 6fc99f7d..f4fe651b 100644 --- a/src/scpi.h +++ b/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 diff --git a/src/scpi/scpi.c b/src/scpi/scpi.c index 8d05235c..e2f5cd58 100644 --- a/src/scpi/scpi.c +++ b/src/scpi/scpi.c @@ -176,10 +176,16 @@ static int scpi_send_variadic(struct sr_scpi_dev_inst *scpi, * * @return SR_OK on success, SR_ERR on failure. */ -static int scpi_send(struct sr_scpi_dev_inst *scpi, const char *format, - va_list args) +static int scpi_send(struct sr_scpi_dev_inst *scpi, const char *format, ...) { - return scpi_send_variadic(scpi, format, args); + va_list args; + int ret; + + va_start(args, format); + ret = scpi_send_variadic(scpi, format, args); + va_end(args); + + return ret; } /** @@ -266,11 +272,10 @@ static int scpi_get_data(struct sr_scpi_dev_inst *scpi, GString *response; int space; gint64 timeout; - va_list empty_va_list; /* Optionally send caller provided command. */ if (command) { - if (scpi_send(scpi, command, empty_va_list) != SR_OK) + if (scpi_send(scpi, command) != SR_OK) return SR_ERR; } @@ -943,12 +948,11 @@ SR_PRIV int sr_scpi_get_block(struct sr_scpi_dev_inst *scpi, long llen; long datalen; gint64 timeout; - va_list empty_va_list; g_mutex_lock(&scpi->scpi_mutex); if (command) - if (scpi_send(scpi, command, empty_va_list) != SR_OK) { + if (scpi_send(scpi, command) != SR_OK) { g_mutex_unlock(&scpi->scpi_mutex); return SR_ERR; } @@ -1146,7 +1150,8 @@ SR_PRIV const char *sr_vendor_alias(const char *raw_vendor) return 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) { unsigned int i; const char *cmd; @@ -1165,33 +1170,54 @@ SR_PRIV const char *sr_scpi_cmd_get(const struct scpi_command *cmdtable, int com return cmd; } -SR_PRIV int sr_scpi_cmd(const struct sr_dev_inst *sdi, const struct scpi_command *cmdtable, +SR_PRIV int sr_scpi_cmd(const struct sr_dev_inst *sdi, + const struct scpi_command *cmdtable, + int channel_command, const char *channel_name, int command, ...) { struct sr_scpi_dev_inst *scpi; va_list args; int ret; + const char *channel_cmd; const char *cmd; + scpi = sdi->conn; + if (!(cmd = sr_scpi_cmd_get(cmdtable, command))) { /* Device does not implement this command, that's OK. */ return SR_OK; } - scpi = sdi->conn; + g_mutex_lock(&scpi->scpi_mutex); + + /* Select channel. */ + channel_cmd = sr_scpi_cmd_get(cmdtable, channel_command); + if (channel_cmd && channel_name && + g_strcmp0(channel_name, scpi->actual_channel_name)) { + sr_spew("sr_scpi_cmd(): new channel = %s", channel_name); + scpi->actual_channel_name = channel_name; + ret = scpi_send(scpi, channel_cmd, channel_name); + if (ret != SR_OK) + return ret; + } + va_start(args, command); - ret = sr_scpi_send_variadic(scpi, cmd, args); + ret = scpi_send_variadic(scpi, cmd, args); va_end(args); + g_mutex_unlock(&scpi->scpi_mutex); + return ret; } 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, ...) { struct sr_scpi_dev_inst *scpi; va_list args; + const char *channel_cmd; const char *cmd; GString *response; char *s; @@ -1208,6 +1234,17 @@ SR_PRIV int sr_scpi_cmd_resp(const struct sr_dev_inst *sdi, g_mutex_lock(&scpi->scpi_mutex); + /* Select channel. */ + channel_cmd = sr_scpi_cmd_get(cmdtable, channel_command); + if (channel_cmd && channel_name && + g_strcmp0(channel_name, scpi->actual_channel_name)) { + sr_spew("sr_scpi_cmd_get(): new channel = %s", channel_name); + scpi->actual_channel_name = channel_name; + ret = scpi_send(scpi, channel_cmd, channel_name); + if (ret != SR_OK) + return ret; + } + va_start(args, command); ret = scpi_send_variadic(scpi, cmd, args); va_end(args);