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.

Sign up for GitHub

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

Jump to bottom

[DRAFT] Add Rigol instrument #8

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[DRAFT] Add Rigol instrument #8

wants to merge 1 commit into from

Conversation

VitorRamos
Copy link

Draft of DS1102E support

Arkh42
Arkh42 requested changes Jul 7, 2021
View reviewed changes
Copy link
Owner

@Arkh42 Arkh42 left a comment

Choose a reason for hiding this comment

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

There is a problem with the code: the DS1102E inherits from Electrometer instead of Oscilloscope. I defined the minimum functions to implement in the oscilloscope.py file. I'm sending a commit ASAP.

@VitorRamos VitorRamos marked this pull request as draft July 7, 2021 11:35
@VitorRamos
Copy link
Author

Yes, this PR is not complete yet. I created this draft so that we can discuss the implementation details in case anything needs to change.
The DS1102E class, for now, is just a copy of one of the electrometers with the correct query format, the SCPI commands were also not updated yet.
For the oscilloscope class, I ​wasn't sure there's enough difference between an electrometer and an oscilloscope for this project, so I didn't create anything. But in this case, I wait for the base class.

@Arkh42
Copy link
Owner

Arkh42 commented Jul 7, 2021

The base class is now committed.
You can find it in https://github.com/Arkh42/rtestbench/tree/master/rtestbench/tools/oscilloscope.py

@Arkh42 Arkh42 assigned Arkh42 and unassigned Arkh42 Jul 7, 2021
@Arkh42 Arkh42 added the enhancement New feature or request label Jul 7, 2021
@Arkh42
Copy link
Owner

Arkh42 commented Jul 7, 2021

I also forgot to mention to write the corresponding test functions. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arkh42 Arkh42 Arkh42 requested changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@VitorRamos @Arkh42