labrad / servers

LabRAD servers
24 stars 21 forks source link

Added methods for Setting AC voltage and frequency #350

Closed btchiaro closed 8 years ago

btchiaro commented 8 years ago

and for clearing the event register. Tested on Vince.

maffoo commented 8 years ago

I made some comments; mostly formatting nits. I know these are picky but it really helps to be consistent everywhere (at least with new code...).

My only concern is with the setting name change; is changing the name going to affect many people? Also, with setting names more generally, I might suggest just leaving off the name for settings altogether in which case the python method name will be used for the setting too. I think it was a bad idea originally to get in the habit of making setting names with spaces and caps, only to then mangle these for the purpose of making them callable from python. So instead of

@setting(14, 'set AC voltage', val='v[V]', returns='')
def set_ac_voltage(self, c, val = '1*U.V'):
    ...

just do

@setting(14, val='v[V]', returns='')
def set_ac_voltage(self, c, val = '1*U.V'):
    ...

Then the name will be "set_ac_voltage" whether referring to this setting by string name, or as a python method on the server object with the pylabrad client. We should have done this all along, but for new settings we can certainly adopt this style.

btchiaro commented 8 years ago

Hey @maffoo I made the changes that you suggested. I adopted the new style setting name convention everywhere, but I can change the set DC one back if you want. I think I'm the only one using this server at the moment, so I don't know if the name change would cause problems. I think its nice to be explicit about exactly what is being set and consistent with the AC case.

btchiaro commented 8 years ago

Hey @maffoo were you ok with this given my comment above? Or do you want me to change the "Set DC" back? Thanks.

maffoo commented 8 years ago

@btchiaro, sorry, I think I had a comment written and must have forgotten to actually submit it. Yes, LGTM.