search

pyvisa / pyvisa-py

A pure python PyVISA backend
https://pyvisa-py.readthedocs.io
MIT License
282 stars 120 forks source link

rpc: fix behavior after a normal timeout #173

Closed MatthieuDartiailh closed 5 years ago

MatthieuDartiailh commented 5 years ago

In the absence of any answer from the instrument we can assume that actually the instrument did not acknowledge the communication as valid. In which case the xid should not be incremented.

I also removed the offset on the timeout that were used before we properly implement the timeout mechanism.

@ldpgh could you please check this fixes the issue you are seeing ?

ldpgh commented 5 years ago

Thx for the change. But it needs more time to get my hands on the measurement equipment to check it.

MatthieuDartiailh commented 5 years ago

@ldpgh I tested locally on a Tektronix oscilloscope and with the last commit everything seems fine. If you can confirm it would be great. @tivek @hgrecco if one of you can review and test it would be great.

MatthieuDartiailh commented 5 years ago

ping @tivek @hgrecco @ldpgh

hgrecco commented 5 years ago

I think the PR looks good. I just had two questions while I was going through it.

  1. From the design point of view, it seems that decrementing could be done in the implementation of make_call within an except branch. In that way, _recvrecord would be client agnostic.
  2. RawBroadcastUDPClient.make_call might require this type of decrementing as well.
MatthieuDartiailh commented 5 years ago

Actually I should probably elaborate on that and probably revert that part of the changes.

As mentioned in one comment, in the VXI-11 protocol the instrument is supposed to send a message back if it cannot answer to a query in the imparted time. If it does not happen, it means something is terribly wrong with the communication. As a consequence, we need to be able to timeout to avoid deadlock but we should let the instrument time enough to signal a normal timeout. If a normal timeout happen (with an answer), there is no issue with the xid. In the case of a hard timeout, the issue is deeper (a disconnect cable) (which is why I turned the error into an io_error), and there is no real reason we should be able to recover easily from that.

I will try to remove the xid change and test again.

MatthieuDartiailh commented 5 years ago

I reverted the changes to xid and tested anew. This fixes the error after a normal timeout (where the instrument server answers) and does not concern itself with hard timeout which have a more complex cause.

MatthieuDartiailh commented 5 years ago

ping @hgrecco @tivek @ldpgh

MatthieuDartiailh commented 5 years ago

Local testing passed and this is a definitive improvement so merging.

bors r+

bors[bot] commented 5 years ago

Build succeeded