add drivers for SRS SIM928 and Advantech PCIE-1751 #566
Conversation
| modules = [] | ||
| for i in range(1,10): | ||
| if CTCR & 1 != 0: | ||
| modules.append(i) |
There was a problem hiding this comment.
The code assumes that if there is a module in the slot, it must be a SIM928, while this is not always the case. Having an additional check here for a SIM928 solves this problem. For example:
model = self.get_idn(i)['model']
if model == 'SIM928:
modules.append(i)| def get_voltage(self, i): | ||
| return float(self.ask_module(i, 'VOLT?')) | ||
|
|
||
| def set_smooth(self, voltagedict, equitime=False, verbose=False): |
There was a problem hiding this comment.
This is a very useful function! We usually give the SIM modules a name, such as top_gate, and then forget about the slot number it is in. Would it make sense for the voltagedict to also allow parameter names as keys? It can then retrieve the corresponding slot?
| self.add_parameter('IDN{}'.format(i), | ||
| label="IDN of module {}".format(i), | ||
| get_cmd=partial(self.get_idn, i)) | ||
| self.add_parameter('voltage{}'.format(i), unit='V', |
There was a problem hiding this comment.
We usually give the modules names, such as top_gate etc. Perhaps a way to also give custom names to the SIM modules would be useful? For instance, an optional kwarg that is a dict with items of the form slot: name?
|
Hey @antsr thanks a lot for the PR! We also use the SIM928 voltage sources, and this driver would definitely be useful. I had a few comments, some of them I also addressed in the code.
If you want to implement any of this and would like some help, let me know! |
Add the option of naming SIM928 modules and properly document the class. Make minor changes to follow PEP8 style.
|
Hi @nulinspiratie, thanks for the feedback. I think the additions you proposed would indeed be very useful. I added the possibility to name the SIM928 modules -- would such implementation suit your needs? If you could tell me the commands you use to reset the modules reliably, I could easily also add it to the driver. |
|
Hey @antsr the naming of slots looks good! The reset command for a single slot is |
| from functools import partial | ||
| import collections | ||
| import os | ||
| import logging |
There was a problem hiding this comment.
We use the convetion of
log = logging.getLogger(__name__)
to get nicer logggin per module!
| raise Exception(' '.join(errors + warnings)) | ||
| return errors + warnings | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
shouldn't this be a static method ?
|
@antsr looking good, thanks for the contribution. |
|
I left a few unused variables in in the SIM928 driver, where it made sense to have all the status bits named as in the manual. Otherwise I addressed all the change suggestions and codacy issues. |
|
@antsr yes, good call! Clarity wins in this case! Merging! |
Author: Ants Remm <ants.remm@gmail.com>
driver: add SRS SIM928 and Advantech PCIE-1751 (#566)
Changes proposed in this pull request:
VisaInstrumentdriver for Stanford Research Systems SIM928 module inside a SIM900 mainframecffifor calling methods from the DLL@giulioungaretti @WilliamHPNielsen