Set minimum version of firmware for Rigol DS4000 #1206
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1206 +/- ##
=======================================
Coverage 80.16% 80.16%
=======================================
Files 49 49
Lines 6762 6762
=======================================
Hits 5421 5421
Misses 1341 1341 |
| idn = self.get_idn() | ||
| ver = [int(x) for x in idn['firmware'].split('.')] | ||
| if ver < [0,2,3]: | ||
| raise RuntimeError('Firmware version must be at least 00.02.03') |
There was a problem hiding this comment.
Please add a clear explanation to the exception message about why the firmware needs to be at least that.
| idn = self.get_idn() | ||
| ver = [int(x) for x in idn['firmware'].split('.')] | ||
| if ver < [0,2,3]: | ||
| raise RuntimeError('Firmware version must be at least 00.02.03') |
There was a problem hiding this comment.
also, do you receive corrupted data all the time? as in, is the scope still usable to some extent with 00.02.02 and lower or not? if it is, then please change raise RuntimeError to a waning warnings.warn, because the users should not get suddenly blocked from using the driver/instrument.
There was a problem hiding this comment.
Also, I just trust you that it is indeed the firmware problem, and not some minor error in the way you are using the instrument. Nevertheless, could you please double check that you are doing things in the right way?
There was a problem hiding this comment.
Sincerely I don't know how extended is the breakage. I just noticed that the function to get the raw buffer was broken and got working after an upgrade. Also now I don't have a way anymore to check with the old version. Since it's easily upgredable, I would not waste time to try to use an old version
There was a problem hiding this comment.
ok. but still let's keep it safe. So:
- make a warning instead of error
- add more info to the text of the warning explaining what is wrong with an older firmware
| #Require version 00.02.03 | ||
|
|
||
| idn = self.get_idn() | ||
| ver = [int(x) for x in idn['firmware'].split('.')] |
There was a problem hiding this comment.
is firmware version always a three-number string #.#.#?
you do not have to reinvent the wheel here, just use from distutils.version import LooseVersion (example here or here or even in QCoDeS) .
Another thing you could use is from pkg_resources import parse_version function (examples here and here) but it is not included in the standard library of python.
There was a problem hiding this comment.
Ah good, I was looking for something similar, but I didn't found it, thanks!
|
@astafan8 updated as requested |
| idn = self.get_idn() | ||
| ver = LooseVersion(idn['firmware']) | ||
| if ver < LooseVersion('00.02.03'): | ||
| warnings.warn('Firmware version should be at least 00.02.03, data transfert may not work correctly') |
There was a problem hiding this comment.
typo: transfer (without t) :) fix it and then I'll merge
There was a problem hiding this comment.
The line is also slightly too long. If we really want to get nitty-gritty (and we do!)
|
@YakBizzarro thank you! |
With firmware version 00.02.02, data transfer is not working correctly when accessing the raw buffer (we receive corrupted data). This commit check that the scope is running at least version 00.02.03 of the firmware.
@astafan8