diff --git a/docs/changes/newsfragments/8694.breaking b/docs/changes/newsfragments/8694.breaking new file mode 100644 index 000000000000..1b58992d9f1e --- /dev/null +++ b/docs/changes/newsfragments/8694.breaking @@ -0,0 +1,7 @@ +The ``auto_zero_once`` parameter of ``Keithley7510Sense`` and the ``trigger_in_ext_clear`` +parameter of ``Keithley7510`` have been replaced by methods of the same name. Both were +command-only parameters, so calling them without an argument, e.g. +``dmm.sense.auto_zero_once()``, only returned the cached value and did not send anything to the +instrument. Calling them like this now sends the command. Code that passed a value to them, e.g. +``dmm.trigger_in_ext_clear(None)``, must drop the argument, and they are no longer part of the +instrument's parameters or snapshot. diff --git a/docs/changes/newsfragments/8694.breaking.1 b/docs/changes/newsfragments/8694.breaking.1 new file mode 100644 index 000000000000..fbb170cfec51 --- /dev/null +++ b/docs/changes/newsfragments/8694.breaking.1 @@ -0,0 +1,7 @@ +The ``trigger_count`` parameter of ``Keithley6500``, which sets the number of times a scan is +repeated (``ROUT:SCAN:COUN:SCAN``), now accepts the full range documented in the DMM6500 +Reference Manual: 0 to 100,000,000, where 0 repeats the scan until it is aborted. It previously +only accepted 1 to 9999 or the strings ``"inf"``, ``"default"``, ``"minimum"`` and ``"maximum"``, +which were copied from the Keithley 2000 and are not documented for this command. These strings +are now rejected; use 0 instead of ``"inf"``. Whole-number floats are accepted and sent as +integers, and the parameter is annotated as ``Parameter[int, Self]``. diff --git a/docs/changes/newsfragments/8694.improved b/docs/changes/newsfragments/8694.improved new file mode 100644 index 000000000000..005de4ee6cf4 --- /dev/null +++ b/docs/changes/newsfragments/8694.improved @@ -0,0 +1,5 @@ +The "Creating Instrument Drivers" example notebook now explains how to annotate a parameter with +the type of its value, e.g. ``Parameter[float, Self]``, and all of its example drivers are +annotated this way. This also fixed bugs in the example code: the Ithaco example passed ``units`` +instead of ``unit``, the Keithley 2600 example lacked its ``model`` attribute and +``_display_settext`` method, and the Alazar example used outdated ``TraceParameter`` arguments. diff --git a/docs/changes/newsfragments/8694.improved_driver b/docs/changes/newsfragments/8694.improved_driver new file mode 100644 index 000000000000..2503cb1a85a8 --- /dev/null +++ b/docs/changes/newsfragments/8694.improved_driver @@ -0,0 +1,4 @@ +The parameters of the Basel SP983, SP983a and SP983c, Ithaco 1211, Keithley 2450, DMM7510, S46 +and 2600 drivers that were not yet typed are now annotated with the type of their value, e.g. +``Parameter[float, Self]``, so static type checkers can infer the type returned by ``get`` and +accepted by ``set``. diff --git a/docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb b/docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb index b6da538bf6bf..c6f5faa07345 100644 --- a/docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb +++ b/docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb @@ -18,7 +18,7 @@ "import ctypes # only for DLL-based instrument\n", "import sys\n", "import time\n", - "from typing import TYPE_CHECKING, Any\n", + "from typing import TYPE_CHECKING, Any, Self\n", "\n", "if TYPE_CHECKING:\n", " from typing import Unpack\n", @@ -34,7 +34,7 @@ " VisaInstrumentKWArgs,\n", ")\n", "from qcodes.instrument_drivers.AlazarTech.utils import TraceParameter\n", - "from qcodes.parameters import ManualParameter, MultiParameter, Parameter" + "from qcodes.parameters import MultiParameter, Parameter, create_on_off_val_mapping" ] }, { @@ -96,6 +96,23 @@ "\n", "Note that it is also possible to directly create a parameter and assign it as an attribute. We do not recommend this, since you can easily forget that you are required to pass `instrument=self` to the parameter for the parameter to work correctly.\n", "\n", + "### Annotating the type of a Parameter\n", + "\n", + "`Parameter` is generic in two types: the type of the value that `get` returns and `set` accepts, and the type of the instrument that the parameter belongs to. Annotate a parameter as `Parameter[, Self]`, where `Self` (from `typing`) refers to the instrument class being defined. Static type checkers such as Pyright and Mypy, as well as IDEs, can then infer that e.g. `my_instrument.frequency()` returns a `float`, and flag `my_instrument.frequency(\"high\")` as an error.\n", + "\n", + "```python\n", + "self.frequency: Parameter[float, Self] = self.add_parameter(\n", + " \"frequency\", get_cmd=\"FREQ?\", set_cmd=\"FREQ {}\", get_parser=float, unit=\"Hz\"\n", + ")\n", + "```\n", + "\n", + "The value type is not checked at runtime, so it is up to you to make sure that it matches what the parameter actually returns:\n", + "\n", + "- A VISA instrument returns strings, so use a `get_parser` such as `float` or `int` for numeric parameters. Without one, the value type is `str`.\n", + "- With a `val_mapping`, the value type is the type of its keys, e.g. `bool` when using `create_on_off_val_mapping` (see On/Off parameters below).\n", + "- Following the golden rule above, the type should describe both what `get` returns and what `set` accepts.\n", + "\n", + "A parameter annotated as a bare `Parameter` has the value type `Any`, which turns off type checking of its values. The example drivers below instead specify explicit value and instrument types.\n", "\n", "### Functions\n", "\n", @@ -228,7 +245,7 @@ " ):\n", " super().__init__(name, address, **kwargs)\n", "\n", - " self.attenuation = self.add_parameter(\n", + " self.attenuation: Parameter[float, Self] = self.add_parameter(\n", " \"attenuation\",\n", " unit=\"dB\",\n", " # the value you set will be inserted in this command with\n", @@ -286,7 +303,7 @@ "metadata": {}, "outputs": [], "source": [ - "class KeithleyChannel(InstrumentChannel):\n", + "class KeithleyChannel(InstrumentChannel[\"Keithley2600\"]):\n", " \"\"\"\n", " Class to hold the two Keithley channels, i.e.\n", " SMUA and SMUB.\n", @@ -294,7 +311,7 @@ "\n", " def __init__(\n", " self,\n", - " parent: Instrument,\n", + " parent: \"Keithley2600\",\n", " name: str,\n", " channel: str,\n", " **kwargs: \"Unpack[InstrumentBaseKWArgs]\",\n", @@ -316,7 +333,7 @@ " super().__init__(parent, name, **kwargs)\n", " self.model = self._parent.model\n", "\n", - " self.volt = self.add_parameter(\n", + " self.volt: Parameter[float, Self] = self.add_parameter(\n", " \"volt\",\n", " get_cmd=f\"{channel}.measure.v()\",\n", " get_parser=float,\n", @@ -329,7 +346,7 @@ " )\n", " \"\"\"Parameter volt\"\"\"\n", "\n", - " self.curr = self.add_parameter(\n", + " self.curr: Parameter[float, Self] = self.add_parameter(\n", " \"curr\",\n", " get_cmd=f\"{channel}.measure.i()\",\n", " get_parser=float,\n", @@ -339,7 +356,8 @@ " )\n", " \"\"\"Parameter curr\"\"\"\n", "\n", - " self.mode = self.add_parameter(\n", + " # with a val_mapping, the value type is the type of its keys\n", + " self.mode: Parameter[str, Self] = self.add_parameter(\n", " \"mode\",\n", " get_cmd=f\"{channel}.source.func\",\n", " get_parser=float,\n", @@ -349,16 +367,16 @@ " )\n", " \"\"\"Parameter mode\"\"\"\n", "\n", - " self.output = self.add_parameter(\n", + " self.output: Parameter[bool, Self] = self.add_parameter(\n", " \"output\",\n", " get_cmd=f\"{channel}.source.output\",\n", " get_parser=float,\n", " set_cmd=f\"{channel}.source.output={{:d}}\",\n", - " val_mapping={\"on\": 1, \"off\": 0},\n", + " val_mapping=create_on_off_val_mapping(on_val=1, off_val=0),\n", " )\n", " \"\"\"Parameter output\"\"\"\n", "\n", - " self.nplc = self.add_parameter(\n", + " self.nplc: Parameter[float, Self] = self.add_parameter(\n", " \"nplc\",\n", " label=\"Number of power line cycles\",\n", " set_cmd=f\"{channel}.measure.nplc={{:.4f}}\",\n", @@ -407,6 +425,7 @@ " kmstring = (\"{}, \" * (len(knownmodels) - 1)).format(*knownmodels[:-1])\n", " kmstring += f\"and {knownmodels[-1]}.\"\n", " raise ValueError(\"Unknown model. Known model are: \" + kmstring)\n", + " self.model = model\n", "\n", " # Add the channel to the instrument\n", " self.smua = self.add_submodule(\"smua\", KeithleyChannel(self, \"smua\", \"smua\"))\n", @@ -417,14 +436,17 @@ " # display parameter\n", " # Parameters NOT specific to a channel still belong on the Instrument object\n", " # In this case, the Parameter controls the text on the display\n", - " self.display_settext = Parameter(\n", + " self.display_settext: Parameter[str, Self] = self.add_parameter(\n", " \"display_settext\",\n", " set_cmd=self._display_settext,\n", " vals=vals.Strings(),\n", - " instrument=self,\n", " )\n", + " \"\"\"Parameter display_settext\"\"\"\n", + "\n", + " self.connect_message()\n", "\n", - " self.connect_message()" + " def _display_settext(self, text: str) -> None:\n", + " self.visa_handle.write(f'display.settext(\"{text}\")')" ] }, { @@ -457,10 +479,10 @@ "\n", " def __init__(\n", " self,\n", - " name,\n", - " system_id=1,\n", - " board_id=1,\n", - " dll_path=None,\n", + " name: str,\n", + " system_id: int = 1,\n", + " board_id: int = 1,\n", + " dll_path: str | None = None,\n", " **kwargs: \"Unpack[InstrumentBaseKWArgs]\",\n", " ):\n", " super().__init__(name, **kwargs)\n", @@ -474,24 +496,25 @@ " f\"AlazarTech_ATS not found at system {system_id}, board {board_id}\"\n", " )\n", "\n", - " self.buffer_list = []\n", + " self.buffer_list: list[Any] = []\n", "\n", " # the Alazar driver includes its own parameter class to hold values\n", " # until later config is called, and warn if you try to read a value\n", " # that hasn't been sent to config.\n", - " self.clock_source: TraceParameter = self.add_parameter(\n", + " self.clock_source: TraceParameter[str] = self.add_parameter(\n", " name=\"clock_source\",\n", " parameter_class=TraceParameter,\n", " label=\"Clock Source\",\n", " unit=None,\n", - " value=\"INTERNAL_CLOCK\",\n", - " byte_to_value_dict={\n", - " 1: \"INTERNAL_CLOCK\",\n", - " 4: \"SLOW_EXTERNAL_CLOCK\",\n", - " 5: \"EXTERNAL_CLOCK_AC\",\n", - " 7: \"EXTERNAL_CLOCK_10MHz_REF\",\n", + " initial_value=\"INTERNAL_CLOCK\",\n", + " val_mapping={\n", + " \"INTERNAL_CLOCK\": 1,\n", + " \"SLOW_EXTERNAL_CLOCK\": 4,\n", + " \"EXTERNAL_CLOCK_AC\": 5,\n", + " \"EXTERNAL_CLOCK_10MHz_REF\": 7,\n", " },\n", " )\n", + " \"\"\"Parameter clock_source\"\"\"\n", "\n", " # etc..." ] @@ -517,7 +540,12 @@ "class SomeDLLInstrument(Instrument):\n", " dll_path = \"C:\\\\WINDOWS\\\\System32\\\\ATSApi\"\n", "\n", - " def __init__(self, name, dll_path=None, **kwargs: \"Unpack[InstrumentBaseKWArgs]\"):\n", + " def __init__(\n", + " self,\n", + " name: str,\n", + " dll_path: str | None = None,\n", + " **kwargs: \"Unpack[InstrumentBaseKWArgs]\",\n", + " ):\n", " super().__init__(name, **kwargs)\n", "\n", " if sys.platform != \"win32\":\n", @@ -535,7 +563,7 @@ "metadata": {}, "source": [ "## Manual instruments\n", - "A totally manual instrument (like the ithaco 1211) will contain only `ManualParameter`s. Some instruments may have a mix of manual and standard parameters. Here we also define a new `CurrentParameter` class that uses the ithaco parameters to convert a measured voltage to a current. When subclassing a parameter class (`Parameter`, `MultiParameter`, ...), the functions for setting and getting should be called `get_raw` and `set_raw`, respectively." + "A totally manual instrument (like the ithaco 1211) will contain only manual parameters, i.e. parameters with `get_cmd=None` and `set_cmd=None` that only store the value last set. Some instruments may have a mix of manual and standard parameters. Here we also define a new `CurrentParameter` class that uses the ithaco parameters to convert a measured voltage to a current. When subclassing a parameter class (`Parameter`, `MultiParameter`, ...), the functions for setting and getting should be called `get_raw` and `set_raw`, respectively. When subclassing, also declare the type of the value and of the instrument, as `CurrentParameter` does with `MultiParameter[tuple[float, float], \"Ithaco1211\"]`." ] }, { @@ -544,7 +572,7 @@ "metadata": {}, "outputs": [], "source": [ - "class CurrentParameter(MultiParameter):\n", + "class CurrentParameter(MultiParameter[tuple[float, float], \"Ithaco1211\"]):\n", " \"\"\"\n", " Current measurement via an Ithaco preamp and a measured voltage.\n", "\n", @@ -569,7 +597,7 @@ " measured_param: Parameter,\n", " c_amp_ins: \"Ithaco1211\",\n", " name: str = \"curr\",\n", - " **kwargs,\n", + " **kwargs: Any,\n", " ):\n", " p_name = measured_param.name\n", "\n", @@ -588,7 +616,7 @@ "\n", " self._measured_param = measured_param\n", "\n", - " def get_raw(self):\n", + " def get_raw(self) -> tuple[float, float]:\n", " volt = self._measured_param.get()\n", " assert isinstance(self.instrument, Ithaco1211)\n", " current = (\n", @@ -612,59 +640,64 @@ " def __init__(self, name: str, **kwargs: \"Unpack[InstrumentBaseKWArgs]\"):\n", " super().__init__(name, **kwargs)\n", "\n", - " # ManualParameter has an \"initial_value\" kwarg, but if you use this\n", - " # you must be careful to check that it's correct before relying on it.\n", - " # if you don't set initial_value, it will start out as None.\n", - " self.sens: ManualParameter = self.add_parameter(\n", + " # get_cmd=None and set_cmd=None make a manual parameter that only stores\n", + " # the value last set. Without an initial_value it starts out as None, which\n", + " # does not match the annotated value type, so set one, and make sure that\n", + " # it matches the instrument before relying on it.\n", + " self.sens: Parameter[float, Self] = self.add_parameter(\n", " \"sens\",\n", - " parameter_class=ManualParameter,\n", + " get_cmd=None,\n", + " set_cmd=None,\n", " initial_value=1e-8,\n", " label=\"Sensitivity\",\n", - " units=\"A/V\",\n", + " unit=\"A/V\",\n", " vals=vals.Enum(1e-11, 1e-10, 1e-09, 1e-08, 1e-07, 1e-06, 1e-05, 1e-4, 1e-3),\n", " )\n", - " \"Parameter sense\"\n", + " \"\"\"Parameter sens\"\"\"\n", "\n", - " self.invert: ManualParameter = self.add_parameter(\n", + " self.invert: Parameter[bool, Self] = self.add_parameter(\n", " \"invert\",\n", - " parameter_class=ManualParameter,\n", + " get_cmd=None,\n", + " set_cmd=None,\n", " initial_value=True,\n", " label=\"Inverted output\",\n", " vals=vals.Bool(),\n", " )\n", - " \"Parameter invert\"\n", + " \"\"\"Parameter invert\"\"\"\n", "\n", - " self.sens_factor = self.add_parameter(\n", + " self.sens_factor: Parameter[float, Self] = self.add_parameter(\n", " \"sens_factor\",\n", - " parameter_class=ManualParameter,\n", + " get_cmd=None,\n", + " set_cmd=None,\n", " initial_value=1,\n", " label=\"Sensitivity factor\",\n", - " units=None,\n", " vals=vals.Enum(0.1, 1, 10),\n", " )\n", " \"\"\"Parameter sens_factor\"\"\"\n", "\n", - " self.suppression = self.add_parameter(\n", + " self.suppression: Parameter[float, Self] = self.add_parameter(\n", " \"suppression\",\n", - " parameter_class=ManualParameter,\n", + " get_cmd=None,\n", + " set_cmd=None,\n", " initial_value=1e-7,\n", " label=\"Suppression\",\n", - " units=\"A\",\n", + " unit=\"A\",\n", " vals=vals.Enum(1e-10, 1e-09, 1e-08, 1e-07, 1e-06, 1e-05, 1e-4, 1e-3),\n", " )\n", " \"\"\"Parameter suppression\"\"\"\n", "\n", - " self.risetime: ManualParameter = self.add_parameter(\n", + " self.risetime: Parameter[float, Self] = self.add_parameter(\n", " \"risetime\",\n", - " parameter_class=ManualParameter,\n", + " get_cmd=None,\n", + " set_cmd=None,\n", " initial_value=0.3,\n", " label=\"Rise Time\",\n", - " units=\"msec\",\n", + " unit=\"msec\",\n", " vals=vals.Enum(0.01, 0.03, 0.1, 0.3, 1, 3, 10, 30, 100, 300, 1000),\n", " )\n", " \"\"\"Parameter risetime\"\"\"\n", "\n", - " def get_idn(self):\n", + " def get_idn(self) -> dict[str, str | None]:\n", " return {\n", " \"vendor\": \"Ithaco (DL Instruments)\",\n", " \"model\": \"1211\",\n", @@ -725,7 +758,7 @@ "\n", "Note that this particular example of a `display` parameter is trivial because the ambiguity and inconsistency for \"this kind\" of parameters can be solved by having the name of the parameter be `display_enabled` and the allowed input values to be python `bool` `True`/`False`.\n", "\n", - "Anyway, when defining parameters where the solution does not come trivially, please, consider setting `val_mapping` of a parameter to the output of `create_on_off_val_mapping(on_val=<>, off_val=<>)` function from `qcodes.parameters` package. The function takes care of creating a `val_mapping` dictionary that maps given instrument-side values of `on_val` and `off_val` to `True`/`False`, `'ON'`/`'OFF'`, `'on'`/`'off'`, and other commonly used ones. Note that when getting a value of such a parameter, the user will not get `'ON'` or `'off'` or `'oFF'` - instead, `True`/`False` will be returned." + "Anyway, when defining parameters where the solution does not come trivially, please, consider setting `val_mapping` of a parameter to the output of `create_on_off_val_mapping(on_val=<>, off_val=<>)` function from `qcodes.parameters` package. The function takes care of creating a `val_mapping` dictionary that maps given instrument-side values of `on_val` and `off_val` to `True`/`False`, `'ON'`/`'OFF'`, `'on'`/`'off'`, and other commonly used ones. Note that when getting a value of such a parameter, the user will not get `'ON'` or `'off'` or `'oFF'` - instead, `True`/`False` will be returned, so annotate the parameter as `Parameter[bool, Self]`." ] }, { diff --git a/src/qcodes/instrument/sims/Basel_SP983a.yaml b/src/qcodes/instrument/sims/Basel_SP983a.yaml new file mode 100644 index 000000000000..6f8a1d62c201 --- /dev/null +++ b/src/qcodes/instrument/sims/Basel_SP983a.yaml @@ -0,0 +1,35 @@ +spec: "1.0" +devices: + device 1: + eom: + GPIB INSTR: + q: "\r\n" + r: "\r\n" + error: ERROR + dialogues: + - q: "GET O" + r: "Overload: OFF" + + # replies follow the format that the driver parses + properties: + gain: + default: "1E8" + getter: + q: "GET G" + r: "Gain: {}" + setter: + q: "SET G {}" + r: "OK" + + filter: + default: "1k" + getter: + q: "GET F" + r: "Filter: {}Hz" + setter: + q: "SET F {}" + r: "OK" + +resources: + GPIB::1::INSTR: + device: device 1 diff --git a/src/qcodes/instrument/sims/Keithley_6500.yaml b/src/qcodes/instrument/sims/Keithley_6500.yaml index c17c66a09c56..156e00259a0b 100644 --- a/src/qcodes/instrument/sims/Keithley_6500.yaml +++ b/src/qcodes/instrument/sims/Keithley_6500.yaml @@ -33,6 +33,18 @@ devices: min: 4 max: 7 + scan_count: + default: 1 + getter: + q: "ROUT:SCAN:COUN:SCAN?" + r: "{}" + setter: + q: "ROUT:SCAN:COUN:SCAN {}" + specs: + type: int + min: 0 + max: 100000000 + resources: GPIB::1::INSTR: device: device 1 diff --git a/src/qcodes/instrument/sims/keithley_7510.yaml b/src/qcodes/instrument/sims/keithley_7510.yaml index 39c53d28b44e..047f9f9cb2cd 100644 --- a/src/qcodes/instrument/sims/keithley_7510.yaml +++ b/src/qcodes/instrument/sims/keithley_7510.yaml @@ -80,6 +80,15 @@ devices: specs: type: int + # command-only, no reply + auto_zero_once: + setter: + q: ":SENSe:AZERo:ONCE" + + trigger_in_ext_clear: + setter: + q: ":TRIGger:EXTernal:IN:CLEar" + resources: "GPIB::1::INSTR": device: device 1 diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_2450.py b/src/qcodes/instrument_drivers/Keithley/Keithley_2450.py index e26a86205fa6..fc6aa8ef2e2b 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_2450.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_2450.py @@ -113,7 +113,7 @@ def __init__( ) """To get the number of readings in the reading buffer.""" - self.elements: Parameter = self.add_parameter( + self.elements: Parameter[list[str], Self] = self.add_parameter( "elements", get_cmd=None, get_parser=self.from_scpi_to_name, @@ -455,7 +455,7 @@ def __init__(self, parent: "Keithley2450", name: str, proper_function: str) -> N snapshot_value=False, ) - self.sweep_axis: Parameter = self.add_parameter( + self.sweep_axis: Parameter[npt.NDArray, Self] = self.add_parameter( "sweep_axis", label=self._proper_function, get_cmd=self.get_sweep_axis, @@ -663,7 +663,7 @@ def __init__( ) """returns the power line frequency setting that is used for NPLC calculations""" - self.buffer_name: Parameter = self.add_parameter( + self.buffer_name: Parameter[str, Self] = self.add_parameter( "buffer_name", get_cmd=None, set_cmd=None, diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_6500.py b/src/qcodes/instrument_drivers/Keithley/Keithley_6500.py index 7e9a3bad4e9a..d2f105d4b232 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_6500.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_6500.py @@ -2,7 +2,7 @@ from typing import TYPE_CHECKING, Self from qcodes.instrument import VisaInstrument, VisaInstrumentKWArgs -from qcodes.validators import Bool, Enum, Ints, MultiType, Numbers +from qcodes.validators import Bool, Enum, Ints, Numbers, PermissiveInts if TYPE_CHECKING: from collections.abc import Callable @@ -184,17 +184,18 @@ def __init__( ) """Control the brightness of the display backligt. Off turns the display off andBlackout also turns off indicators and key lights on the device.""" - self.trigger_count: Parameter = self.add_parameter( + self.trigger_count: Parameter[int, Self] = self.add_parameter( "trigger_count", get_parser=int, get_cmd="ROUT:SCAN:COUN:SCAN?", set_cmd="ROUT:SCAN:COUN:SCAN {}", - vals=MultiType( - Ints(min_value=1, max_value=9999), - Enum("inf", "default", "minimum", "maximum"), - ), + # accept whole-number floats such as 10.0 but send them as int + vals=PermissiveInts(0, 100_000_000), + set_parser=round, + docstring="Number of times the scan is repeated. " + "Set to 0 to repeat the scan until it is aborted.", ) - """Parameter trigger_count""" + """Number of times the scan is repeated. Set to 0 to repeat the scan until it is aborted.""" for trigger in range(1, 5): self.add_parameter( diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py index ab5bbe5d6a29..42a73c935981 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py @@ -163,7 +163,7 @@ def __init__( ) """Get the starting index of readings in the reading buffer.""" - self.data_start: Parameter = self.add_parameter( + self.data_start: Parameter[int, Self] = self.add_parameter( "data_start", initial_value=1, get_cmd=None, @@ -172,7 +172,7 @@ def __init__( ) """First index of the data to be returned.""" - self.data_end: Parameter = self.add_parameter( + self.data_end: Parameter[int, Self] = self.add_parameter( "data_end", initial_value=1, get_cmd=None, @@ -181,7 +181,7 @@ def __init__( ) """Last index of the data to be returned.""" - self.elements: Parameter = self.add_parameter( + self.elements: Parameter[list[str], Self] = self.add_parameter( "elements", get_cmd=None, get_parser=self._from_scpi_to_name, @@ -223,7 +223,7 @@ def __init__( ) """Parameter setpoints""" - self.t_start: Parameter = self.add_parameter( + self.t_start: Parameter[float, Self] = self.add_parameter( "t_start", label="start time", unit="s", @@ -234,7 +234,7 @@ def __init__( ) """Parameter t_start""" - self.t_stop: Parameter = self.add_parameter( + self.t_stop: Parameter[float, Self] = self.add_parameter( "t_stop", label="stop time", unit="s", @@ -566,14 +566,6 @@ def __init__( ) """Enable or disable automatic updates to the internal reference measurements (autozero) of the instrument.""" - self.auto_zero_once: Parameter = self.add_parameter( - "auto_zero_once", - set_cmd=":SENSe:AZERo:ONCE", - docstring="Cause the instrument to refresh the reference and " - "zero measurements once", - ) - """Cause the instrument to refresh the reference and zero measurements once""" - self.average: Parameter[bool, Self] = self.add_parameter( "average", get_cmd=f":SENSe:{self._proper_function}:AVERage?", @@ -628,6 +620,13 @@ def clear_trace(self, buffer_name: str = "defbuffer1") -> None: """ self.write(f":TRACe:CLEar '{buffer_name}'") + def auto_zero_once(self) -> None: + """ + Cause the instrument to refresh the reference and zero measurements + once. + """ + self.write(":SENSe:AZERo:ONCE") + class Keithley7510DigitizeSense(InstrumentChannel["Keithley7510"]): """ @@ -772,7 +771,7 @@ def __init__( ) """Make readings using the active digitize function.""" - self.buffer_name: Parameter = self.add_parameter( + self.buffer_name: Parameter[str, Self] = self.add_parameter( "buffer_name", get_cmd=None, set_cmd=None, @@ -787,13 +786,6 @@ def __init__( ) """Return the settings for all trigger model blocks.""" - self.trigger_in_ext_clear: Parameter = self.add_parameter( - "trigger_in_ext_clear", - set_cmd=":TRIGger:EXTernal:IN:CLEar", - docstring="Clear the trigger event on the external in line.", - ) - """Clear the trigger event on the external in line.""" - self.trigger_in_ext_edge: Parameter[str, Self] = self.add_parameter( "trigger_in_ext_edge", get_cmd=":TRIGger:EXTernal:IN:EDGE?", @@ -896,6 +888,13 @@ def wait(self) -> None: """ self.write("*WAI") + def trigger_in_ext_clear(self) -> None: + """ + Clear the trigger event on the external in line. This also discards + the event history and clears the overrun status. + """ + self.write(":TRIGger:EXTernal:IN:CLEar") + def clear_status(self) -> None: """ This command clears the event registers of the Questionable Event and diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_s46.py b/src/qcodes/instrument_drivers/Keithley/Keithley_s46.py index 8238349c8720..9ed378ff9b40 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_s46.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_s46.py @@ -4,7 +4,7 @@ import re from itertools import product -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Self from qcodes.instrument import ( VisaInstrument, @@ -151,7 +151,7 @@ def __init__( ): super().__init__(name, address, **kwargs) try: - self.closed_channels: Parameter = self.add_parameter( + self.closed_channels: Parameter[list[str], Self] = self.add_parameter( "closed_channels", get_cmd=":CLOS?", get_parser=self._get_closed_channels_parser, diff --git a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py index 8ca91a9824bc..a30c43a92310 100644 --- a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py +++ b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py @@ -959,7 +959,7 @@ def __init__(self, parent: Keithley2600, name: str, channel: str) -> None: ... ) """ - self.timetrace_npts: Parameter = self.add_parameter( + self.timetrace_npts: Parameter[int, Self] = self.add_parameter( "timetrace_npts", initial_value=500, label="Number of points", @@ -968,7 +968,7 @@ def __init__(self, parent: Keithley2600, name: str, channel: str) -> None: ) """Parameter timetrace_npts""" - self.timetrace_dt: Parameter = self.add_parameter( + self.timetrace_dt: Parameter[float, Self] = self.add_parameter( "timetrace_dt", initial_value=1e-3, label="Time resolution", diff --git a/src/qcodes/instrument_drivers/basel/BaselSP983.py b/src/qcodes/instrument_drivers/basel/BaselSP983.py index 7a81b8f267e3..5ff7db48de7e 100644 --- a/src/qcodes/instrument_drivers/basel/BaselSP983.py +++ b/src/qcodes/instrument_drivers/basel/BaselSP983.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Self from qcodes.instrument import Instrument, InstrumentBaseKWArgs from qcodes.parameters import DelegateParameter, Parameter @@ -44,7 +44,7 @@ def __init__( ): super().__init__(name, **kwargs) - self.gain: Parameter = self.add_parameter( + self.gain: Parameter[float, Self] = self.add_parameter( "gain", initial_value=1e8, label="Gain", @@ -55,7 +55,7 @@ def __init__( ) """Parameter gain""" - self.fcut: Parameter = self.add_parameter( + self.fcut: Parameter[float, Self] = self.add_parameter( "fcut", initial_value=1e3, label="Cutoff frequency", diff --git a/src/qcodes/instrument_drivers/basel/BaselSP983a.py b/src/qcodes/instrument_drivers/basel/BaselSP983a.py index 77a7e7458ca6..c85211fc2509 100644 --- a/src/qcodes/instrument_drivers/basel/BaselSP983a.py +++ b/src/qcodes/instrument_drivers/basel/BaselSP983a.py @@ -40,7 +40,7 @@ def __init__( self.connect_message() - self.gain: Parameter = self.add_parameter( + self.gain: Parameter[float, Self] = self.add_parameter( "gain", label="Gain", unit="V/A", @@ -49,7 +49,7 @@ def __init__( vals=vals.Enum(1e5, 1e6, 1e7, 1e8, 1e9), ) """Parameter gain""" - self.fcut: Parameter = self.add_parameter( + self.fcut: Parameter[float, Self] = self.add_parameter( "fcut", unit="Hz", label="Filter Cut-Off Frequency", diff --git a/src/qcodes/instrument_drivers/ithaco/Ithaco_1211.py b/src/qcodes/instrument_drivers/ithaco/Ithaco_1211.py index fc1a716b86d4..f5ffe12e001d 100644 --- a/src/qcodes/instrument_drivers/ithaco/Ithaco_1211.py +++ b/src/qcodes/instrument_drivers/ithaco/Ithaco_1211.py @@ -82,7 +82,7 @@ class Ithaco1211(Instrument): def __init__(self, name: str, **kwargs: "Unpack[InstrumentBaseKWArgs]"): super().__init__(name, **kwargs) - self.sens: Parameter = self.add_parameter( + self.sens: Parameter[float, Self] = self.add_parameter( "sens", initial_value=1e-8, label="Sensitivity", @@ -103,7 +103,7 @@ def __init__(self, name: str, **kwargs: "Unpack[InstrumentBaseKWArgs]"): ) """Parameter invert""" - self.sens_factor: Parameter = self.add_parameter( + self.sens_factor: Parameter[float, Self] = self.add_parameter( "sens_factor", initial_value=1, label="Sensitivity factor", @@ -114,7 +114,7 @@ def __init__(self, name: str, **kwargs: "Unpack[InstrumentBaseKWArgs]"): ) """Parameter sens_factor""" - self.suppression: Parameter = self.add_parameter( + self.suppression: Parameter[float, Self] = self.add_parameter( "suppression", initial_value=1e-7, label="Suppression", @@ -125,7 +125,7 @@ def __init__(self, name: str, **kwargs: "Unpack[InstrumentBaseKWArgs]"): ) """Parameter suppression""" - self.risetime: Parameter = self.add_parameter( + self.risetime: Parameter[float, Self] = self.add_parameter( "risetime", initial_value=0.3, label="Rise Time", diff --git a/tests/drivers/test_basel.py b/tests/drivers/test_basel.py new file mode 100644 index 000000000000..bbae5fcbb16f --- /dev/null +++ b/tests/drivers/test_basel.py @@ -0,0 +1,71 @@ +from typing import TYPE_CHECKING + +import pytest + +from qcodes.instrument_drivers.basel import BaselSP983a, BaselSP983c + +if TYPE_CHECKING: + from collections.abc import Generator + + +@pytest.fixture(name="sp983c") +def _make_sp983c() -> "Generator[BaselSP983c, None, None]": + driver = BaselSP983c("sp983c") + try: + yield driver + finally: + driver.close() + + +@pytest.fixture(name="sp983a") +def _make_sp983a() -> "Generator[BaselSP983a, None, None]": + driver = BaselSP983a( + "sp983a", address="GPIB::1::INSTR", pyvisa_sim_file="Basel_SP983a.yaml" + ) + try: + yield driver + finally: + driver.close() + + +def test_sp983c_gain(sp983c: BaselSP983c) -> None: + assert sp983c.gain() == 1e8 + + sp983c.gain(1e6) + assert sp983c.gain() == 1e6 + + with pytest.raises(ValueError): + sp983c.gain(2e6) + + +def test_sp983c_fcut(sp983c: BaselSP983c) -> None: + assert sp983c.fcut() == 1e3 + + sp983c.fcut(30) + assert sp983c.fcut() == 30 + + with pytest.raises(ValueError): + sp983c.fcut(50) + + +@pytest.mark.parametrize("gain", [1e5, 1e6, 1e7, 1e8, 1e9]) +def test_sp983a_gain(sp983a: BaselSP983a, gain: float) -> None: + sp983a.gain(gain) + value = sp983a.gain() + assert isinstance(value, float) + assert value == gain + + +@pytest.mark.parametrize("fcut", [30, 100, 300, 1e3, 3e3, 10e3, 30e3, 100e3, 1e6]) +def test_sp983a_fcut(sp983a: BaselSP983a, fcut: float) -> None: + sp983a.fcut(fcut) + assert sp983a.fcut() == fcut + + +def test_sp983a_fcut_rejects_unknown_value(sp983a: BaselSP983a) -> None: + with pytest.raises(ValueError): + sp983a.fcut(50) + + +def test_sp983a_overload_status(sp983a: BaselSP983a) -> None: + assert sp983a.overload_status() == "Overload: OFF" diff --git a/tests/drivers/test_ithaco.py b/tests/drivers/test_ithaco.py new file mode 100644 index 000000000000..42f9d12440e5 --- /dev/null +++ b/tests/drivers/test_ithaco.py @@ -0,0 +1,63 @@ +from typing import TYPE_CHECKING + +import pytest + +from qcodes.instrument_drivers.ithaco import CurrentParameter, Ithaco1211 +from qcodes.parameters import Parameter + +if TYPE_CHECKING: + from collections.abc import Generator + + +@pytest.fixture(name="ithaco") +def _make_ithaco() -> "Generator[Ithaco1211, None, None]": + driver = Ithaco1211("ithaco") + try: + yield driver + finally: + driver.close() + + +def test_initial_values(ithaco: Ithaco1211) -> None: + assert ithaco.sens() == 1e-8 + assert ithaco.sens_factor() == 1 + assert ithaco.suppression() == 1e-7 + assert ithaco.risetime() == 0.3 + assert ithaco.invert() is True + + +@pytest.mark.parametrize( + ("name", "valid", "invalid"), + [ + ("sens", 1e-4, 2e-4), + ("sens_factor", 0.1, 5), + ("suppression", 1e-3, 1e-2), + ("risetime", 1000, 2000), + ], +) +def test_parameters_validate( + ithaco: Ithaco1211, name: str, valid: float, invalid: float +) -> None: + param = ithaco.parameters[name] + assert isinstance(param, Parameter) + + param(valid) + assert param() == valid + + with pytest.raises(ValueError): + param(invalid) + + +@pytest.mark.parametrize(("invert", "sign"), [(True, -1), (False, 1)]) +def test_current_parameter(ithaco: Ithaco1211, invert: bool, sign: int) -> None: + volt = Parameter("volt", set_cmd=None, initial_value=2.0, unit="V", label="Volt") + ithaco.sens(1e-6) + ithaco.sens_factor(10) + ithaco.invert(invert) + + curr = CurrentParameter(volt, ithaco) + + assert curr.names == ("volt_raw", "curr") + assert curr.units == ("V", "A") + assert curr.labels == ("Volt", "Current") + assert curr() == pytest.approx((2.0, sign * 2.0 * 1e-6 * 10)) diff --git a/tests/drivers/test_keithley_2450.py b/tests/drivers/test_keithley_2450.py index dfb1dee47728..b6b455653015 100644 --- a/tests/drivers/test_keithley_2450.py +++ b/tests/drivers/test_keithley_2450.py @@ -168,3 +168,20 @@ def test_source_delay_is_float(k2450: Keithley2450) -> None: delay = k2450.source.delay() assert isinstance(delay, float) assert delay == 0.25 + + +def test_buffer_elements_round_trip(k2450: Keithley2450) -> None: + assert k2450.buffer_name() == "defbuffer1" + buffer = k2450.buffer(k2450.buffer_name()) + assert buffer.elements() == [] + + elements = ["time", "measurement_formatted"] + buffer.elements(elements) + # the SCPI names are stored, and get translates them back + assert buffer.elements.raw_value == [ + buffer.buffer_elements[element] for element in elements + ] + assert buffer.elements() == elements + + with pytest.raises(ValueError): + buffer.elements(["not_an_element"]) diff --git a/tests/drivers/test_keithley_6500.py b/tests/drivers/test_keithley_6500.py index f42b00cf21e2..e66c8d1e2bff 100644 --- a/tests/drivers/test_keithley_6500.py +++ b/tests/drivers/test_keithley_6500.py @@ -1,5 +1,6 @@ from typing import TYPE_CHECKING +import numpy as np import pytest from qcodes.instrument_drivers.Keithley import Keithley6500 @@ -7,6 +8,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pytest_mock import MockerFixture + @pytest.fixture(scope="function", name="dmm6500") def _make_dmm6500() -> "Generator[Keithley6500, None, None]": @@ -24,3 +27,39 @@ def test_digits(dmm6500: Keithley6500) -> None: dmm6500.digits(5) assert dmm6500.digits() == 5 + + +@pytest.mark.parametrize("count", [0, 1, 1440, 100_000_000]) +def test_trigger_count(dmm6500: Keithley6500, count: int) -> None: + # 0 repeats the scan until it is aborted + dmm6500.trigger_count(count) + value = dmm6500.trigger_count() + assert isinstance(value, int) + assert value == count + + +@pytest.mark.parametrize("value", [10.0, np.float64(10)], ids=["float", "numpy_float"]) +def test_trigger_count_accepts_whole_floats( + dmm6500: Keithley6500, mocker: "MockerFixture", value: float +) -> None: + write_raw = mocker.spy(dmm6500, "write_raw") + + dmm6500.trigger_count(value) # pyright: ignore[reportArgumentType] + + write_raw.assert_called_once_with("ROUT:SCAN:COUN:SCAN 10") + assert dmm6500.trigger_count() == 10 + + +@pytest.mark.parametrize( + "value", + [-1, 100_000_001, 2.5, "inf"], + ids=["negative", "too_large", "fraction", "inf"], +) +def test_trigger_count_rejects_invalid( + dmm6500: Keithley6500, mocker: "MockerFixture", value: float | str +) -> None: + write_raw = mocker.spy(dmm6500, "write_raw") + + with pytest.raises((ValueError, TypeError)): + dmm6500.trigger_count(value) # pyright: ignore[reportArgumentType] + write_raw.assert_not_called() diff --git a/tests/drivers/test_keithley_7510.py b/tests/drivers/test_keithley_7510.py index 06e9d0921ea6..cd96102fb6af 100644 --- a/tests/drivers/test_keithley_7510.py +++ b/tests/drivers/test_keithley_7510.py @@ -123,3 +123,62 @@ def test_digitize_count_is_int(dmm_7510_driver: Keithley7510) -> None: count = digi_sense.count() assert isinstance(count, int) assert count == 500 + + +def test_buffer_elements_round_trip(dmm_7510_driver: Keithley7510) -> None: + buffer = dmm_7510_driver.buffer("defbuffer1") + assert dmm_7510_driver.buffer_name() == "defbuffer1" + assert buffer.elements() == [] + + elements = ["relative_time", "measurement"] + buffer.elements(elements) + # the SCPI names are stored, and get translates them back + assert buffer.elements.raw_value == [ + buffer.buffer_elements[element] for element in elements + ] + assert buffer.elements() == elements + + with pytest.raises(ValueError): + buffer.elements(["not_an_element"]) + + +def test_buffer_index_and_time_parameters(dmm_7510_driver: Keithley7510) -> None: + buffer = dmm_7510_driver.buffer("defbuffer1") + assert buffer.data_start() == 1 + assert buffer.data_end() == 1 + + buffer.data_end(10) + n_pts = buffer.n_pts() + assert isinstance(n_pts, int) + assert n_pts == 10 + + buffer.t_start(1) + buffer.t_stop(2) + t_start = buffer.t_start() + t_stop = buffer.t_stop() + assert isinstance(t_start, float) + assert isinstance(t_stop, float) + assert (t_start, t_stop) == (1.0, 2.0) + + +@pytest.mark.parametrize( + ("action", "command"), + [ + ("auto_zero_once", ":SENSe:AZERo:ONCE"), + ("trigger_in_ext_clear", ":TRIGger:EXTernal:IN:CLEar"), + ], +) +def test_action_methods_send_command( + dmm_7510_driver: Keithley7510, mocker: "MockerFixture", action: str, command: str +) -> None: + # these used to be parameters, so calling them without an argument only + # returned the cached value and did not send anything + write_raw = mocker.spy(dmm_7510_driver, "write_raw") + if action == "auto_zero_once": + dmm_7510_driver.sense.auto_zero_once() + else: + dmm_7510_driver.trigger_in_ext_clear() + + write_raw.assert_called_once_with(command) + # the instrument does not reply, so the next query is unaffected + assert dmm_7510_driver.IDN()["model"] == "DMM7510"