These were left on the cryofree computer; there is:#52
These were left on the cryofree computer; there is:#52
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
| 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.
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.
| retrieved_records = 0 | ||
| while retrieved_records < records_per_capture: | ||
| # WHY ??? | ||
| time.sleep(0.002) |
There was a problem hiding this comment.
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.
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.
Since you have the log, you could contact the manufacturer. Last time it took about an afternoon to fix the bug.
|
|
||
| """ | ||
| cu = ADQControlUnit(self._dll) | ||
| cu.enable_logging(3, b"C:\\Logs") |
There was a problem hiding this comment.
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.
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.
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.
what do you mean added cleanly ? is this needed or not ?
There was a problem hiding this comment.
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. |
4 participants
These are probably old and I did not test the PSA driver.
However the acquisition card and CS4 work fine with this code.