code evaluation, please

[Pierre_F]

I'm a rather recent developer since approximately 1 year 1/2, and I realize I'm allready getting my own programmation habits.

I would like to know if this habits are good from your point of view

you will find joined my most recent code. Can you just take a look of my code design?

 don't hesitate to make criticism if necessary (with arguments of course :))

notes:

-made with labview 2015

-project name is "assembly 2.lvproj"

-main VI is "assembly 2.vi"

-programming time: 12 hours.

-validation not finished

For the curious people:

it's a small program that pilots a bar code reader and industrial screwer, for an operator.

1)the operator scans the barcode of the product (step"lecture douchette")

2)if the barcode is scanned + prerequisites and previous building steps are good for this product(step"verif N-1"), the screwer is unlocked (step"envoi visseuse")

3)at the end of the screwing, the screwer returns a frame, handled in step"lecture visseuse"

thanks four your opinions

[Gregory]

Hi Pierre, 

I am using LV 2014 so I cannot view your actual files, but if you are just concerned with style, I know there is a style guide check list in the LabVIEW help. Also, using VI analyzer you can get some comments on the style (like are the terminals aligned with the connector pane of a VI?). I've also seen the LabVIEW Style Guide by Peter Blume recommended on here, but have never read it myself.

As I said I could not open your project, so perhaps there is some good organization in there, but I would still like to see better organization outside of the project. Mike Porter talks about naming conventions on his blog: notatamelion.com which could be helpful for you. In the top folder (in Windows explorer) I'd like to only see folders and two files (plus the aliases and lvlps files that LV makes): assembly 2.lvproj and assembly 2.vi. Everything else should be in some well-named subfolders like Bar Code Scanner, Industrial Screwer, Integration, User Interface, etc. Those names are really up to you, I just think there should be more structure. If another developer ever has to take over your project, he will surely thank you 🙂 

[altenbach]

Just some simple issues after a 30 second look. 😉

1. Mind your fonts. Since you are using "application font 32" For the button boolean text, it will look different on different machines, depending on the desktop theme and ini settings. In my case, the text is slightly larger than the button.
2. I think the state should use a queue instead of a plain array where you append entries and peel the first one off with every iteration.
3. I would also split things into a UI loop and consumer loop.
4. In the project, I would place the exe build specification above the installer build specification, thats the order they need to be used.
5. I would match the names of the build and exe (currently one is ...1 and the other is ...2)

[Pierre_F]

• I think the state should use a queue instead of a plain array where you append entries and peel the first one off with every iteration.

I first began to use the array of enum for a queued state machine because I've seen it in the labview intermediate 2011 course book.

• I would also split things into a UI loop and consumer loop.

It's just my opinion but I don't understand why event handler prod/consumer is so popular, because there isn't any added value to use it compared to standard state machine + it takes more place in the diagram

• I would match the names of the build and exe (currently one is ...1 and the other is ...2)

In fact I deleted "assembly 1 program and created 2. that's why assembly 1 install is at the top of assembly 2 program 🙂

• Also, using VI analyzer you can get some comments on the style (like are the terminals aligned with the connector pane of a VI?).

I didn't know vi analyzer and I have just tried if. this is a pretty good tool

for example, the principal NOK results returned are:

