add option for pretrigger memsize to channel readout #1461
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1461 +/- ##
=======================================
Coverage 73.86% 73.86%
=======================================
Files 92 92
Lines 10411 10411
=======================================
Hits 7690 7690
Misses 2721 2721 |
| @@ -611,7 +611,7 @@ def convert_to_voltage(self, data, input_range): | |||
| return data * input_range / resolution | |||
|
|
|||
| def initialize_channels(self, channels=None, mV_range=1000, input_path=0, | |||
| termination=0, coupling=0, compensation=None, memsize=2**12): | |||
| termination=0, coupling=0, compensation=None, memsize=2**12, pretrigger_memsize = None): | |||
There was a problem hiding this comment.
Why not make the default value of this argument 16 instead of None? This will remove the need for a couple of lines of code below, and simplify the code because pretrigger_memsize will become of type int instead of Optional[int].
There was a problem hiding this comment.
@astafan8 I agree. There might be future cases where we want to calculate the actual value, but since that is currently not there let's just use the smallest value possible.
| @@ -621,9 +621,13 @@ def initialize_channels(self, channels=None, mV_range=1000, input_path=0, | |||
| mV_range, input_path, termination, coupling, compensation: passed | |||
| to the set_channel_settings function | |||
| memsize (int): memory size to use for simple channel readout | |||
| pretrigger_memsize (None or int): pretrigger memory size to use. If None calculate a default value | |||
There was a problem hiding this comment.
The ".. calculate a default value" part is misleading because nothing is being calculated, the default value is just hardcoded. I recommend to remove this notion. It is much more useful to add the fact that "16" is apparently the smallest possible value for pretrigger memory - this will be very helpful for users.
Merge: bec01dc 4f2e6ab Author: Mikhail Astafev <[email protected]> Merge pull request #1461 from VandersypenQutech/feat/m4i_pretrigger
The option to acquire a single value from the M4i digitizer records an array of data and then takes the mean. Part of this data is from the actual moment the value is requested (the pre-trigger part of the buffer). This PR make sthe pre-trigger size configurable and changes the default to 16 (as small as possible).
@jenshnielsen @astafan8