parameter in command set_trigger

[liuchaoatsz]

Hi,Pannuto I'm trying to use saleae to do something . I try the function set_trigger_one_channel and I find out that I am not able to use it to set the channel int Trigger.High/Trigger.Low mode.but Trigger.Posedge/Trigger.Negedge is ok. I check the API docs in saleae/SaleaeSocketApi/Doc/Logic Socket API Users Guide.md and I found that Python version don't implement the SET_TRIGGER command competely. Did I misunderstand this point ? Looking forward to your reply .

[ppannuto]

I believe that saleae/SaleaeSocketApi/Doc/Logic Socket API Users Guide.md describes only the C# API. Saleae does not maintain the Python API, just me in my spare time :).

The Python API does implement the SET_TRIGGER command. The Saleae protocol is a little awkward, so we expose a convenience method to set one channel, or set_triggers_for_all_channels to set them all, which matches the C# SetTrigger method.

The Python API currently supports NoTrigger, High, Low, Posedge, and Negedge trigger types.

[liuchaoatsz]

Hello ppannuto: Thanks for your reply. This project is very helpful. Thanks for your efforts. I mean we couldn't to specify a pulse gate like below:

4 channels, with a positive pulse on channel 0, no other channels used by the trigger. The pulse must be between 1ms and 2ms in duration. set_trigger, pospulse, 0.001, 0.002,,, Quote from saleae/SaleaeSocketApi/Doc/Logic Socket API Users Guide.md

[ppannuto]

Ah, yeah, that looks like a newer trigger type.

I'm happy to take a pull request to add support if you would like to try to implement it. Otherwise I'm unfortunately a bit busy but could probably try to look at this sometime in the next few weeks.

[liuchaoatsz]

Hello ppannuto, Thanks for your reply. I‘d like to try to implement it .

[Renegade85]

Hi ppannuto,

I could try to implement the new features as well. The only problem is, that the pospulse/negpulse requires two new options to be included - the min and max time value. So if you don't mind, I would extend the current set_triggers_for_all_channels() by a new parameter (tuple) pulse_width_min = 0 and pulse_width_max which would be set to a default value. This could guarantee backwards compatibility with older scripts. If you prefer a different input argument, just let me know your opinion. :-)

Have a nice day, ren

[ppannuto]

That's probably fine. I might be inclined to create a new type of trigger that encapsulate the settings instead, something like:

class TriggerPulseGate():
    def __init__(self, min_width, max_width):
        self.min_width = min_width
        sefl.max_width = max_width

# Then in the _set_triggers_for_all_channels, before trying to coerce to a Trigger, add
if isinstance(trigger, TriggerPulseGate):
   ...

Otherwise you have to update the signatures of at least three functions to handle trigger options. But I only thought about this for a second and could be convinced that's not a good idea.

[Renegade85]

Sounds reasonable to me. Although same things are implemented differently - current triggers are enums, while trigger pulses will be represented by a class, which feels not very clean.

I'm from C world and in Python just a beginner, that's why I proposed my solution that way. Also one part is I don't like the no-type in Python, as usually it's hard to guess, what an input can be. Nevertheless, this is not your case, as your code is well documented in this respect and one knows, which inputs are required.

Maybe it would make sense to implement the triggers as one "type".

I'll implement it based on your suggestion and we will see, it can be fixed in the future if required.

[ppannuto]

n.b.: partial implementation in #43

[ppannuto]

I believe this is addressed via #79.