06-03-2016 08:31 AM
@MarkGreally wrote:Thanks for your comments. Its always nice to get someone elses thoughts. I am the only person in my office who does labview and I must confess I have picked up a few bad habbits. Ah. I'd recommend trying to get somebody hired so that you have vacation coverage or making an arrangement to bring in an NI Alliance partner consultant on occasion. Just having an extra set of eyes will increase your
- I think your front panel is way too busy.
This is not the final UI, just to get things up and running. It will deffinately be altered later on to make more user friendly.
- Your block diagram doesn't fit on my monitor which is a no-no for me.
I have a fairly big monitor but I agree it is way to big.
- You are over-using property nodes which will cause you perfomance hits in the long run (Shift registers will save you here, Local Variables would be an incremental improvement)
Are you refering to the reinit to default properties?
No, I'm talking about things like where you're polling the stop button in the Amp Def State. Reading the value of Stop via Property node instead of a Local Variable is a significant performance hit since Property nodes cause a swap to the UI thread. Same with grabbing the MAC Address in the Save Data. Really, most data you're passing between states should exist in shift registers. Next best would be local variables and property nodes for value only when you have a specific technical reason for doing so.
- I would seriously consider looking into a pre-built state machine such as the JKI
I may look into this.
- Your number to string ramp prior to entering your loop could be replaced with a string constant (negligible improvement)
That keeps changing so a formula is quicker. Fair enough. Maybe make it into a subVI. It'll be easier to maintain that way.
- Your sequence structures are largely unnecessary. Error cluster wiring will control your flow.
Could you elaborate on this LabVIEW operates on dataflow and a node cannot execute until all of it's inputs have data on them. This is one of the great benefits of VIs with error in and error out terminals. Wiring the error out from a previous node to the error in of the subsequent node guarantees the execution order. For the cases where you have the delays in flat sequences, you can replace those Wait nodes with the Time Delay Express VI which has error in and error out terminals.
- Your Config File setup seems like something you're going to want to do in an array. Currently, you have off-screen indicators which you might consider replacing with shift registers.
Never thought of using arrays here but will look into it. Off screen indicators are for debugging. For debugging, use probes and highlight execution.
- Stacked Sequences hide code and I avoid them if at all possible.
These really only have repeated code behind them so I am not too concerned
- Clusters and Arrays would go a long ways towards making your front panel organized and scalable.
Deffinately something I will address
- You can safely delete your local variable write to property nodes of the same. (Please do this if even if you don't agree with any of the other pieces)
Do you mean write a variable to another variable. Specifically, I mean where you are assigning the value of the local variable text to the Value Property node for text. Same for text2, text3, and text4. Since you aren't using the signalling event for text, there is no benefit to writing the property node to the value it already has.
Im also curious as to how you are familiar with this device, its still in development. My wife works in Medical Device Development and I assumed it was related to the Lifetouch line of devices. I may have it confused with something else.
06-03-2016 08:51 AM
Thanks a lot for these comments, it gives me a lot to think about. What company does your wife work for, we are a small UK based company.