Driver/buffered sr830 #653
Conversation
Add a data buffer parameter to the SR830 driver class
| The instrument natively supports this in its TRCL call. | ||
| """ | ||
|
|
||
| def __init__(self, name, instrument, channel): |
There was a problem hiding this comment.
I suggest that we start moving the typos from the docstring into type annotations. I.e.
def __init__(self, name: srt, instrument: SR830, channel: int):
and so on.
It's imho fine to leave the types in the doc strings too if you prefer.
There was a problem hiding this comment.
I'd say we get rid of all typos altogether! 😉
I totally agree with regard to the types.
| channel (int): The relevant channel (1 or 2). The name should | ||
| should match this. | ||
| """ | ||
| if channel not in [1, 2]: |
There was a problem hiding this comment.
I would make this an parameter and probably a tuple i.e. self._valid_channels = (1,2)
There was a problem hiding this comment.
Fair enough. Seems like better practice.
| self.setpoints = (tuple(np.linspace(0, N*dt, N)),) | ||
|
|
||
| params = self._instrument.parameters | ||
| # YES, it should be: "is not 'none'" NOT "is not None" |
There was a problem hiding this comment.
Is there a particular reason why you cant change it to None in the mapping dicts around l 467?
There was a problem hiding this comment.
I think conceptually there is a difference; the 'none' is the proper response string from the instrument. I would expect the python None to mean that the value doesn't exist, whereas 'none' in this case rather means OFF than "not defined" or "not present".
There was a problem hiding this comment.
Fair enough, perhaps we could avoid the confusion by calling it one of those names?
There was a problem hiding this comment.
'none' is what the manual calls it, but I'm fine with 'off' or even 'identity'.
There was a problem hiding this comment.
I don't have any strong opinion. It's probably better to match the manual then
| vals=Strings()) | ||
| self.add_parameter('ch{}_databuffer'.format(ch), | ||
| channel=ch, | ||
| parameter_class=ChannelBuffer) | ||
|
|
||
| # Data transfer | ||
| self.add_parameter('X', |
There was a problem hiding this comment.
How do these work, Do they always return from a specific channel?
There was a problem hiding this comment.
Yes. The internal buffer is filled simultaneously for both channels, which is why there is only one buffer_SR etc., but polling is done from either of the two (but not both).
| The instrument natively supports this in its TRCL call. | ||
| """ | ||
|
|
||
| def __init__(self, name: str, instrument: SR830, channel: int): |
There was a problem hiding this comment.
You might need to do quote i.e. do 'SR830' as a forward declaration because it's defined below the ChannelBuffer and not available when the class is defined
Author: William H.P. Nielsen <whpn@mailbox.org>
Driver/buffered sr830 (#653)
* fix: miniscule beauty fixes * feat: add buffer control parameters to driver * feat: Finish addition of data buffer Add a data buffer parameter to the SR830 driver class * fix: handle setpoint booleans correctly * docs: add example notebook for SR830 * fix: Respond to code review * fix: add proper forward reference * fix: remove accidental blank line
* fix: miniscule beauty fixes * feat: add buffer control parameters to driver * feat: Finish addition of data buffer Add a data buffer parameter to the SR830 driver class * fix: handle setpoint booleans correctly * docs: add example notebook for SR830 * fix: Respond to code review * fix: add proper forward reference * fix: remove accidental blank line
Fixes the fact one can not do buffered acquisition
Changes proposed in this pull request:
@jenshnielsen