Adding example in NI-DC Power , NI-DMM , NI-Switch - #2192
Conversation
Adding one new example each in NI-DMM, NI-DCPower, and NI-SWITCH.
updated CHANGELOG.md to include additional examples added in nidcpower, nidmm, niswitch in "unreleased section.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (81.59%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.
Additional details and impacted files@@ Coverage Diff @@
## master #2192 +/- ##
==========================================
- Coverage 89.32% 81.59% -7.73%
==========================================
Files 73 31 -42
Lines 19048 4308 -14740
==========================================
- Hits 17014 3515 -13499
+ Misses 2034 793 -1241
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Please stick to the pull request template. Include all 3, place an X to fill in the checkbox, where appropriate, and strikethrough anything that doesn't apply. |
|
| import argparse # For parsing command-line arguments | ||
| import sys # For accessing command-line arguments via sys.argv | ||
|
|
||
| import nidcpower # NI-DCPower instrument driver (SMU trigger source) |
There was a problem hiding this comment.
You can't add this dependency on another nimi-python API without updating generated/nidmm/tox-system_tests.ini via changes to build/templates/tox-system_tests.ini.mako.
Whether we should add such an example is a separate issue. This change would also force changes to our post-release checks and internal test events.
| def test_example(): | ||
| """Simulated hardware test — runs example() with virtual NI-2568 switch (no real HW needed).""" | ||
| example( | ||
| resource_name='PXI2568', |
There was a problem hiding this comment.
I don't know why it should matter, but you may need to stick with the pattern of passing an empty string, established by the other niswitch examples. The driver doesn't seem to like you passing this string, when simulating.
| options = {'simulate': True, 'driver_setup': {'Model': '4139', 'BoardType': 'PXIe'}} | ||
| example('PXI1Slot1', options, 2.0, 4.0, 10.0, 0.01, 0.01, 0.05) |
There was a problem hiding this comment.
This is a DAQmx-based device. IVI Simulation of such devices sometimes fails. Pick a different device.
…dded 4163 as simulated device) and switch example(empty string to resource name).
|
Please spell product names correctly. e.g. driver is NI-SWITCH. This is a public repo and we should be professional. |
|
I reviewed only one example. Once the feedback is applied to the rest please reset my feedback. |
| :encoding: utf8 | ||
| :caption: `(niswitch_relay_control.py) <https://github.com/ni/nimi-python/blob/master/src/niswitch/examples/niswitch_relay_control.py>`_ | ||
|
|
||
| niswitch_software_scanning.py |
There was a problem hiding this comment.
I do not think it is wise to add scanning examples. This is a feature no longer supported by any of the switches developed in the last ~18 years.
| -vl1 3.0 -vl2 5.0 -sd 0.1 | ||
|
|
||
| iii. To simulate without hardware: | ||
| PowerShell: python nidcpower_hardware_timed_single_point.py \ |
There was a problem hiding this comment.
The Powershell and cmd instructions look the same to me. Do you need both? If you do, then you definitely need Linux.
But I don't think we need both.
| """ | ||
|
|
||
| # Module imports | ||
| import argparse # For parsing command-line arguments |
There was a problem hiding this comment.
Are all these comments consistent with all other examples? If they are, ok.
If they aren't please remove. I don't think they add much value.
| """Core measurement logic — sources two voltage levels sequentially and returns both measurements. | ||
|
|
||
| Args: | ||
| resource_name (str) : NI-MAX resource name, eg: "PXI1Slot2" |
There was a problem hiding this comment.
Is this consistent with all our other examples?
NI MAX is spelled with no dash, and is deprecated.
I would imagine the documentation when setting up argparse would suffice. DRY.
| current_limit_range (float) : Current range — must be >= current_limit (A) | ||
| source_delay (float) : Delay before Source Complete Event fires(s) | ||
| """ | ||
| # 'with' block ensures session.abort() + session.close() are called automatically on exit. |
There was a problem hiding this comment.
this with block only ensures session.close() is called.
| with nidcpower.Session(resource_name=resource_name, reset=True, channels=0, options=options) as session: | ||
|
|
||
| # Configure source mode and output function. | ||
| # SINGLE_POINT: sources one value and holds; voltage_level can be changed mid-session. |
There was a problem hiding this comment.
instead of mid-session you should say it can be "set while in the running state"
| session.source_delay = source_delay | ||
| session.measure_when = nidcpower.MeasureWhen.AUTOMATICALLY_AFTER_SOURCE_COMPLETE | ||
|
|
||
| # Commit sends all settings to hardware before initiate. |
There was a problem hiding this comment.
in this case, there is no point in explicitly calling commit
|
|
||
| # Initiate output, fetch at voltage_level_1, change to voltage_level_2, fetch again. | ||
| # timeout=1.0 s — adjust if source_delay is longer than 1 s. | ||
| with session.initiate(): |
There was a problem hiding this comment.
this context manager ensures abort() is closed
I've added tests applicable for this pull requestWhat does this Pull Request accomplish?
List issues fixed by this Pull Request below, if any.
None
What testing has been done?