On Wed, May 27, 2020 at 10:46 PM Gerhard Sittig <gerhard.sit...@gmx.net> wrote:
> The reason why I ask: Does that change of collapsing _any_ of the
> CR and/or LF combinations affect other devices which currently
> are supported? I know that there are empty requests (initiating
> remote operation by sending a mere newline). Are there empty
> responses, too? Because these now would get dropped, or are not
> detected any longer, or are mistaken as something different and
> the request/response relation gets out of sync.

I re-thought the changes to SCPI and it's now down to one tiny change
to scpi_serial.c
that should not break any existing SCPI devices. Since any devices
that work with
current SCPI (serial) code must be terminating their responses with NL (<LF>),
or they won't work (symptom being SCPI commands timing out).

I'm suggesting changing the scpi_serial_read_data() slightly to look for special
case where returned data ends with <LF><CR> and squash the <CR>
(only in this specific instance). This allows any devices that
interpret the spec
like GW-Instek does to work. Eithout affecting devices terminating
responses with
only <LF> like what seems to be how majority of vendors interpret the spec.

diff --git a/src/scpi/scpi_serial.c b/src/scpi/scpi_serial.c
index 90c01424..43bd044d 100644
--- a/src/scpi/scpi_serial.c
+++ b/src/scpi/scpi_serial.c
@@ -190,7 +190,12 @@ static int scpi_serial_read_data(void *priv, char
*buf, int maxlen)
        if (ret > 0) {
                if (buf[ret - 1] == '\n') {
                        sscpi->got_newline = TRUE;
-                       sr_spew("Received terminator");
+                       sr_spew("Received NL terminator");
+               } else if (ret > 1 &&
+                          buf[ret - 2] == '\n' && buf[ret - 1] == '\r') {
+                       sscpi->got_newline = TRUE;
+                       sr_spew("Received NL+CR terminator");
+                       ret--;
                } else {
                        sscpi->got_newline = FALSE;
                }


Well, there is another minor change to scpi.c, but this clearly won't
break any existing
stuff (?).  Since it simply adds support for devices that respond to
SCPI command *IDN?
with out the serial number field:

diff --git a/src/scpi/scpi.c b/src/scpi/scpi.c
index 2477cf23..cc213ff8 100644
--- a/src/scpi/scpi.c
+++ b/src/scpi/scpi.c
@@ -1117,8 +1117,8 @@ SR_PRIV int sr_scpi_get_hw_id(struct
sr_scpi_dev_inst *scpi,
         */
        tokens = g_strsplit(response, ",", 0);
        num_tokens = g_strv_length(tokens);
-       if (num_tokens < 4) {
-               sr_dbg("IDN response not according to spec: %80.s.", response);
+       if (num_tokens < 3) {
+               sr_dbg("IDN response not according to spec: '%s'", response);
                g_strfreev(tokens);
                g_free(response);
                return SR_ERR_DATA;
@@ -1134,8 +1134,13 @@ SR_PRIV int sr_scpi_get_hw_id(struct
sr_scpi_dev_inst *scpi,
                hw_info->manufacturer = g_strstrip(g_strdup(idn_substr + 4));

        hw_info->model = g_strstrip(g_strdup(tokens[1]));
-       hw_info->serial_number = g_strstrip(g_strdup(tokens[2]));
-       hw_info->firmware_version = g_strstrip(g_strdup(tokens[3]));
+       if (num_tokens < 4) {
+               hw_info->serial_number = g_strdup("Unknown");
+               hw_info->firmware_version = g_strstrip(g_strdup(tokens[2]));
+       } else {
+               hw_info->serial_number = g_strstrip(g_strdup(tokens[2]));
+               hw_info->firmware_version = g_strstrip(g_strdup(tokens[3]));
+       }

        g_strfreev(tokens);



I would really appreciate comments and review of these changes:

The following changes since commit cbfaf5e073765175236d562c05f21fef9d9cd54f:

  uni-t-ut181a: comment on how to start a recording (2020-06-01 18:35:05 +0200)

are available in the Git repository at:

  https://github.com/tjko/libsigrok gwinstek

for you to fetch changes up to bea151f4ea17f9a2fa66224f79053fd15c133bb3:

  Make sure we report "INFINITY" when meter displays "0L" (2020-06-02
01:50:13 -0700)

----------------------------------------------------------------
Timo Kokkonen (4):
      added support to GW-Instek 8200A series bench meters
      add support for (broken?) devices sending NR+CR terminator
      Add support for devices that do not report serial number in
response to *IDN? command. And fix bug in sr_dbg() format string.
      Make sure we report "INFINITY" when meter displays "0L"

 src/hardware/scpi-dmm/api.c      |  69 +++++++++++++
 src/hardware/scpi-dmm/protocol.c | 209 +++++++++++++++++++++++++++++++++++++++
 src/hardware/scpi-dmm/protocol.h |   2 +
 src/scpi/scpi.c                  |  13 ++-
 src/scpi/scpi_serial.c           |   7 +-
 5 files changed, 295 insertions(+), 5 deletions(-)





-- 
Timo <t...@iki.fi>


_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel

Reply via email to