search

labrad / servers

LabRAD servers
24 stars 21 forks source link

Introduce master oscilloscope server. #377

Closed DanielSank closed 7 years ago

DanielSank commented 7 years ago

Heck yes!

Suggested review strategy:

  1. Review oscilloscope_server.py as this defines the user interface.
  2. Review wrappers.py, the base class for oscilloscope wrappers.
  3. Review agilent.py. There is only one scope implemented so far. All settings have been tested on a real scope.

The interfaces here are not guaranteed to be backward compatible with any of the other scope servers.

DanielSank commented 7 years ago

@ejeffrey @maffoo @kunalq could one of you guys pick this one up?

DanielSank commented 7 years ago

@maffoo please see 055b7727c38a69df1371c527c9d6c28032fd06ba in which we use method factories to reduce granularity of the VISA command parametrizations and to reduce code replication. If you like the pattern I'll fill in the rest of the methods.

DanielSank commented 7 years ago

@maffoo bump

DanielSank commented 7 years ago

@maffoo bump

maffoo commented 7 years ago

@DanielSank, yes, the channel_method and global_method functions look great! I would suggest calling the first and last params parse_input and parse_output instead, but up to you. Also, note that NotImplementedError is an exception class, so you need parens to make an instance:

raise NotImplementedError()
ejeffrey commented 7 years ago

@maffoo: it is actually OK to raise an exception class rather than an instance. The exception machinery will create the instance for you. Yay python and lazy typing! But I agree that we should use the instance form since you need to do that if you want to pass an error message.

DanielSank commented 7 years ago

But I agree that we should use the instance form since you need to do that if you want to pass an error message.

@ejeffrey Huh! I did not know that.

DanielSank commented 7 years ago

@maffoo I think we're all done now. Just waiting for your LGTM.

maffoo commented 7 years ago

LGTM