Great fix. I don't have a device close to me to test myself. Maybe @JAnderson419 can comment if needed.

I'm afraid I won't be much help on the instrument testing front in the short term as I am away from the lab until January, but I can definitely test on the PNA in our lab once I get back (FW 9.32 I think). I did notice that the documentation for the VI module is broken and it looks like it would require running the notebook on a machine connected to a VNA.

This is not related to the changes made in this PR specifically, but I wonder if it would be worth adding tests for the module to run coverage while connected to a local instrument, and then merging that coverage into the main report run by CI? It gets a little tricky with the various versions of VISA and instrument firmware versions, but it might give users more confidence in the VI module and/or allow us to crowd-source a list of supported hardware from users to put in the docs.

I managed to get in to the lab today and test with our PNA which revealed a couple of bugs (all related to the pyvisa update) so I'll update this PR. I'll also investigate updating that documentation.

I managed to get in to the lab today and test with our PNA which revealed a couple of bugs (all related to the pyvisa update) so I'll update this PR. I'll also investigate updating that documentation.

Hi @cafeclimber. Did you make some progresses on that?

I used the latest version of scikit-rf and pyvisa to connect to PNA, but it did not work. So we tried different version combinations of scikit-rf and pyvisa.

1. unconnected

scikit-rf v0.22.1
PyVISA v1.12.0
AttributeError: 'TCPIPInstrument' object has no attribute 'values_format'

2. unconnected

scikit-rf v0.14.9
PyVISA v1.12.0
AttributeError: 'TCPIPInstrument' object has no attribute 'query_values'

3. connectable

scikit-rf v0.22.1
PyVISA v1.10.1

