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 519a1726399b060719df6c43f2432900266e3273..99ccd42f1d9be3b78693a7f626c79e4d2b5010cd 100644 (file)
--- 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 880468b41e307ecb79c2eccde5a14bdc461592cb..a35052dfebbb71215058ab2810b3640d7c768053 100644 (file)
--- 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 ecdf40e598aae48cbdd37fe41ae39553d61711c5..8dd669722d776e6a47c8588cafc48979ed3884a0 100644 (file)
--- 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 e93658208831324953fdf3f3de61cd1b12e063e7..aee7fcc51b6bb7f6ba6558e09b4be415cecc6491 100644 (file)
--- 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 6fc99f7d1f42cc277c9e0e30e2b90fab2e84a83e..f4fe651b94dee45a34dea4784f5ab48e71e63ae3 100644 (file)
--- 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 8d05235ceb723adf342589f85c5867e8fcbc07b6..e2f5cd5870a8ec7d2fa61dc3eed6b2cb754246cc 100644 (file)
--- 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);