SCPI framework is really nice, but currently it cannot be used with a rather large group of instruments that support "SCPI like" protocols (that are not fully SCPI compliant).
I'd like to propose generic "quirks" support to SCPI framework to allow using this excellent framework in drivers for devices that aren't fully SCPI compliant. I'm proposing adding "quirks" variable in sr_scpi_dev_inst struct that contains bitmap of various flags for quirks. This way a hardware driver can enable quirks if needed, and SCPI code can be kept clean and readable while still supporting devices not conforming to standards. This should reduce the need for basically "re-implementing" what SCPI framework already can do in countless drivers for devices that implement only subset of SCPI protocol, etc... This initial implementation is based on adding Siglent (SPD3303C/X/X-E) PSU support to scpi-pps driver. (I also have GW-Instek 8200A series support added to scpi-dmm driver that would also need additional quirk to integrate cleanly...) (https://github.com/sigrokproject/libsigrok/commit/ca3de968497e6b0e890af74cdd26ca4d43c1178a) diff --git a/src/scpi.h b/src/scpi.h index 312f5e8a..7e55c9f3 100644 --- a/src/scpi.h +++ b/src/scpi.h @@ -75,6 +75,12 @@ enum scpi_transport_layer { SCPI_TRANSPORT_VXI, }; +enum scpi_quirks { + SCPI_QUIRK_CMD_OMIT_LF = (1 << 0), + SCPI_QUIRK_OPC_UNSUPPORTED = (1 << 1), + SCPI_QUIRK_SLOW_CHANNEL_SELECT = (1 << 2), +}; + struct scpi_command { int command; const char *string; @@ -113,6 +119,7 @@ struct sr_scpi_dev_inst { uint64_t firmware_version; GMutex scpi_mutex; char *actual_channel_name; + uint32_t quirks; }; SR_PRIV GSList *sr_scpi_scan(struct drv_context *drvc, GSList *options, diff --git a/src/scpi/scpi.c b/src/scpi/scpi.c index 2477cf23..b0fa4ed5 100644 --- a/src/scpi/scpi.c +++ b/src/scpi/scpi.c @@ -157,8 +157,10 @@ static int scpi_send_variadic(struct sr_scpi_dev_inst *scpi, /* Allocate buffer and write out command. */ buf = g_malloc0(len + 2); sr_vsprintf_ascii(buf, format, args); - if (buf[len - 1] != '\n') - buf[len] = '\n'; + if (!(scpi->quirks & SCPI_QUIRK_CMD_OMIT_LF)) { + if (buf[len - 1] != '\n') + buf[len] = '\n'; + } /* Send command. */ ret = scpi->send(scpi->priv, buf); @@ -819,6 +821,11 @@ SR_PRIV int sr_scpi_get_opc(struct sr_scpi_dev_inst *scpi) unsigned int i; gboolean opc; + if (scpi->quirks & SCPI_QUIRK_OPC_UNSUPPORTED) { + g_usleep(SCPI_READ_RETRY_TIMEOUT_US); + return SR_OK; + } + for (i = 0; i < SCPI_READ_RETRIES; i++) { opc = FALSE; sr_scpi_get_bool(scpi, SCPI_CMD_OPC, &opc); @@ -1266,6 +1273,8 @@ SR_PRIV int sr_scpi_cmd(const struct sr_dev_inst *sdi, ret = scpi_send(scpi, channel_cmd, channel_name); if (ret != SR_OK) return ret; + if (scpi->quirks & SCPI_QUIRK_SLOW_CHANNEL_SELECT) + g_usleep(100 * 1000); } va_start(args, command); @@ -1311,6 +1320,8 @@ SR_PRIV int sr_scpi_cmd_resp(const struct sr_dev_inst *sdi, ret = scpi_send(scpi, channel_cmd, channel_name); if (ret != SR_OK) return ret; + if (scpi->quirks & SCPI_QUIRK_SLOW_CHANNEL_SELECT) + g_usleep(100*1000); } va_start(args, command); Does this approach sound ok? (it would seem like existing code has support for some specific "quirks" already but it likely would be easier to maintain having quirks only enabled if needed) Initia Siglent SPD3303 support for scpi-pps driver: https://github.com/tjko/libsigrok/tree/siglent-scpi -- Timo <t...@iki.fi> _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
