driver for HP 83650A #516
driver for HP 83650A #516
Conversation
|
|
||
| """ | ||
| self.verbose = verbose | ||
| logging.debug(__name__ + ' : Initializing instrument') |
There was a problem hiding this comment.
We use the convention of having:
log = logging.getLogger(__name__)
| self.getall() | ||
|
|
||
| def getall(self): | ||
| logging.info(__name__ + ' : reading all settings from instrument') |
There was a problem hiding this comment.
Maybe debug instead of info? I'd also call the method print_all
| ': %e' % self.frequency.get(), self.frequency.units) | ||
| print(self.freqmode.label + ':', self.freqmode.get()) | ||
| self.getmodstatus() | ||
| # self.get_modstatus() |
There was a problem hiding this comment.
If it's commented out either comment on why, or ⚡️ ! :D
| # self.get_FM() | ||
|
|
||
| def getmodstatus(self): | ||
| print(self.fmstatus.label + ':', self.fmstatus.get()) |
There was a problem hiding this comment.
Same as above, I'd change name to print_modstatus
|
@giulioungaretti Fixed some of the comments. @brunobuijtendorp Can you look at the other ones (in particular related to renaming functions)? |
|
@peendebak will do tomorrow when in Delft :) |
|
@brunobuijtendorp PR was blocked because the formatting was not right. I did an autopep8 to fix it. @giulioungaretti PR is ready for inclusion |
|
@peendebak looks like something went wrong with the last commit ? It reverted some changes (e1d9ece) |
|
@giulioungaretti Thanks. It was not in the last commit, but the one before. I reverted the reverted changes. |
|
@peendebak Yes I will check it out tomorrow at qutech! Looks like in commit 'f9e2dabdab3fedfcdf9fe7cbd7a3e1fd94f99f04' I must have accidentally edited an old commit, thereby reverting the changes. Thanks for fixing that :) |
|
@eendebakpt, @giulioungaretti Changed units to unit in two more lines. Driver is tested and ready now. |
|
|
||
| """ | ||
| self.verbose = verbose | ||
| log.debug(__name__ + ' : Initializing instrument') |
There was a problem hiding this comment.
you can remove name here 🎇
There was a problem hiding this comment.
log.debug('Initializing instrument')
| docstring='Pulse source, ....') | ||
|
|
||
| def reset(self): | ||
| log.debug(__name__ + ' : Resetting instrument') |
| self.print_all() | ||
|
|
||
| def print_all(self): | ||
| log.debug(__name__ + ' : reading all settings from instrument') |
|
@brunobuijtendorp see review! then ready to merge! |
|
@giulioungaretti Requested changes have been committed. :) |
|
@brunobuijtendorp @eendebakpt thanks for the contribution! mergin' 💖 |
Author: Bruno <brunobuijtendorp@gmail.com>
driver: HP 83650A (#516)
@Unga @jenshnielsen @WilliamHPNielsen