LabVIEW

cancel
Showing results for 
Search instead for 
Did you mean: 

Cleaning up Local Variables

Nightmare does not even begin to describe that code. Yes, waits are needed in all toplevel loops as mentioned.

(1) As I said before, ALL the small while loops can be eliminated by a simple code alternative that is much more efficient. All these "blind" property reads force a switch to the UI thread and are thus very inefficient.

(2) Duplicate code needs to be cosolidated into FOR loops! Let's have a look at the "Light Tower control". You do 12 different FP writes in random order and for that your read the same old locals (e.g. red, green, yellow) four times in parallel. What makes you think you'll get better information than just reading each once and wire to all the sinks??All you need in that "Light Tower control" loop is a single FOR loop, autoindexing on an array of FP I/Os and an aray of "loss of comms..". You need exactly one diagram constant for "1" and one for "0", and not 12 each! That entire loop can probably shrink to the size of an index card. 😉

(3) Overuse of local variables and unecessary operations. (image below). There is no reason in the world to read from a local if the terminal is right there. You also might want to think hard about the purpose of the "equals true" operation on a boolean. Maybe you can make a "truth table" to see if it can be simplified. 😄 (see also here for a similar construct, that entire thread might be a good read). More comments on the image.

 

Message Edited by altenbach on 07-13-2007 09:56 AM

0 Kudos
Message 21 of 25
(862 Views)
You do not even need the 0 and 1. Use the Boolean to 1,0 primitive.

I suspect that using the Producer/Consumer (events) design pattern and some careful thought wouldget you to two parallel while loops.

Lynn
0 Kudos
Message 22 of 25
(848 Views)


@johnsold wrote:
You do not even need the 0 and 1. Use the Boolean to 1,0 primitive.

Thanks Lynn! The boolean to 0,1 function results in I16, which may, or may not, be appropriate. Of course we could simply append a conversion bullet and still be cleaner ;).

In this particular case, the input (currently I32)  to the FP write gets coerced once more at the FP call, so I have no idea what the right final representation should be.

(I am sure the list of suggestions will grow without bounds once I look into the code in a bit more detail, but currently I don't really have time for all this.)

0 Kudos
Message 23 of 25
(842 Views)
Thanks for the responses altenbach!

I'm going down the list of all the suggestions everyone has made starting with the fieldpoint calls.  I've condensed that whole FP loop down to two auto indexing arrays, and a for loop for sending the signal to the FP hardware.

I'm going to handle all of the values of those lights in another location but I assume what I have attached is what you were suggesting to be done?

Also when each of those arrays reach their end...do they just start over again?  Basically they need to be cycling through those as long as the vi is running...something strange I noticed though I ran the program with that modification in the picture, (and once with a while loop around the for loop) it randomly fills the array...once full it always seems to start by sending "#12" out of both build array outputs.  Thats all it sends....I thought with auto-indexing it would send location 0 from both array, then location 1, 2, 3 and so on....

I'm trying to make sure those arrays are only ever populated once, not point in rebuilding them every time a value needs to be checked/updated.
LV7.1, LV8.5, LV2014/15/16
0 Kudos
Message 24 of 25
(801 Views)


@leachdor wrote:
Also when each of those arrays reach their end...do they just start over again?  Basically they need to be cycling through those as long as the vi is running...something strange I noticed though I ran the program with that modification in the picture, (and once with a while loop around the for loop) it randomly fills the array...once full it always seems to start by sending "#12" out of both build array outputs.  Thats all it sends....I thought with auto-indexing it would send location 0 from both array, then location 1, 2, 3 and so on....

First you should fix your diagram background. The only useable background colors are white (preferred) and very light pastel shades of colors (if needed for distinction of code segments). Any darker color will camouflage at least one of the wire colors and will make the diagram very hard to read. In your case all wires are camouflaged. Why make programming harder than it already is? You might as well tape your fingers together, and stand on your head while programming. Sure it can be done, but why make things more difficult? 😮

When you see #12 during execution highlighting, it just means that there is an array with 12 elements. It makes no sense to show more than that. Inside the loop you will see the indexed individual elements, which are hopefully different for each iteration. You can always create a custom array probe that shows exaclty what you want to see for debugging.

0 Kudos
Message 25 of 25
(787 Views)