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

Drivers: structure of the package and drivers for DC power source #9

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

Conversation

MatthieuDartiailh
Copy link
Member

So far only the "standard" for DC power source is implemented and used for:

  • Itest
    • BE2101: single output precision voltage source
    • BE2141: four outputs precision voltage source
  • Yokogawa:
    • GS200
    • 7651 (we use the prefix Model since 7651 is not a valid class identifier in Python)
  • Keysight/Agilent/HP:
    • E3631A (triple output DC power source +6/+25/-25 V, used in the lab to power amplifiers)
    • E3633A, E3634A (single outputs, not present in te lab and hence not tested).

@Exopy/owners I would appreciate if you can give this a look, hopefully the documentation (http://i3py.readthedocs.io/en/latest/?badge=latest) should be enough to understand the magic happening behind the scene. I would appreciate to get as much feedback as possible since this is the future for drivers.
I will work on implementing a generic task giving access to any feature/action of such a driver which will reduce the burden of developing super specific task for just setting a parameter.

@codecov
Copy link

codecov bot commented May 17, 2018

Codecov Report

Merging #9 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master       #9   +/-   ##
=======================================
  Coverage   98.68%   98.68%           
=======================================
  Files          28       28           
  Lines        2282     2282           
=======================================
  Hits         2252     2252           
  Misses         30       30

@lcontami
Copy link
Member

A few quick documentation questions:
section 1.5 (subsystems)
"By default, a subsystem is a subclass of || " ?

section 4.1.1
" first class of the mro" ?

@MatthieuDartiailh
Copy link
Member Author

Just pushed a documentation fix. The quick answer is |SubSystem| for the first and mro: Method resolution order for the second (hence we use the left most class in the class declaration that defines a particular subsystem/channel).

Copy link
Member

@lcontami lcontami left a comment

Choose a reason for hiding this comment

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

Had a quick look, didn't test anything on a real source.

The Bilt driver is not done right ? I can see different elements coded but I don't see how they come together.

#: Care should be taken that this value may not be up to date if a
#: failure occurred. To know the current status of the output use
#: read_output_status, this feature only store the target setting.
o.enabled = Bool(aliases={True: ['On', 'ON', 'On'],
Copy link
Member

Choose a reason for hiding this comment

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

'On' written twice; I guess you meant on

o.voltage_range = Float(unit='V')

#: How does the source behave if it cannot reach the target voltage
#: because its reached the target current first.
Copy link
Member

Choose a reason for hiding this comment

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

typo: its -> it
(idem later on for voltage_limit_behavior)

o.voltage = Float(unit='V')

#: Range in which the voltage can be set.
o.voltage_range = Float(unit='V')
Copy link
Member

Choose a reason for hiding this comment

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

Is a float sufficient ? Don't you need a tuple ?
(certain sources can have a range - lim -> + lim, others 0 ->lim.

Copy link
Member Author

Choose a reason for hiding this comment

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

For the one I add access to so far it is sufficient. But a source can add stricter limitation like the Bilt does. The fact is that there is never a direct mapping between the range and the associated limits.


@o
@Action()
def read_output_status(self) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Waou what is this syntax ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it the type annatotation that bothers you or the double decoration ?

Copy link
Member

Choose a reason for hiding this comment

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

the type annotation. I saw the python page about it. I didn't know it existed !

"""Driver for the Yokogawa GS200 DC power source.

Because the over-voltage (current) protection is always enabled in
current (voltage) mode, they basically act as limits and fully the same
Copy link
Member

Choose a reason for hiding this comment

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

fully -> fullfill ?

o.voltage_range = set_feat(getter=True,
setter=':SOUR:RANG {}',
checks=(None, 'driver.mode == "voltage"'),
values=(10e-3, 100e-3, 1.0, 10.0, 30.0),
Copy link
Member

Choose a reason for hiding this comment

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

You could use your VOLTAGE_RESOLUTION keys instead of rewriting it (idem below)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do

if self.mode == 'current':
ran = to_float(self.current_range) # Casting handling Quantity
res = CURRENT_RESOLUTION[ran]
if ran != 200e-3:
Copy link
Member

Choose a reason for hiding this comment

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

Great behavior from the source :D

@MatthieuDartiailh
Copy link
Member Author

The bilt is complete and tested (just like the others, as far as the test scripts do their job at least). You need to look at the base class of the rack which declares the modules it can drive.

@bilderbuchi
Copy link

FYI there's a pytest notice in the CI log:

pytest-catchlog plugin has been merged into the core, please remove it from your requirements.

@MatthieuDartiailh
Copy link
Member Author

Thanks @bilderbuchi I fixed that in other repos but I forgot this one.

@MatthieuDartiailh
Copy link
Member Author

@lcontami @galactikvoyager feel free to merge when you have no more questions.

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.

4 participants