Update dmm current modes #225

Jasper-Harvey0 · 2025-07-08T23:06:57Z

This is picking up part of the work that I originally tried to do over on #205, but with a reduced scope to look at the DMM current mode functions.

Basically we need to start using the Keithley DMMs in production sooner rather than later, and the high / low current port measurement range mismatch is something that has been in the back of my mind to fix...

The changes add a required 'port' parameter to inform the driver which port is intended to be used for the measurement. The range parameter is also now required. I think this is a reasonable change because the person designing the test script is going to have this information already.
We don't add any magic to the driver that tries to set ranges for you based on the port, rather, we simply raise an error if the port and range combination is incompatible. This will avoid potentially damaging situations occurring on jigs, and will simply trigger production to grab an Engineer if the exception is raised.

It can then be decided if the multimeter is to be swapped to a compatible one, or change the test script / jig hardware to work across all instruments.

This is a breaking change for any test script that uses the current functions because of the new required port parameter.

jcollins1983

Just a couple of minor things that I think need tweaking. But I'm generally onboard based on our discussions the other day.
WRT breaking changes, what impact is there if a script is updated but is run on an older version of Fixate?
Have you done a review of the number of scripts that would be impacted and considered a rollout plan?

jcollins1983 · 2025-07-12T06:59:09Z

docs/release-notes.rst

+
+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.


I think these two sentences should be swapped.

when an imcompatible port combination

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

Updated to make sense.

jcollins1983 · 2025-07-12T07:05:45Z

src/fixate/drivers/dmm/helper.py

        raise NotImplementedError

-    def current_ac(self, _range):
+    def current_ac(self, _range, port: Literal["HIGH", "LOW"]):


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?

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.

jcollins1983 · 2025-07-12T07:23:50Z

src/fixate/drivers/dmm/fluke_8846a.py


+        # 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}


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

jcollins1983 · 2025-07-12T07:27:07Z

src/fixate/drivers/dmm/fluke_8846a.py

+        # 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."


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

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.

jcollins1983 · 2025-07-12T07:28:55Z

src/fixate/drivers/dmm/fluke_8846a.py

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


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

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

src/fixate/drivers/dmm/keithley_6500.py

jcollins1983 · 2025-07-12T07:31:19Z

src/fixate/drivers/dmm/keithley_6500.py

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


Same comment RE duplication as in the fluke driver.

Jasper-Harvey0 · 2025-07-14T02:37:16Z

This is what happens if you use an updated test script with an older version of fixate (you get an unexpected keyword argument exception):

Entering the test list
***************************************************************************
Test 2.1: Another description
---------------------------------------------------------------------------

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Test 2.1: Exception Occurred, <class 'TypeError'> current_dc() got an
unexpected keyword argument 'port'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

Test 2.1: Retry
---------------------------------------------------------------------------
Checks passed: 0, Checks failed: 0
Test 2.1: ERROR
---------------------------------------------------------------------------


RETRY, ABORT or FAIL

Likewise, if you update fixate, and not the test script, you will get an error because the function call will be missing a required argument.

Running atgrep with the following: atgrep "current_ac(.*)"

el relays:39:el_final_common.py:154: dm.dmm.current_ac(_range=0.4)
el relays:39:elv_cpu.py:637: dm.dmm.current_ac(_range=self.dmm_range)
ipx_ocs:21:current_tests.py:87: dm.dmm.current_ac(_range=self.target_current * 1.5)
ipx_ocs:21:current_tests_board.py:49: dm.dmm.current_ac()
AMP-402:2:AMP402_Tests.py:256: dm.dmm.current_ac(_range=range_max)  # Set DMM range to improve DMM triggering speed and accuracy
rks_protection:26:rks_protection_board_level.py:3643: dm.dmm.current_ac(_range=self.trip_current)
rks_protection:26:rks_protection_board_level_manual.py:2410: dm.dmm.current_ac(_range=1e-3)
J474 Scripts:13:as2081_validation_tests.py:804: dm.dmm.current_ac(_range=2)

And for DC current:

auto_auto:13:auto_final.py:644: dm.dmm.current_dc()
el relays:39:elv_power.py:58: dm.dmm.current_dc(_range=0.4)
el relays:39:elv_cpu.py:139: dm.dmm.current_dc(_range=0.1)
GG2_Display_Module:14:J383_GG2_Display_Module_Tests.py:976: dm.dmm.current_dc(_range=range_max)  # Set DMM range to improve DMM triggering speed and accuracy
ipx_mem_top:13:J374_IPX_MEM_Top_Tests.py:740: dm.dmm.current_dc(_range=range_max)  # Set DMM range to improve DMM triggering speed and accuracy
rks_ui_bottom:11:i2c_test.py:56: dm.dmm.current_dc()
AMP-402:2:AMP402_Tests.py:325: dm.dmm.current_dc(_range=range_max)  # Set DMM range to improve DMM triggering speed and accuracy
rks_protection:26:rks_protection_board_level.py:2574: dm.dmm.current_dc(_range=100e-3)
rks_protection:26:rks_protection_board_level_manual.py:2169: dm.dmm.current_dc(_range=100e-3)
IPM2_board_level:4:input_board_calibration.py:77: dm.dmm.current_dc("0.5")

I have removed some of the lines from the output to hide multiple calls in one file, just to reduce the noise a little.
So there is roughly 10 scripts that will need an update. The plan initially will be to just update the function calls so it doesn't raise an error. Then we can have a look at messing with the ranges to make the script compatible with both DMMs.

Other than that, I think I have addressed the comments, and updated the tests to work with the updates.

clint-lawrence · 2025-09-08T01:12:12Z

@Jasper-Harvey0 If I understand your last comment correctly, this can be merged until a handful of scripts are updated first. I assume that hasn't happened, so this is waiting on that?

Jasper-Harvey0 · 2025-09-09T20:44:43Z

Yes I believe that is the case. As long as people are happy with the proposed way forward,
Script updates will be a relatively easy job, and I can have them loaded into the config ready to go.
I have created a Jira issue to track updates to the required test scripts (CPE-4148).

Jasper-Harvey0 added 5 commits July 8, 2025 15:23

Add concept for changes to current mode selection

f89c849

Fix function def

2f2ae63

Update doc strings

027a569

Update helper. Fix range and port check bounds

9a7efc7

Update release notes.

9532f5c

Jasper-Harvey0 requested review from clint-lawrence and jcollins1983 July 8, 2025 23:07

Remove unused import.

ab363f1

jcollins1983 requested changes Jul 12, 2025

View reviewed changes

Jasper-Harvey0 added 2 commits July 14, 2025 10:45

Updates from review comments. Update tests.

087356c

Update tests.

a6cf2f1

Update dmm current modes #225

Are you sure you want to change the base?

Update dmm current modes #225

Conversation

Jasper-Harvey0 commented Jul 8, 2025

Uh oh!

jcollins1983 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jasper-Harvey0 commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clint-lawrence commented Sep 8, 2025

Uh oh!

Jasper-Harvey0 commented Sep 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jasper-Harvey0 commented Jul 14, 2025 •

edited

Loading