Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/release-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
Release Notes
==================================

*************
Version 0.6.5
*************
Release Date xx/xx/xx

New Features
############

Improvements
############
- DMM current mode functions now raise an exception when an incompatible port combination is used. The range and port parameters are now required parameters.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these two sentences should be swapped.

when an imcompatible port combination

Is this trying to say an incompatible port and range combination?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to make sense.



*************
Version 0.6.4
*************
Expand Down
53 changes: 51 additions & 2 deletions src/fixate/drivers/dmm/fluke_8846a.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from fixate.core.exceptions import InstrumentError, ParameterError
from fixate.drivers.dmm.helper import DMM
import time
from typing import Literal


class Fluke8846A(DMM):
Expand Down Expand Up @@ -54,6 +55,10 @@ def __init__(self, instrument, *args, **kwargs):
self._default_nplc = 10 # Default NPLC setting as per Fluke 8846A manual
self._init_string = "" # Unchanging

# High and low current port definition. Each definition encodes the maximum current able to
# be measured by the port (in amps)
self.current_ports = {"HIGH": 10, "LOW": 400e-3}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think self.current_ports should be private in the hopes that people aren't tempted to update them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


@property
def samples(self):
return self._samples
Expand Down Expand Up @@ -268,10 +273,54 @@ def voltage_dc(self, _range=None, auto_impedance=False):
command = "; :SENS:VOLT:DC:IMP:AUTO OFF"
self._set_measurement_mode("voltage_dc", _range, suffix=command)

def current_ac(self, _range=None):
def current_ac(self, _range, port):
"""
Set the measurement mode on the DMM to AC current.

If the range and port selection are not compatible, i.e. someone has requested to measure
1 A on the low range port with a maximum capability of 400 mA, an exception is raised.

If the range requested can be measured by the low port, but the high port is selected, an
exception is raised.
"""

# Check the requested range is not more than the port capability:
if _range > self.current_ports[port]:
raise ValueError(
"The selected port and range combination is not available for this instrument. Consider using a different multimeter"
)

# Raise an error if the high port is selected when the low port should be used:
if _range < self.current_ports["LOW"] and port == "HIGH":
raise ValueError(
"High range port selected when the low range port should be used! Consider using a different multimeter."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the consideration here be to use the low current port?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not what it is doing already? If a small range is selected, and the high port requested, the driver will throw an exception requesting that the low range port should be used instead.

This is because the DMM will often select the low range port automatically based on the given range, so I thought it helpful to force the port selection to reduce unexpected behavior.

)

self._set_measurement_mode("current_ac", _range)

def current_dc(self, _range=None):
def current_dc(self, _range, port: Literal["HIGH", "LOW"]):
"""
Set the measurement mode on the DMM to DC current.

If the range and port selection are not compatible, i.e. someone has requested to measure
1A on the low range port with a maximum capability of 400 mA, an exception is raised.

If the range requested can be measured by the low port, but the high port is selected, an
exception is raised.
"""

# Check the requested range is not more than the port capability:
if _range > self.current_ports[port]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of duplicating these checks, could we pull them out into their own method and calling that from the AC and DC methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the checks over to a function that is now defined in the helper. That way both the drivers just use that function.

raise ValueError(
"The selected port and range combination is not available for this instrument. Consider using a different multimeter"
)

# Raise an error if the high port is selected when the low port should be used:
if _range < self.current_ports["LOW"] and port == "HIGH":
raise ValueError(
"High range port selected when the low range port should be used! Consider using a different multimeter."
)

self._set_measurement_mode("current_dc", _range)

def resistance(self, _range=None):
Expand Down
9 changes: 7 additions & 2 deletions src/fixate/drivers/dmm/helper.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from dataclasses import dataclass
from typing import Literal


class DMM:
Expand Down Expand Up @@ -60,10 +61,14 @@ def voltage_dc(self, _range=None, auto_impedance=False):
"""
raise NotImplementedError

def current_ac(self, _range):
def current_ac(self, _range, port: Literal["HIGH", "LOW"]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to introduce braking changes, maybe now is a good opportunity to make the not so private private _range not private anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did have this thought, then it annoyed me that only one function in the whole API uses the correct naming convention. I know it's wrong, but I almost prefer to have everything consistent? Let me know if you really want me to change it.

"""
Sets the DMM in AC current measurement mode and puts it in the range given
by the argument _range. Signals expected to be measured must be < _range.
"""
raise NotImplementedError

def current_dc(self, _range):
def current_dc(self, _range, port: Literal["HIGH", "LOW"]):
"""
Sets the DMM in DC current measurement mode and puts it in the range given
by the argument _range. Signals expected to be measured must be < _range.
Expand Down
51 changes: 49 additions & 2 deletions src/fixate/drivers/dmm/keithley_6500.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ def __init__(self, instrument, *args, **kwargs):
self._nplc_default = 1
self._init_string = "" # Unchanging

# High and low current port definition. Each definition encodes the maximum current able to
# be measured by the port (in amps)
self.current_ports = {"HIGH": 10, "LOW": 3}

# Adapted for different DMM behaviour
@property
def display(self):
Expand Down Expand Up @@ -303,10 +307,53 @@ def voltage_dc(self, _range=None, auto_impedance=False):
command = "; :SENS:VOLT:DC:INP MOHM10"
self._set_measurement_mode("voltage_dc", _range, suffix=command)

def current_ac(self, _range=None):
def current_ac(self, _range, port):
"""
Set the measurement mode on the DMM to AC current.

If the range and port selection are not compatible, i.e. someone has requested to measure
1A on the low range port with a maximum capability of 400 mA, an exception is raised.

If the range requested can be measured by the low port, but the high port is selected, an
exception is raised.
"""
# Check the requested range is not more than the port capability:
if _range > self.current_ports[port]:
raise ValueError(
"The selected port and range combination is not available for this instrument. Consider using a different multimeter"
)

# Raise an error if the high port is selected when the low port should be used:
if _range < self.current_ports["LOW"] and port == "HIGH":
raise ValueError(
"High range port selected when the low range port should be used! Consider using a different multimeter."
)

self._set_measurement_mode("current_ac", _range)

def current_dc(self, _range=None):
def current_dc(self, _range, port):
"""
Set the measurement mode on the DMM to DC current.

If the range and port selection are not compatible, i.e. someone has requested to measure
1A on the low range port with a maximum capability of 400 mA, an exception is raised.

If the range requested can be measured by the low port, but the high port is selected, an
exception is raised.
"""

# Check the requested range is not more than the port capability:
if _range > self.current_ports[port]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment RE duplication as in the fluke driver.

raise ValueError(
"The selected port and range combination is not available for this instrument."
)

# Raise an error if the high port is selected when the low port should be used:
if _range < self.current_ports["LOW"] and port == "HIGH":
raise ValueError(
"High range port selected when the low range port should be used!"
)

self._set_measurement_mode("current_dc", _range)

def resistance(self, _range=None):
Expand Down