Re: [sigrok-devel] A few comments from the middle of the night

[Soeren A. <so...@ap...>]

Hello Brian,

I agree with you about the discussion, it's always nice to realize that
we essentially have the same goals :)

I haven't yet looked at the ticklegends commit in depth, only saw that
it's relatively small. Will do as you suggested.


> I think we agree on the undesirability of having the capture_state
> state machine being embedded in MainWindow, and I think it should be
> in its own class.

Certainly agree, yes.


> The question is where that class should go.  There is no existing
> class that clearly should adopt it.
> Should it be a child of MainWindow?
> Should it be a child of Session?  Or parallel with it?
> If the future of PV is to provide simultaneous capture on different
> devices in different sessions, then it should be near Session.
> If not, then probably nearer to MainWindow.
> If we can identify a proper place to put it, to better support the
> future, I would be happy to take on revising it into its own class,
> including the ability to do the single/repetitive thing, and perhaps
> the auto thing.

At this point, I don't think we're going to see acquisition from
multiple devices in PV. The main reason for this is that my naive
assumptions are flawed. If you look at the sigrok-devel mailing list,
you'll find a thread started by Jason Pepas on 2017-07-21.
His goal was to use two 8-channel FX2 LAs to capture 16 channels.
My naive assumption was that starting acquisition on an FX2 would be
fast enough to allow this without any synchronization.

Jason's experiments have shown that this is untrue and that the
time difference is actually significant and some kind of calibration
would be needed to support this. On top of that, merging the two
8ch sessions into one 16ch session is doable with console scripts
(and certainly within PV) but creating a UI that makes sense to the
user doesn't seem straightforward.
With that, I say we save this feature for when we have the flowgraph
API in place because this use case will be much simpler to support
then.

Long story short: I'd say that we keep PV the way it currently is:
one session = one device. This means that the class you propose would
be closer to Session than MainWindow.


> The auto thing would require a change in the protocol with
> libsigrok.  Currently, the protocol seems to pretty much assumes that
> capture is a two phase
> state machine that begins with watching for a trigger event, then
> beginning doing the actual capture, and running the capture to
> completion.
> It is essentially a two state sequential state machine, with a single
> start/stop interface.
> Supporting Auto would require being able to tell libsigrok to go
> ahead and make that transition into capture even though the trigger
> did not happen.

Assuming a user has a trigger configured, currently PV indeed has no
way of telling libsigrok that the trigger should be ignored. This is
unfortunately a bigger issue than it seems at first glance because in
the case of a remote-controlled scope or full-featured LA, the trigger
is configured and enabled in the acquisition hardware. The device
drivers are stateless here and just relay the trigger configuration
settings to the hardware.
To make this work, the device drivers need to have a mechanism that
saves the current trigger configuration, runs the device in Auto
mode and then restores the trigger configuration to what it was before.
Currently, no such mechanism exists.

Also, what should device drivers do when the hardware device doesn't
support Auto and a trigger *must* be configured? Do we disable Auto
mode in the PV UI then?
If that's what we want to do, we need to also have a mechanism that
allows PV to query whether the driver supports Auto at all.


> I suspect this would best entail changing some assumptions currently
> embedded in the (mostly implied) trigger/capture state machine,
> and remaking it into a more explicit state machine, with a better
> external interface with the capture_state state machine.
> However, I would not make it mandatory to do that at this time, just
> to make it possible to move the capture_state state machine into its
> own class.
> Auto support could be planned for now, but left to a future revision.

As you have noticed, the trigger mechanism in libsigrok needs fleshing
out, yes. From what I know, we also don't have any driver that supports
setting a trigger more complex than the simple rising or falling edge.
With hardware scopes this isn't an issue since you can just configure
the triggers on the device itself, which is why the pain of not being
able to properly configure triggers hasn't been big enough for anyone
to work on this yet.
A PV UI for this would also be non-trivial but we need this in the long
run anyway, so if you intend to work on this at some point in the
future, we should discuss this when the time is right.


> I am not entirely happy with the current location of the run/stop
> button in the menu bar.
> I am inclined to think it should be in the MainBar.
> This is particularly true if my speculation above on having more
> independent sessions were true.

I would've been opposed to this before but no longer am.


> Considering what you said last night, I kind of think if it were in
> the MainBar, then it would be in the form
> of a run/stop button with an adjacent button to activate a dropdown
> dialog for the trigger mode parameters.
> This dropdown is where one would configure single/normal, trigger
> holdoff delay for normal mode, and auto,
> with perhaps a separate auto time interval.

I think we should have three separate buttons:
- Run with a drop-down to select "Auto" and "Normal" modes
- Run single (only shown if the hardware supports it, e.g. hidden
for FX2 devices)
- Stop

The rationale for this is based on two issues:
- Users want separate Run/Stop buttons because when using the hotkey
for Run/Stop (Space), it is impossible to tell whether acquisition
is running without looking at the screen. Breaking this up into
separate buttons/hotkeys would remedy this.
- The acquisition options (e.g. sample count, frame/segment count,
sample rate, trigger hold-off delay, ...) are so plentiful that
they would clutter the toolbar and since they're only used for
acquisition and not during data examination, it makes sense to provide
them in a single dialog, accessible from the toolbar at the click of a
button. Pretty much in the same way the decoders are currently
configured.


> My repetitive branch more or less does this except I have done
> nothing for auto, and it is still in the MenuBar.
> Also, it is not properly integrated into the capture_state state
> machine, although it works fine.
> I would like to fix all this.

I'm happy you want to tackle this. It's much needed.


> 
> Lets continue the conversation.
> Is there anyone else that should be in on it?

I'm moving this discussion to the mailing list so that it's public
and so that anyone can chime in with considerations we may have
overlooked.

-Soeren