Rewrite Keysight N52xx base driver

[sohailc]

While developing a driver for the N5222B, I noticed some inconveniences.

1. If the trigger source is set incorrectly then the run_sweep function can get stuck in an infinite loop. If the user aborts because of this, provide a useful hint of what can be wrong
2. Add a parameter to change the trigger source.

[codecov[bot]]

Codecov Report

Merging #1244 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1244   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files          52       52           
  Lines        7201     7201           
=======================================
  Hits         5702     5702           
  Misses       1499     1499

[sohailc]

After a through review of the Keysight N52xx driver I came to the conclusion that a significant modification and improvement is necessary. The following issues need to be addressed in the current driver:

1. There is no way to add a trace. In the example notebook it is assumed that some traces exist already. If this is not the case the driver fails when selecting the trace (e.g. pna.trace("S21")).

2. The current driver uses ArrayParameter to return trace data. This will be deprecated in due course.

3. Many commands on the N52xx are channel specific, yet the current driver omits the channel numbers all together. This will make operations default to channel 1. There is no way to modify which channel we want to operate on.

For this reason a new base class has been written. The following modifications have been made to address these issues:

1. Introduce methods add_trace, delete_trace and delete_all_traces.

2. Instead of using ArrayParameter, methods return numpy arrays directly. This will break compatibility with he old loop but as that is being phased out anyway we do not mind.

3. Create a channel class N52xxChannel and most parameters live here. Since we almost always use channel 1, parameters on this channel are accessible directly on the main instrument as well so that pna.power (for instance) points to pna.channel[0].power. If the user wants, (s)he can modify the default channel.

@spauka can you review this? Especially look at the new example notebook.

[sohailc]

@spauka Thank you for your comments. I think a lot of your comments stem from my misunderstanding of what a port and channel is. They are not the same thing. I removed support for a port class because I had the impression these are the same thing as channel. I will revise my design accordingly.

In the meantime, can you provide examples of the types of sweeps you need to make? A script where you are performing them would be great.

We also have to talk about ArrayParameter and how arrayed valued data is returned. After talking to @WilliamHPNielsen and @jenshnielsen I confirmed that we are phasing out ArrayParameter. The link between a measurement parameter and instrument where the data originates from is provided by how we register parameters in the data set.

[spauka]

Yep, agreed that ArrayParameter will be deprecated. Just want to confirm that the replacement will be using register_custom_parameter as used in this particular driver. If this is the case, as shown in the example notebook, measurement parameters are NOT associated with where the data comes from since register_custom_parameter doesn't have a concept of an instrument?

Sounds like you figured it out, but just to confirm, ports as referred to in the original driver refer to the physical connections on the front of the machine. On our N5245A for example, it is possible to output up to two signals simultaneously at different powers and potentially at offset frequencies (FOM), used for characterizing mixers for example.

Channels are a collection of settings on the PNA, like you have used them. For example it is possible to set up a S21 sweep across two different frequency spans using channels, but it is important to note that only one channel is active at a time. The concept of channels is also how measurement classes are implemented, but this will be a difficult feature to support.

(Also forgot to mention mypy failures which I assume you're aware of?)

[spauka]

@sohailc, given the latest changes to the driver, and the understanding of channels/ports, do you think it's worth revisiting modifying that one/merging these two files together such that there aren't two versions of the driver with slightly varying interfaces? The latest changes seem to make it quite similar to the previous version of the N52xx base?

Based on a quick look, I don't think it's too much work to make the two drivers compatible...

One additional note, it would still be useful to be able to change/read out what parameter a given trace measures, or at least read it out. This is most often the mode that will be used when we are measuring. Often, a user will set up a trace on the PNA, then measure it after the fact, so being able to read out the measured parameter is a necessary feature. Unfortunately changing the parameter doesnt change the name of the trace so it would be worth thinking about how labelling traces works best in the driver.

Incidentally, this was the mistake in the original PNA base driver that caused your earlier issues, I had assumed that the name updates when the trace is changed, however this turned out to be untrue.

[sohailc]

@spauka I think there is a bit of a mis understanding (sorry for that). I never intended to make two different version of the base driver. The "N52xx_new" is something temporary and I did not intend for things to be merged like this. Indeed there should only be one driver, it would be crazy otherwise.

I am working to merge both drivers as you suggest.

Thank you for your comments and I will investigate how to accomplish what you suggest. Also, I am building an interface to create windows and adding traces to that window.

[sohailc]

@spauka can we have another look at this? Specifically, look at https://github.com/sohailc/Qcodes/blob/doc/PNA_N5222B_example/docs/examples/driver_examples/Qcodes_example_with_Keysight_Network_Analyzer.ipynb

I show that the functionality and the interface of the re-written driver is mostly the same as the driver you wrote. There is still some work to be done, but comments are welcome

[spauka]

I'll have a play around shortly. In the mean time, I've pushed changes to the existing driver that fixes some issues, it would be good to pull this in the mean time?

[astafan8]

@sohailc do you know if the work on this PR will continue?

[astafan8]

@sohailc do you know if the work on this PR will continue? Or shall we close this PR?