-
Notifications
You must be signed in to change notification settings - Fork 10
These were left on the cryofree computer; there is: #52
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
base: main
Are you sure you want to change the base?
Conversation
- some functions for using the psa - a small change in the acquisition card driver - a few errors corrected ? - new safeguards for the cryomagnetic sources that are super buggy during communication
if self.condition_callable(): | ||
return True | ||
if remaining_time < 0 or break_condition_callable(): | ||
return False | ||
time.sleep(min(refresh_time, remaining_time)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is meaningless, we check the condition before entering the while loop so checking again after a couple of operations won't do any good.
@@ -216,6 +222,8 @@ def get_traces(self, channels, duration, delay, records_per_capture, | |||
get_data = self._dll.GetData.func | |||
retrieved_records = 0 | |||
while retrieved_records < records_per_capture: | |||
# WHY ??? | |||
time.sleep(0.002) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why indeed? Depending on the length of the traces this could lead to a large accumulation of traces and slow down the acquisition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fix for various intermittent and hard to debug issues we had and it was absolutely needed for us at the time. I suspect that this solves some sort of race condition. It's very hard to reproduce and the issue probably comes from the dll driver itself. The time we chose is a compromise between the frequency of crashes and the speed of the measurement so I think this should stay in the local cryofree version but it shouldn't go on the master branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you have the log, you could contact the manufacturer. Last time it took about an afternoon to fix the bug.
@@ -99,6 +104,7 @@ def open_connection(self): | |||
|
|||
""" | |||
cu = ADQControlUnit(self._dll) | |||
cu.enable_logging(3, b"C:\\Logs") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ugly ! We should pass a path for logging to the instrument as part of its setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least it should be behind an os check (the ADQ can run linux) and we should try to get a user related path that is more likely to be writable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just a quick hack I wrote to debug the issues I was having. It can be safely removed from this patch. (And added cleanly in the future)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean added cleanly ? is this needed or not ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed, I added this to debug an issue with the sp_adq14. What I meant by added cleanly was the fact that we could have a settings somewhere to toggle logging on and off where we could choose the logging directory instead of hard coding all of that.
Travis fails because we call directly a fixture function in a test (which we should not do), this should be an easy fix. |
#53 should fix the tests failures, so a simple rebase should allow Travis to go green. |
This is not a really clean solution for restarting a driver. I will try to have a look next week. |
These are probably old and I did not test the PSA driver.
However the acquisition card and CS4 work fine with this code.