Advice on design and possible improvements

[tycho75]

Hi all,

I'm writing a VI to read a CCD sensor and control a LED in front of it.

I tried to follow some rules I found in the forums (use of queued state machines, etc.), but I'd like to have some advice on the design, on the ways to improve the diagram and efficiency of the VI.

Any comment will be greatly appreciated !

Thanks a lot,

T

[josborne]

Hmmm.... Rule #1 is to use a LabVIEW project file!  I don't see one in your application.  See here:

https://www.ni.com/docs/en-US/bundle/labview/page/managing-a-project-in-labview.html

Also, see here:

https://www.ni.com/docs/en-US/bundle/321393d/resource/321393d.pdf

https://www.ni.com/en/support/documentation/supplemental/08/best-practices-for-managing-ni-labview-a...

[johnsold]

Tycho75,

Some other things, more specific to your program.

1. You probably do not need more than 2 loops. One with the event structure and the other with a state machine to handle everything else.

2. All those controls and indicators hanging out at the left, not wired to anything, generally indicates someone trying to program LV like a text-based langauge.  Use the dataflow! Wires are almost always preferred to local variables.

3. Try to keep the size of the block diagram to one screen. SubVIs are your friend.

4. Additional documentation is a good idea.

5. You may want to consider more robust error handling than just a dialog at the end.

Lynn

[tycho75]

@josborne

Thanks for your comment. I created a project file, including several directories for subVIs, controls, ressources. Thanks also for the links. 

[tycho75]

Hi Lynn,

johnsold a écrit :

Tycho75,

Some other things, more specific to your program.

1. You probably do not need more than 2 loops. One with the event structure and the other with a state machine to handle everything else.

I designed this application as a set of processes that can run in parallel. Some have to run in parallel. For example, I need to generate a clock and a pulse to trigger the pixel readout (loop 4). At the same time, I have to read the sensor output (loop 1). While all this is running, I need to be able to turn on the LED. To my present knowledge, using only one state machine would force me to stop doing one thing to start another thing, and that's not what I want.

2. All those controls and indicators hanging out at the left, not wired to anything, generally indicates someone trying to program LV like a text-based langauge.  Use the dataflow! Wires are almost always preferred to local variables.

Well, I read somewhere that local variables are often better than long wires. In addition, using references to indicators seems to be the prefered way to update a GUI from sub-VIs. My idea for each of the different processes was that when I need to update something (e.g. data acquisition rate), the better way was to stop the task entirely (using a notifier in my case) and restart it with the new parameters. These new parameters are given by local variables that are read (only) when the process restarts. For me the use of local variables was logical here.

That said, I agree that this part with the cluster of references is not really nice, and probably I should come to functional globals, but I still have doubts of how to handle big GUIs with this mechanism. Probably FGs would also hide all the property nodes I need to enable/disable buttons, which would be a good thing !

3. Try to keep the size of the block diagram to one screen. SubVIs are your friend.

You're right ! It's a little big for the moment... but I have a wide screen 

4. Additional documentation is a good idea.

Yes, I will write some more.

5. You may want to consider more robust error handling than just a dialog at the end.

I added a error queue and I will improve error management. Ideally, I should be able to treat differently the errors coming from my different processes, but I still need to think about a way to do that.

Lynn

Thanks for all your comments !!

I attach a zip files with the project file and some updates.

[johnsold]

Tycho75,

1. I do not completely understand what some of your subVIs are doing, but I think that this could all be handled by one state machine.  It would require some changes to the subVIs.   For example the CCD_LEDTask.vi creates an AO task, writes data to the buffer, starts the task, then sits in a loop waiting for the task to end, then stops and clears the task.  Of course the wait loop will wait for the LED Notifier forever, so the subVI is frozen in that loop until the notification occurs.  Separating this into two subVIs (create, write and start) and (end?, stop, clear) would allow this to work in a state machine without significant interference to the other tasks. It appears that the other tasks have similar structures.  The stat machine might need to keep track of which tasks are running and allocate some time to each.  The CCD Read loop cannot execute faster than 20 Hz and the LED process appears to be slower. One state machine should be abel to handle this.

2. Local variables are never better than long wires.  Whoever wrote what you read was probably a text programmer who had not yet learned to use the power of dataflow.  Using references is apporpriate for updating the GUI from a subVI. Doing so will force the subVI to run in the UI thread which can have performance issues.  Your use of a queue to feed data to the GUI subVI may be OK. This subVI could be in one state of a state machine rather than in its own loop.  Search the Forum for information on updating the GUI from subVIs.  I think there are several good threads on the subject and maybe a Nugget or two.

Lynn

[tycho75]

Ok, now I think I understand what you mean.

I'll try to implement this and come back to this thread when it's done.

Thanks a lot !

[tycho75]

Hi,

I'm working to update the VI. I attach the latest version. Most of it is done, even if I still have some improvements to do here and there.

In particular, I'd like to have your opinion on how I use the functional global (FG) variable. I have some trouble with my 2 loops (state machine and event managment) because I can have two FG "set" about at the same time, resulting in one of the "set" being ignored. I see that eg when my acquisition is ON and while I try to change the LED settings.

Thanks for your help !

T

[tycho75]

Ok, here is the final version, I hope. I solved the multiple race conditions by moving more and more things in the FG. At the end, the state machine is only used to control data acquisition. the VI is still big but I don't see how I could reduce it more.

I'll still apreciate any comment !

Thanks for the help,

T

[johnsold]

Tycho75,

I only have a few minutes to look at your VI.  My first thought is to do the CCD FG stuff in the state machine, not in the event loop. For example you have 50 ms Waits in the some event cases between enqueuing the DAQ GET STATS command and a DAQ OFF action call to the FG. If both were in the state machine, no arbitrary delay is required. After the Get Stats state completes, the state machine then goes to a DAQ OFF state.

This eliminates the possiblilty of race conditions because the FG actions are in different states of the state machine but not in the event loop. It also eliminates delays in event cases which can make the event structure seem unresponsive.

Your comment in the DAQ GET STATS state case near the Flush Queue function seems strange. I would think that you would want to process all the GET PIC commands and get all the data before you start calculating the statistics.  If the DAQ State Machine is not "keeping up" with the commands being queued, then your data may not be valid and you should determine the reason the queue is filling up.

Lynn