pretrig argument needs better documentation to specify that it is a fraction

[deeearth1]

In picobase.py the runBlock function is passed a pretrig value.
In all the examples ive seen this value picks up the default 0 value, if you change this so that you want to use a pretrig value you end up getting an error.
"

Errored: Error calling _lowLevelRunBlock: PICO_TOO_MANY_SAMPLES (Number of samples requested is more than available in the current memory segment.)

"
the line:
nSamples_pretrig = int(round(nSamples * pretrig))
should be changed to:
nSamples_pretrig = int(pretrig)

[hmaarrfk]

It probably depends what your definition is. I don’t like thinking about things in terms of samples as often the sample rate is dependent on many other factors. Typically you want a fraction of your observation time to be before the trigger. The parameter is a fraction of the observation time and not the number of samples. Does that work?

…

On Mon, Jul 16, 2018 at 4:11 AM deeearth1 ***@***.***> wrote: In picobase.py the runBlock function is passed a pretrig value. In all the examples ive seen this value picks up the default 0 value, if you change this so that you want to use a pretrig value you end up getting an error. " Errored: Error calling _lowLevelRunBlock: PICO_TOO_MANY_SAMPLES (Number of samples requested is more than available in the current memory segment.) " the line: nSamples_pretrig = int(round(nSamples * pretrig)) should be changed to: nSamples_pretrig = int(pretrig) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#134>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFfmOkW48S-vVrBPgVtYmPOHQFIDk8Fks5uHFiwgaJpZM4VQyjA> .

[deeearth1]

With pretrig set to 50 and a sample size of 8192, i can see that the trigger is at data point 50, not 4096 this implies that the pretrig value is based on the sample size not as a percentage of the total samples.

[hmaarrfk]

Maybe that is the maximum number of prertriger samples possible? Can you try it with a different number. maybe `.12`, or even `0.01`?

…

On Tue, Jul 17, 2018 at 1:26 AM deeearth1 ***@***.***> wrote: With pretrig set to 50 and a sample size of 8192, i can see that the trigger is at data point 50, not 4096 this implies that the pretrig value is based on the sample size not as a percentage of the total samples. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#134 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFfmMV2gNHPPVWXQA_QoQ0Qmgib9Ys-ks5uHYOpgaJpZM4VQyjA> .

[deeearth1]

you are correct i can enter a value between 0.0 and 1.0 the trigger point moves as a percentage of the total samples requested.
any value over 1.0 causes the PICO_TOO_MANY_SAMPLES error.
should the pretrig value then be tested that its between 0 and 1 before calling _lowLevelRunBlock to get rid of the error?

[hmaarrfk]

When we were designing the python wrapper, we explicitly chose not to have error checking in Python. Their SDK does ALOT of error checking already and it would be alot of work to keep up with their changes. I would suggest that you bring up this issue with PicoTech and have them release a new error code that is more appropriate. Something like: "Pretrigger sample more than total sample number". Once they include this, you can then regenerate the error list following the instructions that https://github.com/colinoflynn/pico-python/blob/master/picoscope/error_codes.py details

For example, say you want 100 samples, but you want to look at the -200 to -100 sample from the start of the trigger. Maybe this will be possible in a future version of the SDK and if we add our own checking, it might not be possible until we explicitly update our wrappers due to error checking.

If you want to add additional information, I would add it to the docststring.

Thanks for helping us get to the bottom of the issue.
I think:

1. pretrig should definitely have better documentation and state that is a fraction and the value should be between 0.0 and 1.0
2. Submit a request to Picotech for a more informative error code on their forum.
3. Include a note of the forum post in the docstring.

Finally, maybe we should consider the following:

1. Deprecate the pretrig keyword argument in favor of: pretrig_fraction and pretrig_samples.
2. Add some logic allowing specification of the nSamples_pretrig using either pretrig_fraction and pretrig_samples.

The function signature could be changed to

def runBlock(self, pretrig_fraction=None, segmentIndex=0, 
             pretrig_samples=None, pretrig=None):
    if pretrig is not None:
        warn('The ``pretrig`` keyword argument has been deprecated and will be'
             ' removed in January 2020. Please use the arguments '
             '``pretrig_fraction`` or ``pretrig_samples``.')
        pretrig_fraction = pretrig

    # More logic to handle pretrig_samples vs pretrig_fraction