pico-python icon indicating copy to clipboard operation
pico-python copied to clipboard

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

Open deeearth1 opened this issue 6 years ago • 5 comments

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)

deeearth1 avatar Jul 16 '18 09:07 deeearth1

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 [email protected] 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 https://github.com/colinoflynn/pico-python/issues/134, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFfmOkW48S-vVrBPgVtYmPOHQFIDk8Fks5uHFiwgaJpZM4VQyjA .

hmaarrfk avatar Jul 16 '18 16:07 hmaarrfk

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.

deeearth1 avatar Jul 17 '18 06:07 deeearth1

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 [email protected] 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 https://github.com/colinoflynn/pico-python/issues/134#issuecomment-405473791, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFfmMV2gNHPPVWXQA_QoQ0Qmgib9Ys-ks5uHYOpgaJpZM4VQyjA .

hmaarrfk avatar Jul 17 '18 14:07 hmaarrfk

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?

deeearth1 avatar Jul 18 '18 06:07 deeearth1

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

hmaarrfk avatar Jul 18 '18 15:07 hmaarrfk