LabVIEW

cancel
Showing results for 
Search instead for 
Did you mean: 

please rate my code and be brutal. need to know weakpoints.

Hi All,
    I've attached some code that I've been working on for quite a while.  Actually the code I've attached is an extremely simplified version of my stuff, but the important parts are there.    What I would really appreciate is some feedback.  I have scoured the forums, read at least 100 other threads on a variety of topics that are related to/implemented  in my code, but there are still some things that I feel could be done better/more efficiently/prettier/etc.

I've added some yellow textboxes throughout with useful little bits of info about what's happening in certain sections just as clarification.  The VIs are labelled with *_00.vi so that people can make changes and post them with the newer version (i.e. *_01.vi, etc.)

MainVI_00.vi - This part has 2 parallel while loops that are both stopped with a singe "STOP" button by using a control reference to a boolean indicator (called 'T boolean?').  I wanted two loops two run simultaneously so that I could have both subVI1_00 and subVI2_00 opened from MainVI_00 and running at the same time.  It seems to work pretty well, but it's a little ugly.  The only problem I'm having with this setup is that if subVI2_00 is enabled, and STOP is pressed in mainVI, I can't figure out how to close subVI2 and thus terminate the entire program.  I think the problem is that subVI2_00 uses an event Structure, but some advice would help here.

subVI1_00.vi - this would normally be the subVI for a scope, but now it just generates random numbers... if you look at it, it's really self-explanatory.

subVI2_00.vi - This is a little more complicated.  I use an event structure here to monitor several different possible user interactions.  If the radio buttons or some other values are changed, an event is triggered, and the machine is switched to a different field position.  I've taken out the instrument dependent code though, so now it just shows a string.  When the 'load params' button is pressed, an array is generated and passed through a control refnum back to mainVI.  Also, I had to include the stop button in the event structure because I couldn't figure out how to get the subVI2 to shut if I didn't.  As I write this, I suppose it's possible to put a time delay in the while loop which may catch the change in the stop button, but I'm definitely open to suggestions on how to do this part better.

Thanks for any advice that you may have on this.  Even the really brutal stuff is appreciated so that I can get this running in the most efficient and well-written manner as possible.

-Z
Download All
0 Kudos
Message 1 of 3
(2,484 Views)
Z,

I think some simple changes in architecture will help. Typically the event structure is in the main VI rather than in a subVI where it might cause exactly the problem you are experiencing. When an event occurs pass a message to the subVI via a queue. "Stop" would be one of the messages, of course. Depending upon when subVI 1 needs to run, it might be better off in another independent loop with a queue for control messages or perhaps it could run in the timeout case of the event structure.

Generally avoid local variables (e.g., error out in subVI 2). Direct wiring through a shift register should work.

Very minor point, but may make a difference when your program gets big and complicated. Try to use left to right wiring for the dataflow. It makes programs much easier to read (and therefore, to debug).

Good thing: Use the simplified code to work out your architectural problems before applying the solutions to the whole program.

Lynn
0 Kudos
Message 2 of 3
(2,477 Views)
Hi Lynn,
    Thanks for your comments... 


I think some simple changes in architecture will help. Typically the event structure is in the main VI rather than in a subVI where it might cause exactly the problem you are experiencing. When an event occurs pass a message to the subVI via a queue. "Stop" would be one of the messages, of course. Depending upon when subVI 1 needs to run, it might be better off in another independent loop with a queue for control messages or perhaps it could run in the timeout case of the event structure.

I would really appreciate some clarification on this one because I feel like I'm not understanding exactly what you're saying.  The way that I have designed the program is so that the two subVIs interact with the two instruments, respectively, that need to be "set up" for the experiment.  In order to do that, they both need to be able to be open at the same time (this is why there are 2 parallel 'while loops' in mainVI.  I could also put subVI1 in another while loop like subVI2, but I actually don't really like using references very much, b/c it breaks the dataflow model.... so instead, I just put one of the two in a separate loop.  Among other things (in the event structure) the subVI2 then generates a "sweep array" which is used in the final data collection in conjunction with the information that is returned from subVI1 (which is a cluster of scope controls).  
    From what you said, I am unclear how I could put an event structure in mainVI and control the individual events possible in subVI2?  or is this even what you're suggesting?  If possible could you make simple corrections to the code I provided illustrating your point?


Very minor point, but may make a difference when your program gets big and complicated. Try to use left to right wiring for the dataflow. It makes programs much easier to read (and therefore, to debug).

good point.  style is important because this code may need to be modified by someone else in my lab in the future, so I'll fix that.

thanks,
-z

0 Kudos
Message 3 of 3
(2,461 Views)