Driver: Keysight 344xxA - flexible measurements and modularization #1433

astafan8 · 2019-01-04T20:17:53Z

Improve old Keysight 344xxA DMM (digital multimeter) driver by adding methods and parameters that allow to perform measurement more flexibly than the existing data_buffer ArrayParameter implements. Also, add a new version Keysight 344xxA DMM drivers where parameters are logically grouped in submodules whenever possible.

New driver:

uses submodules for traiggering/sampling/display parts
does not have data_buffer ArrayParameter
tests were ported

Old driver:

Deprecate data_buffer ArrayParameter
Add many useful missing methods and parameters, like read and fetch

Both drivers:

Add example notebook for the drivers with an example of measurement routines
Extract error handling to a common mixin class KeysightErrorQueueMixin that is used in 4 Keysight drivers
Add many more docstrings
Add MIN/MAX/DEF/INF values to parameters that didn't have them
Fix a number of cases where a particular model has or doesn't have a particular parameter or a value of a parameter

Replaces and Closes #1393.

(cherry picked from commit 24dc67f) (that commit has incorrect cast for 'reset' function which is taken care of here)

it autoranges and then sets autoranging off.

…nt' into driver/34465A/flexible_measurement

astafan8 · 2019-01-07T17:03:43Z

@QCoDeS/core ready for review

…nt' into driver/34465A/flexible_measurement

..introduced couple commits ago

jenshnielsen · 2019-01-08T10:05:44Z

qcodes/instrument_drivers/Keysight/Keysight_33500B.py

@@ -36,21 +36,6 @@ def setcmd(channel, setting):
        def getcmd(channel, setting):
            return 'SOURce{}:'.format(channel) + setting + '?'

-        def errorparser(rawmssg):


Is it not the time to just remove this driver?

~~ok, i'll remove it.~~

I won't remove it because it requires updating some example notebooks, i'd prefer to do that in a different PR.

jenshnielsen · 2019-01-08T10:05:59Z

qcodes/instrument_drivers/Keysight/Keysight_33500B_channels.py

@@ -253,21 +256,6 @@ def __init__(self, name, address, silent=False, **kwargs):

        super().__init__(name, address, **kwargs)

-        def errorparser(rawmssg):


Same as above

~~ok, i'll remove it.~~

I won't remove it because it requires updating some example notebooks, i'd prefer to do that in a different PR.

jenshnielsen

Only made it part way through the review so far, Will get back to it asap

Should these tests be ported https://github.com/QCoDeS/Qcodes/blob/master/qcodes/tests/drivers/test_keysight_34465a.py

jenshnielsen · 2019-01-08T10:08:14Z

qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py

+    def __init__(self, parent: '_Keysight_344xxA', name: str, **kwargs):
+        super(Trigger, self).__init__(parent, name, **kwargs)
+
+        if self.parent.model in ['34465A', '34470A']:


Would it be better if all the magic constants were defined in one place. In the root instrument?

yes, definitely!

also, tests have been ported.

this avoids reference to '34465A', '34470A' in the submodules.

astafan8 · 2019-01-08T11:21:19Z

@QCoDeS/core please feel free to continue reviewing :)

jenshnielsen · 2019-01-08T11:28:10Z

Dave pointed out that the DIG option is now included by default in the latest fw of at least some of the models (34460A/34461A/34465A/34470A) https://www.keysight.com/main/software.jspx?ckey=2367633&lc=eng&cc=US&nid=-32051.1242959&id=2367633

Should we mention that somewhere in the docs of the new driver?

See https://www.keysight.com/upload/cmc_upload/All/Firmware_3.0_Release_Notes.pdf

jenshnielsen · 2019-01-08T13:28:55Z

qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py

+from qcodes.instrument_drivers.Keysight.private.error_handling import \
+    KeysightErrorQueueMixin
+
+log = logging.getLogger(__name__)


Is this log ever used. I could not see it used anywhere

indeed. will remove.

jenshnielsen

Apart from the small issues pointed out this looks good. I have tested both the 335xx and 34xxx drivers with the new changes and they seem to work correctly

jenshnielsen · 2019-01-08T13:35:09Z

qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py

+        Create an instance of the instrument.
+
+        Args:
+            name (str): Name used by QCoDeS. Appears in the DataSet


is it on purpose that the types are still here and not as type hints in the signature?

definitely not. i'll clean this up.

astafan8 · 2019-01-08T14:03:28Z

@jenshnielsen Thanks for testing! I'll finish the minor things and merge after CI is done.

…nt' into driver/34465A/flexible_measurement

Merge: ecf286c 24af221 Author: Mikhail Astafev <astafan8@gmail.com> Merge pull request #1433 from astafan8/driver/34465A/flexible_measurement

astafan8 added 30 commits January 4, 2019 14:01

Add missing space to parameter docstring

02b68a1

Add seconds as unit for trigger_delay parameter

95b0ccc

Add docstrings for NPLC, display_text, autozero, trigger_delay params

99c82b1

Add get parser to display_text parameter to remove technical quotes

844334b

Recast add_function in Keysight_344xxA drivers

40f8379

(cherry picked from commit 24dc67f) (that commit has incorrect cast for 'reset' function which is taken care of here)

Update display_text value when calling display_clear

d3f36e1

Add display_enabled parameter to control display on/off state

435c142

Add _fetch method, FETCh? waits for completion and retrieves data

b893d22

CLarify display_clear docstring

e8f44fa

Clarify init_measurement docstring with extra info

64dd967

Extract parsing retrieved data into a function _raw_vals_to_array

009420c

Add _read method for READ? command

ba208f5

Add docstring to sample_timer parameter

