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

Alazar add buffer completed callback #1234

Merged
merged 8 commits into from
Aug 28, 2018

Conversation

jenshnielsen
Copy link
Collaborator

The idea is to enable the user to perform an action such as stepping another instrument once a buffer is complete. There is a small performance hit of this. According to my benchmarks this costs ~80 ns per iteration if the function is a noopt.

Also fix some invalid parameters in the Alazar 9360 driver

This can be used to step another instrument or trigger something when a buffer is complete
while (buffers_completed < self.buffers_per_acquisition.get()):
# Wait for the buffer at the head of the list of available
# buffers to be filled by the board.
buf = self.buffer_list[buffers_completed % allocated_buffers]
buf = self.buffer_list[buffers_completed % allocated_buffers ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before ]?

@@ -1227,3 +1228,6 @@ def post_acquire(self):
"""
raise NotImplementedError(
'This method should be implemented in a subclass')

def buffer_done_callback(self, buffer_number):
Copy link
Contributor

@astafan8 astafan8 Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buffer_number or buffers_completed? and, no docs again? :(

Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't really say anything again

@codecov
Copy link

codecov bot commented Aug 17, 2018

Codecov Report

Merging #1234 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1234   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files          52       52           
  Lines        7201     7201           
=======================================
  Hits         5702     5702           
  Misses       1499     1499

@@ -679,8 +680,9 @@ def acquire(self, mode=None, samples_per_record=None,
# -----set final configurations-----

# Abort any previous measurement
# With AlazarAbortAsyncRead in a finally clause below
# this should be fully redundant and is rather slow
self._call_dll('AlazarAbortAsyncRead', self._handle)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it "should be" redundant, shall this line be removed?

@astafan8
Copy link
Contributor

@jenshnielsen Do you think we can merge this, or is there more stuff coming? (to me it seems that this can be safely merged.)

@jenshnielsen
Copy link
Collaborator Author

I think this is safe to merge. Lets remove the redundant line first

@jenshnielsen jenshnielsen merged commit 7fe1643 into microsoft:master Aug 28, 2018
@jenshnielsen jenshnielsen deleted the alazar_changes branch August 28, 2018 08:28
giulioungaretti pushed a commit that referenced this pull request Aug 28, 2018
Merge: 6d41f51 53fef14
Author: Jens Hedegaard Nielsen <jenshnielsen@gmail.com>

    Merge pull request #1234 from jenshnielsen/alazar_changes
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

2 participants