-
Notifications
You must be signed in to change notification settings - Fork 301
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
Driver/awg70000Aseries #761
Driver/awg70000Aseries #761
Conversation
@WilliamHPNielsen Should I review this or do you want to wait till there is time to fix the issues? |
@jenshnielsen Let's wait until we actually have a unit to play with. |
the clock parameter 'sample_rate' needs a custom validator based on the clock source
… into driver/AWG70000A
@core this baby is ready for a review. |
unit='V', | ||
get_parser=lambda s: 2*10**((float(s)-10)/20), | ||
set_parser=lambda V: 10 + 20 * np.log10(V/2), | ||
vals=vals.Numbers(0.250, 0.500)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using [SOURce[n]]:VOLTage[:LEVel][:IMMediate][:AMPLitude] as Carl suggested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that seems more straightforward and less errorprone. I couldn't find that command in the manual, but I'll test is this afternoon.
event_jumps: List[int], | ||
event_jump_to: List[int], | ||
go_to: List[int], | ||
wfm_names: List[List[str]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these modified in place otherwise I would in general suggest using Sequence see http://mypy.readthedocs.io/en/stable/common_issues.html#invariance-vs-covariance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the docs... The above code will fail if I pass something which is a list
of a subclass of int
because mypy considers List
to be invariant? But immutable containers like Sequence
are covariant. Is this why Sequence
is preferred?
If so, I would say that it doesn't seem like such a big issue here, since we rarely subclass int
, but I could be overlooking something crucial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in general Sequence is preferable just to tell the user that it's imitable. The
above would in principle be wrong if you pass a tuple as go_to
event_jumps: List[int], | ||
event_jump_to: List[int], | ||
go_to: List[int], | ||
wfms: List[List[bytes]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence
… into driver/AWG70000A
Author: William H.P. Nielsen <whpn@mailbox.org> Driver/awg70000Aseries (#761)
Add a driver for the Tektronix 70000A series AWG.
Still very unfinished.
Changes proposed in this pull request:
Done so far:
.wfmx
files and transfer and load them.seqx
formatStill pending:
.seqx
format ( I guess we have that down now)Due to time constraints, the handling of the
.seqx
files is not well done. Two bugs that I know of:.seqx
file isinvalid if only one channel is specifiedcannot reproduce.seqx
file isinvalid if more than 20 waveforms are in it (at least if they have 100k points)that was a bug on our side that has been fixedAnd one thing that should be optimised is that the
.seqx
file is stored to disk locally upon creation, this has been fixed now.UPDATE: The
.seqx
file format is reasonably well understood now, thanks to support from Tektronix and more time spent staring at our code.@jenshnielsen