b76942b

Add docstring to sample_timer_minimum parameter

eb6c060

Correct label of sample_timer_minimum parameter to mention "recommended"

0c8caec

Rename _fetch method to fetch

89219b6

Rename _read method to read

fb301d6

Deprecate data_buffer ArrayParameter via its prepare method

b9683ed

Make DIG boolean instance attribute of driver

a623777

data_buffer now works even if instrument does not have DIG

0601323

CLarify docstrings for error and flush_error_queue methods

6104cd7

Add autorange once method for DC voltage measurements

eec3d68

it autoranges and then sets autoranging off.

Set maximum value for sample_count parameter depending on the model

bbfa7b4

Add dcostring for sample_count parameter

ee64dcf

Add docstring for aperture_mode parameter

8c9ce64

Add docstring for aperture_time parameter

8562b10

sample_source parameter is only available for 34465A, 34470A models

3cafeeb

Fix line length for sample_count_pretrigger parameter definition

fef45fb

Unify formatting within __init__

23480bb

Clarify section name

7aa1a89

Merge remote-tracking branch 'origin/driver/34465A/flexible_measureme…

9ac208f

…nt' into driver/34465A/flexible_measurement

astafan8 changed the title ~~[WIP] Driver: Keysight 344xxA - flexible measurements~~ Driver: Keysight 344xxA - flexible measurements and modularization Jan 7, 2019

astafan8 added 6 commits January 7, 2019 18:03

Merge branch 'master' into driver/34465A/flexible_measurement

9b66648

Add AbstractInstrument ABC to allow clean instrument mixins definition

38217fe

Merge remote-tracking branch 'origin/driver/34465A/flexible_measureme…

261c557

…nt' into driver/34465A/flexible_measurement

Make mypy strict_optional==True happy by removing None value from ABC

8a267e4

Fix bug with undefined symbol in type hint

8f7452c

Fix import bug of function that ended up in "if TYPE_CHECKING"

0f864ca

..introduced couple commits ago

jenshnielsen reviewed Jan 8, 2019

View reviewed changes

astafan8 added 4 commits January 8, 2019 12:00

Test new Keysight 34456A driver (with submodules), not old one

c333129

Add test for Keysight 34465A display text parameter

0b818e2

Add test for DIG option for Keysight 34456A

6d4c0d7

Extract is_34465A_34470A attribute in Keysight 344xxA driver

a36b079

this avoids reference to '34465A', '34470A' in the submodules.

Note that DIG is included in 3.0 firmware for Keysight 34465A/34470A

daf9f97

See https://www.keysight.com/upload/cmc_upload/All/Firmware_3.0_Release_Notes.pdf

jenshnielsen reviewed Jan 8, 2019

View reviewed changes

Remove unused module-level log of Keysight 344xxA submodules driver

0c76093

jenshnielsen approved these changes Jan 8, 2019

View reviewed changes

jenshnielsen mentioned this pull request Jan 8, 2019

Remove deprecated keysight waveform drivers drivers #1435

Merged

jenshnielsen and others added 3 commits January 8, 2019 16:19

Merge branch 'master' into driver/34465A/flexible_measurement

0eabad3

Remove type hints in docstrings of Keysight 344xxA driver

a2f3794

Merge remote-tracking branch 'origin/driver/34465A/flexible_measureme…

24af221

…nt' into driver/34465A/flexible_measurement

astafan8 merged commit cb51b81 into microsoft:master Jan 8, 2019

astafan8 deleted the driver/34465A/flexible_measurement branch January 8, 2019 17:43

giulioungaretti pushed a commit that referenced this pull request Jan 8, 2019

Generated gh-pages for commit cb51b81

e6a5065

Merge: ecf286c 24af221 Author: Mikhail Astafev <astafan8@gmail.com> Merge pull request #1433 from astafan8/driver/34465A/flexible_measurement

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Driver: Keysight 344xxA - flexible measurements and modularization #1433

Driver: Keysight 344xxA - flexible measurements and modularization #1433

astafan8 commented Jan 4, 2019 •

edited

astafan8 commented Jan 7, 2019

jenshnielsen Jan 8, 2019

astafan8 Jan 8, 2019 •

edited

astafan8 Jan 8, 2019

jenshnielsen Jan 8, 2019

astafan8 Jan 8, 2019 •

edited

astafan8 Jan 8, 2019

jenshnielsen left a comment

jenshnielsen Jan 8, 2019

astafan8 Jan 8, 2019

astafan8 Jan 8, 2019

astafan8 commented Jan 8, 2019

jenshnielsen commented Jan 8, 2019

jenshnielsen Jan 8, 2019

astafan8 Jan 8, 2019

jenshnielsen left a comment

jenshnielsen Jan 8, 2019

astafan8 Jan 8, 2019

astafan8 commented Jan 8, 2019

		@@ -253,21 +256,6 @@ def __init__(self, name, address, silent=False, **kwargs):

		super().__init__(name, address, **kwargs)

		def errorparser(rawmssg):

Driver: Keysight 344xxA - flexible measurements and modularization #1433

Driver: Keysight 344xxA - flexible measurements and modularization #1433

Conversation

astafan8 commented Jan 4, 2019 • edited

astafan8 commented Jan 7, 2019

Choose a reason for hiding this comment

astafan8 Jan 8, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astafan8 Jan 8, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenshnielsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astafan8 commented Jan 8, 2019

jenshnielsen commented Jan 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenshnielsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astafan8 commented Jan 8, 2019

astafan8 commented Jan 4, 2019 •

edited

astafan8 Jan 8, 2019 •

edited

astafan8 Jan 8, 2019 •

edited