Query over TCPIP that should timeout instead returns a blank string?

[joshsleeper]

Although it should be raising a timeout error, all I get back is the empty string and PyVisa continues on as if everything is alright. I've traced it all the way down to rpc.Client.make_call() (link) and the query is definitely returning error code 15, which represents a timeout according to the vxi11.ErrorCodes enum (link).

It looks like the only possible places that the error would be raised are in highlevel.PyVisaLibrary._return_handler() (link) and highlevel.PyVisaLibrary.list_resources() (link), and neither of those seem to be hit for some reason.

I'm having a hard time tracking this down, but it's forcing us to roll back to installing a more commonly used VISA lib, and we dislike that very much. ;)

Any thoughts as to why this might be happening?

[joshsleeper]

Hey @hgrecco,

I've researched a bit more, and it looks like highlevel.PyVisaLibrary._return_handler()(link) might never actually be called, which would explain why read and write timeouts aren't being handled properly. In fact, a comment from the docstring explicitly states TODO: THIS IS JUST COPIED PASTED FROM NIVisaLibrary.

Debugging my way through a query that times out using the standard NI Visa (and thus pyvisa.ctwrapper.highlevel.NIVisaLibrary), I believe I discovered a handful of things.

1. NIVisaLibrary._init() calls functions.set_signatures(self.lib, errcheck=self._return_handler) (link), which is what I believe is setting NIVisaLibrary._return_handler() to handle return data from NIVisaLibrary.read() and NIVisaLibrary.write(). PyVisaLibrary doesn't seem to do anything equivalent to that, which is why I think errors like timeout are getting ignored when they're clearly detected at lower levels.
   • Also, on an unrelated note, I can't figure out why you're using _init() instead of the built-in __init__(). Is there a reason for creating your own init function instead of making use of the one Python provides?
2. Even though the vxi11.ErrorCodes enum (link) has a nice (but still pared down, I believe) list of error codes to use, tcpip.TCPIPInstrSession.read() incorrectly returns all errors as StatusCode.error_io (link). This means that even when timeout errors are detected properly, they (and all other errors) are masked under the generic StatusCode.error_io label (which translates to VI_ERROR_IO in the output.)

These two things seem pretty major as far as parity with the NI Visa backend goes, and I'd love to get them resolved so we can keep using this backend ourselves.

For now, I've made the following changes to pyvisa-py files on our end that seems to get us in the right direction:

1. Added an additional check to highlevel.PyVisaLibrary.read() and highlevel.PyVisaLibrary.write() to properly handle translation (and raising!) of pyvisa error codes. The modifications look like this:

def read(self, session, count):
    """Reads data from device or interface synchronously.

    Corresponds to viRead function of the VISA library.

    :param session: Unique logical identifier to a session.
    :param count: Number of bytes to be read.
    :return: data read, return value of the library call.
    :rtype: bytes, VISAStatus
    """

    # from the session handle, dispatch to the read method of the session object.
    try:
        ret = self.sessions[session].read(count)
    except KeyError:
        return constants.StatusCode.error_invalid_object
    else:
        ret_code = constants.StatusCode(ret[1])
        if ret_code < 0:
            raise errors.VisaIOError(ret_code)

        return ret

def write(self, session, data):
    """Writes data to device or interface synchronously.

    Corresponds to viWrite function of the VISA library.

    :param session: Unique logical identifier to a session.
    :param data: data to be written.
    :type data: str
    :return: Number of bytes actually transferred, return value of the library call.
    :rtype: int, VISAStatus
    """

    # from the session handle, dispatch to the write method of the session object.
    try:
        ret = self.sessions[session].write(data)
    except KeyError:
        return constants.StatusCode.error_invalid_object
    else:
        ret_code = constants.StatusCode(ret[1])
        if ret_code < 0:
            raise errors.VisaIOError(ret_code)

        return ret

1. I've also made some preliminary changes to tcpip.TCPIPInstrSession.read() and tcpip.TCPIPInstrSession.write(), although I'm not confident this is the correct way to handle things, and even if it is they could do with being fleshed out beyond this:

def read(self, count):
    """Reads data from device or interface synchronously.

    Corresponds to viRead function of the VISA library.

    :param count: Number of bytes to be read.
    :return: data read, return value of the library call.
    :rtype: bytes, VISAStatus
    """
    if count < self.max_recv_size:
        chunk_length = count
    else:
        chunk_length = self.max_recv_size

    flags = 0
    reason = 0

    if self.get_attribute(constants.VI_ATTR_TERMCHAR_EN)[0]:
        term_char, _ = self.get_attribute(constants.VI_ATTR_TERMCHAR)
    else:
        term_char = 0

    if term_char:
        flags = vxi11.OP_FLAG_TERMCHAR_SET
        term_char = str(term_char).encode('utf-8')[0]

    read_data = bytearray()

    end_reason = vxi11.RX_END | vxi11.RX_CHR

    read_fun = self.interface.device_read

    status = SUCCESS

    while reason & end_reason == 0:
        error, reason, data = read_fun(self.link, chunk_length, self.timeout,
                                       self.lock_timeout, flags, term_char)

        if error == vxi11.ErrorCodes.io_timeout:
            return read_data, StatusCode.error_timeout
        elif error:
            return read_data, StatusCode.error_io

        read_data.extend(data)
        count -= len(data)

        if count <= 0:
            status = StatusCode.success_max_count_read
            break

        chunk_length = min(count, chunk_length)

    return bytes(read_data), status

def write(self, data):
    """Writes data to device or interface synchronously.

    Corresponds to viWrite function of the VISA library.

    :param data: data to be written.
    :type data: str
    :return: Number of bytes actually transferred, return value of the library call.
    :rtype: int, VISAStatus
    """

    send_end, _ = self.get_attribute(constants.VI_ATTR_SEND_END_EN)

    chunk_size = 1024
    try:

        if send_end:
            flags = vxi11.OP_FLAG_TERMCHAR_SET
        else:
            flags = 0

        num = len(data)

        offset = 0

        while num > 0:
            if num <= chunk_size:
                flags |= vxi11.OP_FLAG_END

            block = data[offset:offset + self.max_recv_size]

            error, size = self.interface.device_write(self.link, self.timeout,
                                                      self.lock_timeout, flags, block)

            if error == vxi11.ErrorCodes.io_timeout:
                return offset, StatusCode.error_timeout
            elif error or size < len(block):
                return offset, StatusCode.error_io

            offset += size
            num -= size

        return offset, SUCCESS
    except vxi11.Vxi11Error:
        return 0, StatusCode.error_timeout

Please feel free to correct me if I'm wrong on any point(s), since I'm by no means well versed in the code structure. I'm just someone with a vested interest in a pure-Python VISA backend and trying to make sure the problems we encounter are our own and not the packages we use. ;)

Let me know what's the best way I can help, or if there's anything else you need from me, but I'd love to see this get resolved. I've always hated having to install a full VISA lib, so pyvisa-py really is a godsend for me.

[phunter00]

I've seen this as well, thanks for the research joshsleeper!

[absheri]

I've also seen this issue.

[joshsleeper]

@hgrecco, I'll keep this one shorter than the last, but I've found a few more things that concern me.

I finally tracked down the biggest issue I was having, which was that pyvisa was freezing when using pyvisa-py as the backend when the instrument application hung or crashed while connected. This hang wasn't present when using a standard visa.dll-based library, and I think I've tracked down the problem.

From what I can tell, the underlying sockets used for rpc.RawTCPClient (link) are instantiated as blocking, which means they'll wait forever for a response. This might not be a problem if the overarching vxi11 or tcpip classes properly utilized the provided VISA timeout (usually called io_timeout in the code, I believe), but they don't seem to make use of those values either. That means that if the instrument stops responding to the socket, lines like header = sock.recv(4) in rpc.recvfrag (link) will just hang forever until the socket is forcefully closed on one end.

There are a number of different ways this could be solved, but I'm not sure the pyvisa community on GitHub is very active, so I don't know if I'll get much feedback. These issues are major blockers for me, but I'd love to work with you to help resolve them if at all possible.

[MatthieuDartiailh]

Sadly @hgrecco does not seem to be very active lately. I use and develop using PyVISA but I am not very familiar with pyvisa-py, nonetheless I will be happy to review any patch you propose before @hgrecco gets back online.

[joshsleeper]

That's great to hear @MatthieuDartiailh, thanks for the support. I'll probably wait a little bit longer to hopefully get a response from @hgrecco, but otherwise I may just start working on a patch (or maybe a fork, in the meantime) myself.

[MatthieuDartiailh]

I think the most productive way to go will be to fork and open a pull request. It will also ease the reviewing process.

[joshsleeper]

Mmk, after playing with it for another week I think I've got all the bugs worked out and it seems to act as much like NI Visa as I would expect now. Appreciate any review people have the time to do!

https://github.com/hgrecco/pyvisa-py/pull/85

[hgrecco]

Thanks all for the work and support! I have been absent for a while but I am back. Indeed forking and issuing a pull request is the best way.

[hgrecco]

The problem is homogenizing the way it works so that all interfaces can interoperate with pyvisa (replacing properly visa.dll). We did this by selecting the minimum common characteristics of each interface (which is inefficient) But this help us to define better the boundary between pyvisa and the backends (which is a good thing). But now that we have established that (at least in a basic manner), I think we should move forward talking to each interface in the best possible way.

[MatthieuDartiailh]

Closed by #100