-not enough docu
-too much nodes, structures and subdiagrams in the main diagram
-debugging enabled for all vis (what is that?)
-diagrams too big (don't agree)
-terminals inside diagrams

• Mike Porter talks about naming conventions on his blog: notatamelion.com which could be helpful for you.

I have taken a look at this website, it seems interresting. favorite added. 🙂

• In the top folder (in Windows explorer) I'd like to only see folders and two files (plus the aliases and lvlps files that LV makes): assembly 2.lvproj and assembly 2.vi.

in windows explorer point of view, my code is a mess 😛 I only use labview virtual folders

[Bob_Schor]

Some more comments:

• Congratulations on designing Icons for (most of) your VIs, and for putting (some) descriptions on them.  I would recommend putting the names of the Inputs and Outputs and (very brief) notes on what needs to be wired to them (or what their Default Value is, particularly for Enums).  Note that while I list Error In and Error Out on my descriptions, I don't bother saying what they are ...
• My preference for wires into and out of Structures (such as Case Statements) is to make a great effort to come in on the left and out on the right, and not to go in and out through the top.  It keeps the wires straighter and makes it easier to see inputs and outputs.  One "violation" of this practices is the Configuration wire in the Shift Register in the main VI.
• I didn't recognize that you were using the Queued State Machine at first, because you had made your own Queue out of an Array and Array functions.  One advantage of using LabVIEW Queue functions instead of "doing it yourself" is that other Developers (such as Altenbach and I) will recognize what you are doing and say "Oh, he's built a Queued State Machine, I know what that is ...".  It is Good Design if someone can glance at your code and understand the Broad Principles that you are using.
• Along the same lines of "Following LabVIEW Best Practices", the Producer/Consumer Event Pattern for handling the UI works extremely well with the Queued State Machine -- whenever the Recommencer button is pushed, it puts a "Get Configuration Values" State on the State Queue, which does the actions that you currently are doing in the "Recommencer Value Change" Event.  Note that another LabVIEW Practice is to put the Recommencer control in this Event state, as you only need to read (and reset) the Control when it is changed.
• In the new Get Configuration Values State, you would, of course, enqueue the Display and Use Screwdriver States, but wouldn't need to use Choix Menu, as it would be handled by the Producer code.  Similarly, the Menu Selection Event would fire a (new) Configuration State that would update the Configuration Shift Register and then enqueue Use Screwdriver.  
• You might not need an explicit Timeout Event on the Event Structure.  If you want to run your State Machine at a timed rate (instead of "as fast as it can"), you could put a Timed Wait in individual States or outside the main Case Statement in the Consumer loop.
• One thing to consider when using Multi-looped LabVIEW code is how to stop all of the loops.  You have a Stop State that will stop the Main loop, which is the Consumer Loop in a Producer/Consumer Model, but how to stop the Producer?  If you use Panel Close, that's an Event that can not only tell the Consumer to stop, but can be used to stop the Producer.  But you also stop the Consumer Loop on errors, so you need to signal the Producer Loop to stop, as well.  Two methods that I've used are to create a User Event (which is, I think, the method used in the LabVIEW Producer/Consumer Template) to stop the loop, or to have a TimeOut Event that runs, say, every 500 msec that checks if the Queue Status is OK.  This will generate an Error if the Queue has been Released (say, then the Consumer Loop exits), and this Error can be used to stop the Producer.  Another method I just thought up is to put an Error Out Value Signalling Property on the output Error Line and an Error Out Value Changed Event -- this (while it is perhaps a little silly) will also stop the Loop and should be easily recognized as performing this function.
• Speaking of Error Out, you do not bring the Error Line through the main While Loop, nor out of your Error Handler.  I'd bring it out of the Error Handler and use it instead of a No Error constant to keep the Error Line continuous throughout your code.  If you want to Clear Error, use the Clear Error function.
• Several of your sub-VIs (like Screwdriver I/O) have a silly Error Case Structure just before exiting the While loop.  If there is an Error, it is passed through, while if there is no error, you break the Error Line and wire in an Error Constant.  So in both cases, you Do Nothing!  Eliminate this loop.
• I am very confused by the sub-VI Read Initialization from Disk.  The Cluster on its Front Panel is of type Configuration (which you define), but the names on the Indicator don't match the names on the TypeDef.  Very Confusing.  I also find this VI, which seems to read an Initialization file and load it into a variety of Global Variables "messy and confusing".  I would recommend creating a second Cluster called, perhaps, VISA I/O Settings, having this VI output a Configuration and VISA I/O Settings cluster, getting rid of the Global Variables, and using the VISA cluster when you need VISA elements and the Configuration cluster when you need its elements.  Note that you might have to pass the VISA cluster into (and out of) some of your sub-VIs, but they seem to be mostly called from your Main VI (where the Cluster would be present on another Shift Register), so this should be no problem.
• You mention that your Folder Structure is "nice" in your Project, but "a mess" on your disk.  I try to keep my folder structures identical in both Project and on Disk (it really helps, as you need to use the Disk structure for doing such things as Backups and Version Control).  However, if you try to move VIs already in the Project into folders in Windows, you'll have to "rebuild" your Project (this isn't as difficult as it might seem, but can be a little frightening to see all those "missing VIs" in the Project listing).  However, it can be safely done from the Project.  To save some time, I'll go ahead and do this here, and will attach a Zip file with matching Project and Windows folders.
• I turned off Auto-Populating on the Data Folder (it can do "unexpected things) to your Project).  While re-arranging your files, I found one (Test Impression Rapport) that was missing, and several Controls (Indicateurs, Retour Config, Valeurs, Variables) that were not in the Project -- I added them to the Controles folder(s).  There was also the remnants of other Projects (Assembly Relai Keyence, Packaging ISMP) that I deleted.
• I should mention that the easiest way to add "missing VIs" into your Project in the correct folder (see previous point) is to move them to the right folder in Windows, go into Project and make that Project Folder an Auto-Populating folder (which brings them in), then turn off Auto-Populating.
• Are you using some form of Version Control to manage your LabVIEW Code?  Many of us use the Tortoise Subversion client (which means you need to find a Subversion Server to manage your Repository) -- no serious LabVIEW Developer should be without Version Control!

Bob Schor

[Bob_Schor]

I meant to say something about "Diagrams too big", with which you disagree.  When I began doing serious LabVIEW development, I read (several times!) The LabVIEW Style Book, by Peter Blume.  One of his Style Principles is that every VI should fit on a standard Monitor screen (almost all of mine do).  The advantage to this is that you see everything at a glance, and it forces you to hide irrelevant (at that level) code.

Two things help in keeping VIs small -- keep your wires as straight, going left-to-right, as possible, and "encapsulate" blocky code (which is often distracting and less relevant) into sub-VIs.  You already know about making Icons to help instantly understand something about what the sub-VI is doing -- it is a lot easier to grasp a small Block Diagram with nicely laid out code, connecting 32 x 32 pixel LabVIEW functions with 32 x 32 pixel User Sub-VIs, each with a nice Icon, than a huge VI (I've seen some that would require 50 monitors to view!) with wires running everywhere.

BS

[Bob_Schor]

Pardonnez moi -- when I saved the Zip File and attached it two posts ago, I forgot to close (and save) the Project file, so I'm not sure if the Zip file has the correct Project or latest version of all of the VIs in it.  Please discard it and use this one, instead.

Bob (Careless) Schor