LabVIEW
check the VI
Padmavathi.B wrote:Check this Vi ...
OK, I "checked" your VI and concluded that it is not very good. It is inefficient, confusing, and most likely very buggy.
Your use of oversized diagrams and stacked sequence structures make the code very hard to read. Using proper dataflow, you probaby don't even need any sequence structures and globals. You seem to exclusively use globals as a substitute for variables in text based programs (See this discussion for details).
More specifically:
Frame 0:
- You read the 0th column twice using index array. It is sufficient to read it once, then branch the wire.
- You can use a resizeable index array to read the three elements on the right.
- It is not clear why you push them into global varaibles. Are they needed elsewhere? If you only use them in this program it would be sufficient to wire the index outputs to the subsequent inputs.
- For some reason, you re-read the same global variable over and over again multple times in subsquent frames. One simple wire could carry all that data.
- Look at coercions. Wire the correct representation to "decimal string to number" (probably U16).
Frame 1:
- Wire the script path directly to the subVI. your use of "build path" makes no sense.
- The datatype for "spreadsheet string to array" should probably be "U8 array", again avoiding all the coercions later.
- Again, "index array" is resizeable. You can get all elements with one instance.
- Again, is there a reason to push MSG_STRUCTin into a global? Why not wire directly to where it is needed in frame 3?
- The while loop can probably be replaced with much simpler code.
Frame 2:
- You prompt user for input, but don't use the user input anywhere. Why do it at all?
- You repeatedly read the same old global evey few inches of code. Use a wire instead.
- Using STOP is a bit ugly.
- You constantly overwrite the error with either an error message or OK. It is not clear if the operator will get a clear picture of the error condition.
- The single frame sequences inside the default cases serve no purpose.
Frame 3:
- Again, the single frame sequences inside the default cases serve no purpose.
- Again, way too many reads of the same globals.
- Hidden wires, right-to-left wires make the code hard to read.
Frame 4:
- Again, the single frame sequences inside the default cases serve no purpose.
- Again, way too many reads of the same globals.
Frame 5:
- Artificially keeping the VI running with a loop that spins millions of time/sec consuming all CPU is just silly. Especially since no reasonable interaction with the code (except for the stop button) is possible anyway. Simple UI polling loops need a reasonable wait statement.
So, what was your question.... 😉 Does the code run at all? Are the results correct?
Personally, I would recommend to tear down the program and rewrite it from scratch using a clean state machine architecture. A lot of your code is nearly duplicate and can probably be shared.
Message Edited by altenbach on 12-11-2006 11:48 AM
