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

Add support for Keysight 34411A #2927

Merged
merged 17 commits into from
Apr 20, 2021

Conversation

janekfleper
Copy link
Contributor

Changes proposed in this pull request:
-Correctly implement the Keysight 3441XA series. A few settings for the 34410A were wrong/missing and the 34411A was not considered at all
-Set the correct DC voltage ranges for all models supported by the _Keysight_344xxA class

According to the Data Sheet of Keysight 34410A and 34411A, page 5
According to the Data Sheet of Keysight 34410A and 34411A, page 5
There were to many resolution factors in the list. 34410A only has eight possible NPLC values.
According to the Data Sheets of both generations (3441x and 3446x/3447x) the range for the DC voltages is 100mV to 1kV (in steps of x10).
The values in the attribute 'ranges' were stating the resistance measurement ranges. The attribute was however used for the DC voltage range.
According to the Data Sheet of Keysight 34410A and 34411A, page 6
I'm really not sure about the upper limits for the 60Hz line frequency. For 50Hz the limits correspond to the values given in the Programming Reference.
The name was initially changed in 45e41e9. Since DC Voltage is implemented as the default case, I decided to change it back. Otherwise, the methods 'increase_range()' and 'decrease_range()' would've needed to be changed to use the new attribute name.
@jenshnielsen jenshnielsen added this to the 0.25.0 milestone Apr 14, 2021
In 45e41e9 the 'ranges' attribute was changed to a list with fewer elements. The function 'test_increase_decrease_range()' therefore failed.
@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #2927 (2308c98) into master (5772051) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##           master    #2927   +/-   ##
=======================================
  Coverage   65.31%   65.31%           
=======================================
  Files         208      209    +1     
  Lines       28031    28038    +7     
=======================================
+ Hits        18308    18313    +5     
- Misses       9723     9725    +2     

@jenshnielsen
Copy link
Collaborator

@astafan8 I think this looks fine to merge what do you think?

@astafan8 astafan8 enabled auto-merge April 20, 2021 15:44
@astafan8 astafan8 merged commit d7d0c63 into microsoft:master Apr 20, 2021
@janekfleper janekfleper deleted the keysight_34411a_support branch May 5, 2021 08:20
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

3 participants