Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPInstrument._send() could not send all its data #748

Closed
nyxus opened this issue Sep 18, 2017 · 1 comment · Fixed by #719
Closed

IPInstrument._send() could not send all its data #748

nyxus opened this issue Sep 18, 2017 · 1 comment · Fixed by #719

Comments

@nyxus
Copy link
Contributor

nyxus commented Sep 18, 2017

I was browsing the instruments code and found out that in the IPInstrument class the socket.send(...) function is implemented wrongly:

    def _send(self, cmd):
        data = cmd + self._terminator
        self._socket.send(data.encode())

This is what the documentation says about socket.send(...):

socket.send(bytes[, flags])
Send data to the socket. The socket must be connected to a remote socket. The optional flags argument has the same meaning as for recv() above. Returns the number of bytes sent. Applications are responsible for checking that all data has been sent; if only some of the data was transmitted, the application needs to attempt delivery of the remaining data.
https://docs.python.org/3.6/library/socket.html

At this moment, if send(...) fails, only a part of the message is transmitted. Which will create strange bugs.

A better solution is to use socket.sendall(...) or as the example shows:

    def mysend(self, msg):
        totalsent = 0
        while totalsent < MSGLEN:
            sent = self.sock.send(msg[totalsent:])
            if sent == 0:
                raise RuntimeError("socket connection broken")
            totalsent = totalsent + sent

https://docs.python.org/3.6/howto/sockets.html#socket-howto

jenshnielsen referenced this issue in jenshnielsen/Qcodes Sep 19, 2017
@jenshnielsen
Copy link
Collaborator

Thanks for the report. I agree this is obviously wrong. Fixed in #719

WilliamHPNielsen pushed a commit that referenced this issue Sep 19, 2017
* Fix: ip instrument log empty responce

As I understand it that is a sign of a closed socket

This should probably be an error in the future

* Use sendall fixes #748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants