add axis input to reset ring buffer #60
Conversation
|
@Poofjunior please take a look at this when you have a chance? I changed the functionality of the ring buffer based on the tigerbox for the new exaspim system. I also added set/get functions for the backlash parameter. |
src/tigerasi/tiger_controller.py
Outdated
| .. code-block:: python | ||
| box.get_backlash_compensation('x') # returns: {'X': 0.1} |
There was a problem hiding this comment.
It looks like this comment is out of date and should probs be get_axis_backlash
| self._set_cmd_args_and_kwds(Cmds.ARRAY, card_address=card_address, wait=wait) | ||
|
|
||
| def reset_ring_buffer(self, wait: bool = True): | ||
| def reset_ring_buffer(self, axis: str = None, wait: bool = True): |
There was a problem hiding this comment.
for reset_ring_buffer and _clear_ring_buffer, let's reference this page from ASI's docs in our docstring. Otherwise, the commands seem really random. For the syntax, have a look at move_relative or move_absolute method docstrings.
src/tigerasi/tiger_controller.py
Outdated
| else: | ||
| self._clear_ring_buffer(wait=wait) | ||
| self._rb_axes = [] | ||
|
|
There was a problem hiding this comment.
Let's consolidate these two functions into just one since they're only used here, and reset_ring_buffer appears to be a wrapper for _clear_ring_buffer without really adding anything.
|
It looks like this code got run through a formatter (black?), making the actual changes very difficult to read. I did run through the whole file, but I'll limit my review to the changes you're referring to. Ok, after reading the docs, I see why we need the change. It looks like RBMODE command is "card-addressed" (i.e: specific to a particular card), and that has to be specified first. I added some in-line comments, but here's the summary:
|
|
@Poofjunior I cleaned things up. Should be more readable now. Let me know if you have additional feedback. Thanks! |
src/tigerasi/tiger_controller.py
Outdated
| self._rb_axes = [] | ||
| def reset_ring_buffer(self, axis: str = None, wait: bool = True): | ||
| """Clear the ring buffer contents. | ||
| See `RING BUFFER MODULE <https://asiimaging.com/docs/ring_buffer>` |
There was a problem hiding this comment.
This needs an underscore at the end of it. (Gah! Restructured Text syntax is wonky.)
Like this:
`RING BUFFER MODULE <https://asiimaging.com/docs/ring_buffer>`_
|
Yayy--thanks so much! I'd say let's tweak that last change and then ship it! |
Poofjunior
left a comment
Poofjunior
left a comment
There was a problem hiding this comment.
Just one syntax change at this point in the docstring. Looks good otherwise! I'll pre-approve so you can merge after.
|
Thanks @Poofjunior. I will let you do the honors and merge! :) |
@Poofjunior when you have a chance please take a look at this PR. On the new ExA-SPIM system, the Tiger hardware axis that is being triggered for ring buffer moves is the Z axis (on a ZF card) (it is X axis on an XY card for the original system). For some reason, the original
reset_ring_buffercommand executes but does not actually reset the ring buffer. The solution is to allow an optionalaxisinput arg, i.e.axis=Zfor the new system. If no axis is given, the function behaves as previously.