Tests and BFs for Keithley 195 #262
Conversation
Full coverage test suite BFs: - Transfer queried values to bytes before using `struct.unpack`. - Do subsequent comparisons on bytes as well. - Remove `if self._testing` statement to skip a sleep time. In the test, mock `time.sleep` for speedup and ensure it was called with a 2 seconds argument.
instruments/keithley/keithley195.py
Outdated
| @@ -301,18 +300,19 @@ def parse_status_word(statusword): # pylint: disable=too-many-locals | |||
|
|
|||
| (trigger, function, input_range, eoi, buf, rate, srqmode, relative, | |||
| delay, multiplex, selftest, data_fmt, data_ctrl, filter_mode, | |||
| terminator) = struct.unpack('@4c2s3c2s5c2s', statusword[4:]) | |||
| terminator) = struct.unpack('@4c2s3c2s5c2s', bytes(statusword[4:], | |||
There was a problem hiding this comment.
Keithley does answer with mostly integers according to the manual, those can be decocded with "utf-8". For the unpacking later, need to put them into bytes again. This is analog to how the keithley485.py handles the same issue on line 362.
The statusword is well defined, thus read_raw could be used. However, if an error is returned by the instrument, we will have stuff left in the buffer, thus making query and re-encoding for subsequent unpacking the preferred method.
There was a problem hiding this comment.
Are you saying that in get_status_word we can end in a condition where we haven't cleared out the buffer if we use read_raw?
There was a problem hiding this comment.
When read_raw is called with size=-1, it should read until a terminator is encountered, is that correct? If so, that would actually work as well and would be the preferred method. Sorry, I might have mixed this up with binblockread from looking at all the scopes.
I'm worried to read back a given size - if the instrument has a failure mode (seems like they tend to have those from your comment below) the size of what it would send might be different, and then get things into weird states. Of course, this always assumes that weird states send termination characters...
There was a problem hiding this comment.
Yes read_raw with size=-1 will go until the termination character is found. In fact, read does not check for termination characters, it is only the impl of read_raw that does this.
There was a problem hiding this comment.
That makes a lot more sense, thanks for the clarification @scasagrande!
I changed it such that the status word now gets read using read_raw, thus it does not need to be encoded with bytes afterwards.
To use read raw: I'm now first sending the command with sendcmd and then get the response with read_raw. Is this the preferred way? query basically wraps the sendcmd and read into one, but I guess there's no query_raw equivalent?
There was a problem hiding this comment.
That seems correct to me. Perhaps something like that can be added. Or, we could add support for something like encoding=None which would mean to not encode the returned bytes. Not sure how I feel about that later suggestion though, since we would be changing return type.
Anyways that's a conversation for another time, but your current version is what I would have done.
|
I actually have a 195, and I will say that its a pain to get working. If you don't do everything perfectly, it gets into a strange state and you have to power cycle it. At least, that's what I had to do. |
Changed the read to use `read_raw` to read the instrument. Thus, no need to re-encode the status word using `bytes`.
|
lgtm |
Full coverage test suite
BFs:
struct.unpack.if self._testingstatement to skip a sleep time. In the test,mock
time.sleepfor speedup and ensure it was called witha 2 seconds argument.