add magnet control to triton driver #893
Conversation
| @@ -141,11 +225,10 @@ def _get_control_channel(self): | |||
| self._control_channel = i | |||
| break | |||
|
|
|||
| return self._control_channel | |||
There was a problem hiding this comment.
should this be removed. Looks like the last loop has no effect then?
| def _set_control_channel(self, channel): | ||
| self._control_channel = channel | ||
| self.write('SET:DEV:T{}:TEMP:LOOP:HTR:H1'.format(channel)) | ||
| self.write('SET:DEV:T%s:TEMP:LOOP:HTR:H1' % | ||
| self._get_control_channel()) |
There was a problem hiding this comment.
Any reason to rewrite using old style formatting
There was a problem hiding this comment.
Probably someone not familar with .format style formatting
| self.write('SET:SYS:VRM:COO:CART:RVST:MODE:RATE:RATE:' + str(s) + | ||
| ':VSET:[' + str(x) + ' ' + str(y) + ' ' + str(z) + ']\r\n') | ||
| else: | ||
| print('Warning: set sweeprate in range (0 , 0.2] T/min') |
There was a problem hiding this comment.
Should perhaps clarify that this did nothing because it's out of range
| get_cmd = 'READ:DEV:%s:TEMP:SIG:TEMP' % chan, | ||
| get_parser = self._parse_temp) | ||
| self.chan_temps = set(self.chan_temps) | ||
| def _get_temp_channels(self, file): |
There was a problem hiding this comment.
are these changes intentional, They dont seem to relate to the magnet?
There was a problem hiding this comment.
This PR is a merge for an old branch of qcodes. There indeed seem to be some changes not strictly related to the magnet
|
@jenshnielsen I addressed some of your comments. I did not write the changes myself, so I cannot comment any further. |
Codecov Report
@@ Coverage Diff @@
## master #893 +/- ##
==========================================
+ Coverage 77.92% 79.64% +1.72%
==========================================
Files 37 79 +42
Lines 5236 10242 +5006
==========================================
+ Hits 4080 8157 +4077
- Misses 1156 2085 +929 |
|
@jenshnielsen Should I do something with the codecov report? |
|
@eendebakpt There are no tests for the driver hence the coverage stays the same as it is. No need to do anything |
|
@jenshnielsen What is the status of the PR? |
|
It still have changes not related to the magnet that I don't think belongs here |
peendebak
force-pushed
the
update/triton
branch
from
834f24a
to
58f8e83
Compare
|
@jenshnielsen I reverted the changes not related to the magnet. We will keep track of these changes locally. |
|
@jenshnielsen Is this one good to merge? |
|
Yes it looks fine now |
Author: peendebak <P.T.eendebak@tudelft.nl>
add magnet control to triton driver (#893)
Control of the magnet is added.
@MerlinSmiles @jenshnielsen @WilliamHPNielsen The merge of the driver was not easy, and I cannot test all features here. Can you have a good look at the PR diff?
@gzheng @MNoordam