Skip to content

Keysight Oscilloscope Implementation with pyVISA #129

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

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

Kimu754
Copy link

@Kimu754 Kimu754 commented May 13, 2025

Hi,
I am a physics student working with the APQ (Atoms-Photons-Quanta) group at the University of Darmstadt.
As part of my work, I integrated Keysight oscilloscopes into labscript which may be a nice addition for the community. It basically allows you to configure an osci via LAN or USB and acquire data on an external trigger. The oscilloscope configurations can be set on the device, saved in one of the config slots via a button in the blacs tab, and then be loaded by its id in the experiment shot. All configurations are displayed in blacs with their current values. Please see the README file for further information.
I will be happy to answer any questions.

Best reagards
Kerim Takouti

@dihm
Copy link
Contributor

dihm commented May 15, 2025

Hello Kerim! Thank you for this addition! It looks very good. Scopes have always been an annoyance in labscript since they have so many inter-linked parameters that fully specifying them is a pain. Leveraging the internal saved-configs to provide some level of programmability while maintaining typical manual configuration by the user is a nice compromise.

It is going to be a little while before I have time to fully review this (at which point I will no doubt have many little suggestions and probably a few larger ones). I'll ask a few questions now from my first read-through of the code, mostly focusing on the capabilities definition/connection_manager machinery:

  • Could you help me understand your reasoning behind how capability definitions work? When it comes to complicated capabilities definitions, I would normally lean towards something more like what labscript does for NI_DAQs. Though I don't think you ought to go all in on creating a local database with introspected capabilities, I do like that the labscript device just takes all the relevant parameters as instantiation arguments. What this means is you can use the device in a script by just providing the correct arguments in the script, without having to necessarily use a ton of other infrastructure for defining things. Especially given that your parameters are fairly limited in number, I'd lean towards just passing them as independent arguments (or a dictionary if you expect things to expand significantly in the future).
  • Of course, now that I've looked back again, it appears the capabilities aren't used for anything, beyond the serial number. But I would argue that the serial number shouldn't be in the capabilities. What if someone has more than one scope of the same model? I'd be much more comfortable with the serial number being an instance argument the user passes in the script.
  • How does your code handle scopes that only differ in number of channels? For example, the 1204g and the 1202g. I think it could be nice to slightly generalize the osci_capabilities to include number of channels, so we don't necessarily need a new config for every single model number Keysight dreams up. Ultimately, I'd like to see this code handle scopes beyond the 12xx series since the interfaces for all their scopes are highly similar.
  • I would suggest renaming the interfacing class/files to something distinct from the labscript one (ie KeysightScope is the labscript-device class, the low-level interface class, and the filename of the low-level interface class). Renaming KeysightScope.py and the contained KeysightScope class to something distinct will limit future confusion and confusing import errors.
  • connection_manager.py has a few random things in it that would probably fit better in a separate utils or the like (that way other files aren't importing your connection manager just to get unit conversions).

Kimu754 added 5 commits May 26, 2025 12:53
… logic into LabscriptDevice

    Refactored connection logic: Removed the ConnectionManager class and implemented its core functionality directly in the LabscriptDevice class, enabling connection to the oscilloscope based on serial_number.

    Deprecated unused code: Removed oscilloscope capabilities since they are not currently in use.
@Kimu754
Copy link
Author

Kimu754 commented May 27, 2025

Hello David, I appreciate your feedback on this. Here’s my response to your questions/comments:

Could you help me understand your reasoning behind how capability definitions work? When it comes to complicated capabilities definitions, I would normally lean towards something more like what labscript does for NI_DAQs. Though I don't think you ought to go all in on creating a local database with introspected capabilities, I do like that the labscript device just takes all the relevant parameters as instantiation arguments. What this means is you can use the device in a script by just providing the correct arguments in the script, without having to necessarily use a ton of other infrastructure for defining things. Especially given that your parameters are fairly limited in number, I'd lean towards just passing them as independent arguments (or a dictionary if you expect things to expand significantly in the future).
Of course, now that I've looked back again, it appears the capabilities aren't used for anything, beyond the serial number. But I would argue that the serial number shouldn't be in the capabilities. What if someone has more than one scope of the same model? I'd be much more comfortable with the serial number being an instance argument the user passes in the script.

  • My goal while implementing the oscilloscope was to minimize the effort required from the user by avoiding the need to specify information already known by both the user and the device (unless necessary for initialization). However, some parameters are not inherently known to the oscilloscope and need to be provided manually. The capabilities you saw are legacy code from an earlier development phase. Initially, I attempted to expose them through the connection_table properties, so they could still be accessible to the user. However, I never ended up using them in testing, and wasn’t sure how to integrate them effectively. After developing the GUI, I’ve reconsidered their relevance and now believe they’re redundant. To simplify things, I’m removing the connection_manager class and moving its essential functionality into the device class. Since the capabilities aren't used anywhere in the current workflow, I'm removing them too. Initialization now only requires the user to specify the serial number of the scope.

How does your code handle scopes that only differ in number of channels? For example, the 1204g and the 1202g. I think it could be nice to slightly generalize the osci_capabilities to include number of channels, so we don't necessarily need a new config for every single model number Keysight dreams up. Ultimately

  • The implementation is already flexible, since the number of channels is handled internally. Scopes know how many channels they have and whether those channels are hidden or displayed. So my thinking was: no need to make the user state the obvious. The idea actually comes from the TekScope implementation, where the scope only reads data from displayed channels. This keeps the device usage intuitive, users can just press the display button on the oscilloscope to choose which channels to read from. Of course, any changes to the oscilloscope configuration need to be saved and loaded in BLACS via the GUI. So even if Keysight releases a ten-channel scope, it wouldn’t be a problem.

I'd like to see this code handle scopes beyond the 12xx series since the interfaces for all their scopes are highly similar.

  • If the implementation is successful, I’ll be happy to go through the code and check for compatibility with all models if needed.

I would suggest renaming the interfacing class/files to something distinct from the labscript one (ie KeysightScope is the labscript-device class, the low-level interface class, and the filename of the low-level interface class). Renaming KeysightScope.py and the contained KeysightScope class to something distinct will limit future confusion and confusing import errors.

  • I totally agree.

connection_manager.py has a few random things in it that would probably fit better in a separate utils or the like (that way other files aren't importing your connection manager just to get unit conversions).

  • connection manager was probably a bad idea. I dropped it in the new version and handled initialization directly in the labscript device using the serial number.

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.

2 participants