LabVIEW

cancel
Showing results for 
Search instead for 
Did you mean: 

Please Critique My Code

Hi.  I am new to LabVIEW and was wondering if anyone could offer me advice on how to make my code more efficient.  I know I could have done things more easily, and I was wondering if I could have some advice from some of you experts. 

Let me first describe what I am trying to do:  I need to take temperature measurements from several thermocouples (eventually three) for an extended period of time (up to 40 hours) and I need to take them at a very slow rate (once every 5 or 10 minutes).  I also need to write these points to a text file.  The acquisition for the thermocouples must be made independent from one another; I need to be able to start one test and then perhaps 20-30 hours later, start the next. 

My current program is set up for taking two thermocouple measurements.  The server file that I uploaded creates the virtual channels with the settings that I want, and writes the temperature data to two global variables.  The client file has three frames in the sequence: the first to reset and local variables and apply settings, the next for acquisition, and the last to close the file. 

That was just a rough summery.  If you can, please take a look at my program and give me any insight that you might have.  Thank you.

0 Kudos
Message 1 of 7
(1,639 Views)

Disco,

 

Here are some various thoughts and observations on your code.  First, I assume that the server is always running, sampling the thermocouples, and writing the current values to the variables.  The client looks like it waits for a period of time, and then reads out the current values from the variables.

 

Some thoughts:

  1. It may be a complication to have a client and a server.  I think you could do all of this data taking and logging in one VI.  You may have other reasons for wanting it this way, but it seems to be unnecessary.
  2. You could convert the server.vi into a simple vi that you call from your client when you want to snapshot a measurement.  Get rid of the loop in the server and return the temperature measurements from the 1-shot.  This would also allow you to get rid of the global variables.
  3. You must have a 52" monitor because your client block diagram is huge on real estate!
  4. On the client, I see that the bottom loop does not have a timing vi in it (wait until, or wait) so it will probably peg the CPU.

-cb

  1. You have some initializations in the center sequence frame above the top loop.  These will be initialized in parallel with the startup of the two loops.  You may encounter some odd race condition or something due to this.
  2. I think you could easily implement this whole thing in a state machine design pattern.  Include an initialization, and shut down (or finalization) state for your init and shut down.
0 Kudos
Message 2 of 7
(1,622 Views)

10Degree,

 

Thanks for your reply.  There were a few things that you mentioned that I was unsure about.  First, in my bottom loop of my client VI, I don't think I should have problems with pegging the CPU because I have an event structure inside the while loop, that way it just waits for me incase I hit the stop button, and if I dont, it times out. 

 

Those initializations I have are just to diable my controls once the program starts.  They shoyldn't make any race conditions should they?

 

 

I don't think I could use a state machine, especially since I want to sample meaurements so infrequently, (every 5 min or so).  If I were to use a state machine, wouldn't I have to put it in a while loop and have a delay of 5 min?  That would make my program unresponsive.

0 Kudos
Message 3 of 7
(1,571 Views)

Disco,

 

A state machine would work fine for this application.  Rather than waiting 5 minutes in one state, the machine would just check the timer(s) to see if it was time to do something.  If not, it checks for user input and then checks the time again after a short wait (~100 ms).  Proper use of the event structure can make any program quite responsive.  Sequence structure are generally to be avoided.  They make modifying the program difficult and tend to make the program unresponsive.

 

Rather than using a reference and a property node, just create the property nodes directly form the control.  The property node then has the name of the control.  It works the same but saves some block diagram space.

 

Use of shift registers and a bit of preplanning will allow you to get rid of the local variables.  Wires are always preferable to locals, globals, and value property nodes.

 

The stop Value(Signaling) property node is not needed. The event structure will detect the change when the stop button is pressed.

 

Having multiple controls with the same names (Minutes, Seconds) can be confusing.  It is better to give them different names.  If you want them to have the same text on the front panel, use the captions.

 

The error status line into the top loop is a confusing arrangement.  Usually to skip a section of the program if an error occurred in an earlier part of the program, a case structure with the error cluster wired to the selector terminal is used.  If it is necessary for the loop to execute once after an error, then what you have will work.  A comment documenting the intent would be good in such a case.

 

Look at the Producer/ Consumer  design patterns.  What you have looks like these but works somewhat differently.  The Build table and Write to File in the timeout/Boolean Value Change event case would normally be in the other loop.

 

I would move the Initialization of the DAQ outside the loop and only Read inside the loop.  Queues could be used to move the data to the client rather than shared variables.  Shared variables are not available on all platforms.  Queues are more cross-platform compatible.

 

Lynn 

0 Kudos
Message 4 of 7
(1,548 Views)

Thanks for all your help.  I tried some of your suggestions.  For the state machine though, I am very interested to try and implement it, I just don't know if I have the time.  

 

I was finally able to get my code to work correctly with both of your advice, so there is no urgency right now for me to experient with other methods, even though I know a state machine would allow me to have a cleaner block diagram among other benefits.  All I care right now though is that the code works.   

 

Thanks again for all your help.

0 Kudos
Message 5 of 7
(1,538 Views)
I could argue very persuasively that you do not have time to not implement a state machine.  They are easier than you may think, especially since you do not have to write the framework yourself - examples are available in the LabVIEW help and several places on the net.  An example is in order.  Nine years ago, I was asked to help clean up an application.  The app did not use a state machine, and really did not need to at the time.  Two years later, I was asked to clean up the same application again.  It had expanded dramatically by three developers and was so brittle it could not be modified without breaking something else.  Needed features were not being implemented because it was too difficult.  I refactored the application to use a state machine in about a week (it contained about 200 VIs).  It has been used and expanded ever since with no real issues (it is up to about 600 VIs).  The entire problem could have been avoided with the initial use of a state machine.  It may seem like overkill now, but will save you enormous amounts of time later.  State machines are your friend.  Use them Smiley Happy.
This account is no longer active. Contact ShadesOfGray for current posts and information.
Message 6 of 7
(1,512 Views)
Thank you for your advice.  I am definetly going to look into your suggestion.  It is great to hear some advice from people who know the program better.  Though my VI does everything that I would like it to, I know that it is inefficient and that is the reason I came seeking advice.
0 Kudos
Message 7 of 7
(1,465 Views)