Test suite and BFs for Tektronix TDS5xx

[trappitsch]

Test suite with full coverage

• Tests for get_hardcopy routine is a bit creative, but couldn't
  find any manual entry for what is done there.

Bug fixes:

• Typos
• map(float, list_of_string) not allowed anymore, switched to directly
  transferring to np.array with specific dtype.
• flush_input in read_waveform replaced with reading one character.
  Manual states that newline character is present after binary block.
• Enums called with name instead of value -> fixed, now called properly
• get_hardcopy routine tried to unpack read data. However, the
  read routine already decodes it using "utf-8" as the default
  encoding already. Thus: need to encode again before using
  struct.unpack to unpack them.
• Class had import time and from time import sleep and used sleep
  and time.sleep. Used import time, which is useful to mock out
  sleep time.

[coveralls]

Coverage increased (+1.03%) to 93.724% when pulling 4f5d6d0 on trappitsch:tests_Tektronix_TDS5xx into c9ffd6e on Galvant:master.

[trappitsch]

The get_hardcopy routine is weird to me. I can't seem to find it in the programmers manual. Furthermore, looking up header length for bmp files, which I think should be transferred here, does not show a "standard" header length of 54, as is assumed here.
The test here is basically stiched together such that the routine is tested...

[scasagrande]

I dug through the archives and here is where it was originally merged in: #29

Obviously the comments are way out of date, but it looks like it was the first PR submitted by someone that I didn't already know. I completely forgot about that!

[trappitsch]

I dug through the archives and here is where it was originally merged in: #29

Thanks @scasagrande that helps a lot. Now I know at least where this split between header and data comes from in the get_hardcopy routine. The test is not very fancy, but I think it should do the trick for what we want to ensure here. Don't have any of those Tektronix scopes around, but would be interesting to test.

One more note: I've seen the issue before (and it's coming up in the uncovered Keithley classes) that a query expected to get binary data back. The read routine it goes to in abstract_comm.py however has a decoding in it:

return self.read_raw(size).decode(encoding)

I assume this is something that was later added to ik as well and was not there originally? Does this justify that we simply have to encode reads again to make it compatible, as I've done above? As mentioned, this issue is about to come up in the missing tests for some Keithleys (that's the reason they are still missing).

[scasagrande]

Yeah, we originally didn't differentiate between reading bytes and reading unicode. Part of that was due to ignorance, part was due to Python 2's handling of strings.

So originally, everything would just use self.read(...) to get data. Then, I believe around the time I was moving towards Py2/3 dual compatibility, it was split into self.read(...) which was expected to return unicode, and self.read_raw(...) for any calls that just need access to the raw bytes. This way we try to maintain a boundary at the edge of the api: anything that is human readable should be in unicode, anything that just represents 0s and 1s should be bytes, and only converting at the edge of our software.

[trappitsch]

Thanks for the explanation, this really helps. I switched the decode / encode things over to self._file.read_raw, since (1) I don't know what data is in fact transferred and (2) the lengths and everything is already in place. So this I think is a significantly better fix for the issue, i.e., is basically the original implementation adjusted to the changes over time in ik.

Now the test is also better, using hypothesis now to create binary data of random length, then stitch it together with a header to agree with the format and length of the header that is expected in the original code. I think this is as good as it will get w/o the hardware.

[scasagrande]

lgtm