labrad / servers

LabRAD servers
24 stars 21 forks source link

U/brooks/infiniium scope server cleanup #367

Closed brooksfoxen closed 8 years ago

brooksfoxen commented 8 years ago

Update infiniium scope server to use read_raw to pull binary data.

Adds two OPC? queries to the RST and *CLS writes (Keysight suggested this as a best practice).

Also fixed a lot of white space issues and camel case variable names.

maffoo commented 8 years ago

I just have a couple of small remaining comments, but I think this is coming along nicely. This is a huge improvement over what we had before, so thanks for working on this! With all these changes, especially parsing the binary data, it would be good to test this out. Can you run it on some real hardware with the new gpib server?

maffoo commented 8 years ago

Two small comments, then LGTM from a code perspective. Please test with a scope before merging. And thanks again, this is great!

brooksfoxen commented 8 years ago

I tested through all the functions above get_trace so far and caught some minor things. I haven't figured out what I broke here though.

In [80]: scope.get_trace(1)
---------------------------------------------------------------------------
Error                                     Traceback (most recent call last)
<ipython-input-80-f55e13445fe5> in <module>()
----> 1 scope.get_trace(1)

C:\pylabrad\labrad\client.pyc in __call__(self, *args, **kw)
     65                           "Use setting.future(...) instead.")
     66         f = self.future(*args, **kw)
---> 67         return f.result() if wait else f
     68
     69     def future(self, *args, **kw):

C:\pylabrad\labrad\concurrent.pyc in result(self, timeout)
     98     def result(self, timeout=None):
     99         if not self.__done:
--> 100             self.__result = super(MutableFuture, self).result(timeout)
    101             self.__done = True
    102         if self.__error is not None:

C:\Python27\lib\site-packages\concurrent\futures\_base.pyc in result(self, timeo
ut)
    403                 raise CancelledError()
    404             elif self._state == FINISHED:
--> 405                 return self.__get_result()
    406             else:
    407                 raise TimeoutError()

C:\Python27\lib\site-packages\concurrent\futures\_base.pyc in __get_result(self)

    355     def __get_result(self):
    356         if self._exception:
--> 357             raise type(self._exception), self._exception, self._tracebac
k
    358         else:
    359             return self._result

Error: (0) [Agilent Infiniium Oscilloscope] Remote Traceback (most recent call l
ast):
  File "C:\pylabrad\labrad\server.py", line 232, in request_handler
    result = yield setting.handleRequest(self, c.data, flat_data)
  File "C:\Python27\lib\site-packages\twisted\internet\defer.py", line 1099, in
_inlineCallbacks
    result = g.send(result)
  File "agilent_infiniium_scope.py", line 307, in get_trace
    # Transfer waveform data
  File "agilent_infiniium_scope.py", line 330, in _parsePreamble
    def _parsePreamble(preamble):
exceptions.NameError: global name 'preamble_dict' is not defined
 [payload=None]
brooksfoxen commented 8 years ago

Oh, it looks like I just have to initialize preamble_dict before setting the values like this?:

    preamble_dict = {}
    for (key, included), val in zip(PREAMBLE_KEYS, preamble_vals):
        if included:
            preamble_dict[key] = val
brooksfoxen commented 8 years ago

Alrighty, everything seems to be working well now. I added some exceptions to make sure channel was in [1,2,3,4] in a few places, and I also added a check in get_trace to make sure that the requested channel was enabled.

I pulled data from the device and plotted it and it seems to be scaled correctly and everything.

All good to merge?

brooksfoxen commented 8 years ago

ugh, it seems I accidentally pulled the gpib_server changes from master into this branch as well... do I have to figure out how to pull that back out, or can it just be re-merged with master?

maffoo commented 8 years ago

I would suggest pulling out the gpib changes from the branch; I'm not sure how github will merge those. It is reporting that there are no conflicts with master, but still showing the files as changed, which is odd and makes me worried. git rebase -i should be able to fix this up easily enough; the pyle contributing file describes how to do this if you're unfamiliar.

brooksfoxen commented 8 years ago

Jimmy just helped me rebase. Did you want a separate pull request for the channel string check, or just a separate commit in this pull request?

maffoo commented 8 years ago

By "channel string check" do you mean the _check_channel function, or something else? It's find to include that function in this PR, so if that's all you meant them LGTM.