c:path\lib\site-packages\pyvisa\resources\messagebased.py:106: FutureWarning: values_format is deprecated and will be removed in 1.10
  warnings.warn('values_format is deprecated and will be removed in '
c:path\lib\site-packages\pyvisa\resources\messagebased.py:632: FutureWarning: query_values is deprecated and will be removed in 1.10, use query_ascii_values or quey_binary_values instead.
  warnings.warn('query_values is deprecated and will be removed in '

If you're able, could you try using my fork here and let me know what issues you have? It's still incomplete but I am actively trying to rewrite the VI module.

Link to fork

Using your fork, the following worked correctly in python 3.9.

from skrf.vi.vna import PNA
my_pna = PNA(VISAaddress)
measure = my_pna.get_twoport()
measure.s11.plot_s_db()
measure.s22.plot_s_db()

Python 3.9.0
scikit-rf 0.21.0
PyVISA 1.12.0
PNA : N5227B

Note that the following error occurred in python 3.10.
from skrf.vi.vna import PNA
ImportError: cannot import name 'Iterable' from 'collections' (C: \Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1520.0_x64__qbz5n2kfra8p0\lib\collections_init_.py)
Python 3.10.5

This post seems to be close to the error.
stackoverflow

The Python 10 error due to Iterable was fixed in PR #674. Maybe the fork should be sychronized to the current master to include this fix.

The Python 10 error due to Iterable was fixed in PR #674. Maybe the fork should be sychronized to the current master to include this fix.

I'll get on this ASAP!

Using your fork, the following worked correctly in python 3.9.

from skrf.vi.vna import PNA
my_pna = PNA(VISAaddress)
measure = my_pna.get_twoport()
measure.s11.plot_s_db()
measure.s22.plot_s_db()

Python 3.9.0
scikit-rf 0.21.0
PyVISA 1.12.0
PNA : N5227B

Note that the following error occurred in python 3.10.
from skrf.vi.vna import PNA
ImportError: cannot import name 'Iterable' from 'collections' (C: \Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1520.0_x64__qbz5n2kfra8p0\lib\collections_init_.py)
Python 3.10.5

This post seems to be close to the error.
stackoverflow

Out of curiosity, what interface were you connecting to the PNA with? TCP/IP worked great for me but for anything else, I ran in to ask kinds of driver issues.

I connected via ethernet and used tcpip, I did not try GPIB as I do not have the cable.

Sorry if I am posting to the wrong place.
An error occurs when uploading calibration data in the following code.
my_pna.upload_twoport_calibration(cal, port1s=1, port2s=2)
AttributeError: 'SCPI' object has no attribute 'set_calset_data'

The specific location of the error is here.

        for eterm, coef in zip(("EDIR", "ESRM", "ERFT"), ("directivity", "source match", "reflection tracking")):
            self.scpi.set_calset_data(channel, eterm, port1, port1, eterm_data=cfs["forward " + coef])
            self.scpi.set_calset_data(channel, eterm, port2, port2, eterm_data=cfs["reverse " + coef])
        for eterm, coef in zip(("ELDM", "ETRT", "EXTLK"), ("load match", "transmission tracking", "isolation")):
            self.scpi.set_calset_data(channel, eterm, port2, port1, eterm_data=cfs["forward " + coef])
            self.scpi.set_calset_data(channel, eterm, port1, port2, eterm_data=cfs["reverse " + coef])

keysight_pna.py#L198

The cause seems to be the separation of the ascii and binary versions due to changes in pyvisa.

    def set_calset_data_ascii(self, cnum=1, eterm="", portA=1, portB=2, param="", eterm_data=None):
        """no help available"""
        scpi_command = scpi_preprocess(":SENS{:}:CORR:CSET:DATA {:},{:},{:},{:},", cnum, eterm, portA, portB, param)
        self.write_ascii_values(scpi_command, eterm_data)
    
    def set_calset_data_binary(self, cnum=1, eterm="", portA=1, portB=2, param="", eterm_data=None):
        """no help available"""
        scpi_command = scpi_preprocess(":SENS{:}:CORR:CSET:DATA {:},{:},{:},{:},", cnum, eterm, portA, portB, param)
        self.write_binary_values(scpi_command, eterm_data)

keysight_pna_scpi.py#L106

@jeanbiego Yeah this is really what motivated my forking the repo. There seems to have been a breaking change in PyVisa several years ago that scikit-rf did not update with. I saw this as an opportunity to update unify the virtual instrument module. I'm actively PhDing but can try to work on getting my fork more updated / mature as I have free time. I had asked in the Slack a while ago if the path I was heading down with my fork was okay with those most involved in maintaining the VI module, but I haven't heard much back.

@jeanbiego In the short term, if you need to fix your issue, you could clone my fork and implement the calset upload yourself by copying the code from the original VI module but updating the set_calset_data calls to set_calset_data_ascii.

@cafeclimber Thank you. I will try your advice.

An error occurred when sending set values to PNA using the following code.

f_start = 0.01
f_stop = 120
f_npoints = 1201

my_pna.set_frequency_sweep(
    f_start=f_start, f_stop=f_stop, f_npoints=f_npoints, f_unit="ghz")

PNA error : [SCPI: -114] Header suffix out of range; Header suffic of 1201 exceeds maximum of 500

The cause of this seems to be self.scpi.set_sweep_n_points(f_npoints), which set_frequency_sweep calls internally.
Looking at the definition, set_sweep_n_points(self, cnum=1, n_points=401), so f_npoints is recognized as cnum, not n_points.
In fact, if f_npoints is set to 1, set_frequency_sweep does not throw an error, and if f_npoints is set to 2, the error message [SCPI: 122] The specified channel was not found. In both cases, the f_npoints of the PNA are not updated.

I doubt if the above report can be pyvisa related, so please let me know if I should make a separate issue.

Hey @jeanbiego. Are you using my fork (specifically the vi branch)? I don't see the function you mentioned in that branch. The way I've designed the refactor, there should be no calls to self.scpi as there's no self.scpi object.

If you still run into an issue using that branch, please let me know / provide the full stack trace and I'll try to make an update asap.

If it's related to the current upstream skrf vi module, please file a new issue. Thanks!

I prepared a test environment with venv and installed scikit-rf there with pip install git+https://github.com/cafeclimber/scikit-rf.
The relevant section is here(keysight_pna.py#L376), am I looking in the wrong place?

And, there is no error on the python side, so there is no stack trace.

Ah that was my mistake not telling you. That pulls the main branch. I've been working on the vi branch which you can install with

pip install "skrf[vi] @ git+https://github.com/cafeclimber/scikit-rf@vi"

(I'm not positive about that syntax as I've never installed something with extras tags and from a specific branch on GitHub 😳)

WARNING: Generating metadata for package skrf produced metadata for project name scikit-rf. Fix your #egg=skrf fragments. Discarding git+https://github.com/cafeclimber/scikit-rf@vi: Requested scikit-rf from git+https://github.com/cafeclimber/scikit-rf@vi has inconsistent name: filename has 'skrf', but metadata has 'scikit-rf' ERROR: Could not find a version that satisfies the requirement skrf (unavailable) (from versions: none) ERROR: No matching distribution found for skrf (unavailable)
I got the above error, so I tried the following and was able to install.
pip install "scikit-rf[vi] @ git+https://github.com/cafeclimber/scikit-rf@vi"
However, after the installation, I checked the Lib\site-packages\skrf\vi\vna\keysight_pna.py, and the code of set_frequency_sweep do not appear to have changed.