Skip to content
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 for tektronics awg5200 #724

Merged
merged 10 commits into from
Sep 7, 2017

Conversation

peendebak
Copy link
Contributor

The driver is copied from the 5014 which is largely compatible. Changed:

  • the number of channels in the driver file (which is 8 for the 5200).
  • sending the waveform and marker to the device, which is a little bit different from the 5014

Not all new options of the new 5200 are supported, and not all the old options have been tested, but the device seems to work fine.

@Christian-Volk @jenshnielsen

Note: we driver could be combined using a shared base class, but this is not something we will implement in the near future.

@WilliamHPNielsen
Copy link
Contributor

Not all new options of the new 5200 are supported, and not all the old options have been tested [...]

That kind of leaves me not wanting to put this driver in the code base.

A quick diff reveals that you only changed the number of channels and made an "integer" into a "real" in the internal AWG file format. Wouldn't a subclassing of the AWG5014 then suffice for now? You'd just need to actually use the self.num_channels in the AWG5014 driver (and eventually add a self.num_markers) and then override send_waveform_to_list.

Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This falls under the good old "don't repeat yourself" rule. We should not have 1000+ lines of code maintained twice.

@peendebak
Copy link
Contributor Author

@WilliamHPNielsen I changed the driver to reuse the 5014 code. Full testing of the driver will not happen soon, we will only use the parts of the driver that we need for our experiments.

Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks much better! I agree that we should make a AWG5000series base class at some point in the future when it is more clear what the differences are between the 5014 and the 5200.

@WilliamHPNielsen WilliamHPNielsen merged commit 06fdbae into microsoft:master Sep 7, 2017
giulioungaretti pushed a commit that referenced this pull request Sep 7, 2017
Author: peendebak <P.T.eendebak@tudelft.nl>

    Driver for tektronics awg5200 (#724)
@eendebakpt eendebakpt deleted the feat/awg5200 branch November 12, 2018 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants