Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-channel instrument consistency #156

Open
scasagrande opened this issue Oct 28, 2016 · 5 comments
Open

Multi-channel instrument consistency #156

scasagrande opened this issue Oct 28, 2016 · 5 comments

Comments

@scasagrande
Copy link
Contributor

So I've been working on the PR for the Minghe MHS5200 (PR #150). The problem here is that this is a multi-channel function generator, which was not taken into account in the original FunctionGenerator ABC. What I'm trying to do is make sure that a user can go from a single-channel fgen to a multi-channel fgen without having to change any of their program code.

Here is an example use case. Lets say a user starts with a single-channel fgen, like so:

fgen.amplitude = (2 * pq.V, fgen.VoltageMode.rms)
fgen.frequency = 10 * pq.MHz

Then, the user moves up to a multi-channel function generator, but is only using the first channel. This is not a new problem for IK; we already have single-/multi-channel instruments (eg, power supplies). However in this case, the FunctionGenerator.amplitude property is the tricky one. The ABC provides a concrete getter/setter, which in turn call _get_amplitude_ and _set_amplitude. This is because the concrete impl provides the support for amplitudes specified in Vpp, Vrms, or dBm.

After playing with different combinations of ABCs, conditional decorators, etc etc, I think the best course is to do some refactoring. What I'm proposing is that, for development, we treat all of these single- vs multi-channel instruments as multi-channel, but with just one channel.

So how would this work? Basically in development, you would work assuming you are developing a >1 channel device, but you would just set _channel_count = 1 in the constructor. Then, by inheriting from FunctionGenerator, you also get a set of concrete properties that pass requests onto the appropriate channel.

Let me show you:

# AB-property defined in FunctionGeneratorChannel
fgen.channel[0].frequency = 10 * pq.MHz

# Concrete property defined in FunctionGenerator that passes your calls on to self.channel[0].frequency
fgen.frequency = 10 * pq.MHz

# Uses the current concrete getter/setters for amplitude, but moved into FunctionGeneratorChannel
# which is also where _get_amplitude_ and _set_amplitude_ are now defined
print(fgen.channel[0].amplitude)

# Just like for frequency, this passes the calls onto the first channel object
print(fgen.amplitude)

Now you'd have a consistent API independent of which direction (single to multi, or multi to single) you move in. It also simplifies development, since all instruments of the same type are developed in the exact same way.

It does leave one open question about implementation specifics, but I'll type that up later.

@scasagrande
Copy link
Contributor Author

I scrapped all my progress on this and started again. I finally have the start of this refactor with all the tests passing.

@bilderbuchi bilderbuchi mentioned this issue Feb 7, 2019
6 tasks
@scasagrande
Copy link
Contributor Author

I am currently tackling this in the multichannel_consistency branch. I had to make a compromise with the abstract property decorators.

The approach I'm taking is to attach a _channel_count property to all instruments. This is added in the abstract instrument definition, and defaults to 1. This way instruments that inherit from these abstract instruments, and only have 1 channel, don't need to do anything.

Next, the abstract classes are given an abstract Channel child class. The parent and child are linked via the usual ProxyList helper. Next, the parent instrument, and the child channel classes both have the same API stubs. For example, for the FunctionGenerator, we will now have FunctionGenerator.frequency and FunctionGenerator.Channel.frequency.

For both of these functions, they conditionally call the other, based on the value of _channel_count. If there is 1 channel, then the parent instrument is where the concrete implementations are, and calls to the child are forwarded to the parent. Similarly, if _channel_count > 1, then the concrete implementations are at the Channel level, and calls to the parent instrument are forwarded to the 0th channel.

This change is a drop-in improvement for all the existing instrument drivers that inherit from these base instruments and only have 1 channel. It gives a consistent API across similar instruments independent of the number of channels.

@scasagrande
Copy link
Contributor Author

Taking a look at the changes required to do this for the PowerSupply base class, we'll need to change the contract for the HP6624a. That set of the changes should go in with v1 release.

@trappitsch
Copy link
Contributor

Since this is on the list for v1 as well, I wanted to see if I can help out with it as well (I don't there touch the async task...). As far as I can see, the FunctionGenerator is the only class where a single channel option is cleanly implemented. The signal_generator also has a single channel option, however, that one is kind of messy at this point and for sure needs some work. Was your intention to have all abstract instruments to have a single channel option?

@scasagrande
Copy link
Contributor Author

Yeah, FunctionGenerator is the one that I re-implemented from scratch, so hopefully its alright.

That goal is something similar for most of the base images, but maybe this isn't needed for v1. And honestly the async stuff should just wait. We can keep adding features for years and just continue the versioning paralysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants