car wash CLD exam solution?

[BADVI]

Hello,

  Like a lot of people I'm trying to prepare for the CLD exam.  I'd like feedback on my solution - particularly where the exam graders are likely to take points off. So first I'd like an overal "grade" for what I'm posting here (labview 9).

This took me about 6 hours to do - too long.  Part of that was deciding on the overall structure, and getting hung up on how to implement the "universal all stop" button.  

Are there too many wire bends?  For me it can be a very time consuming process to re-arrange everything to reduce the number of bends.

The description/tips and icon things also sucked up too much time.  

Some specific things that I'm not sure about are:

Is it ok to use property nodes for the "stop" button and "main power-wash" switches?  This solution works, but is it undesireable for some reason?  I had difficulty comming up with an alternative structure that allowed the stop button to activate in the middle of any part of the cycle.  I had worked on using an event structure to do this, but got frustrated and went with the property node.

Does anyone have any sense if using a queue to sequence the cycle is considered any better than using a numerically sequenced case structure?  If I only have 4 hours, using a queue structure seems to be a little more labor intensive... (propagating error, initializing and releasing queue...).   I can see the advantage of using a queue (flexibility), but functionally a regular sequence would also work.  To phrase this better:  are points awarded for using more flexible programming structures?

The timing sequences are rather primitive polling loops.  Does anyone think that I'd get docked for that lameness?  Is there a better way to do that?  I thought about using timing VI's and event structures, but the added complexity didn't seem worth it.  Well, ok, that and I couldn't implement it because there's still some things I really don't get with more sophisticated event structures.

[Mark_Yedinak]

Unfortunately I don't have LV 2009 installed yet so I can't look at your code. However I can answer several of your questions.

NI does not recommend using property nodes for reading or writing from controls and indicators. There are a few instances where it is OK such as initializing a control in the beginning of an application. Otherwise it is best to avoid using them to access the value of the control or indicator. You don't necessarily get extra credit for using an event structure but they certainly don't discourage it. Plenty of people have passed the exams without using one.

Using sequence frames would not be advised. LabVIEW is a data flow language and the use of sequence frames is effectively turning it into a sequential language. Using them would not be a good idea.

Without seeing your exact use of polling I can't say whether yours is OK or not. If you can avoid polling it is good. Event driven solutions are always cleaner. However there are times when polling does work and it can be used effectively. This one really depends on how you used it. If you do use polling though make sure you allow your application time to process other events or actions in your system. A free running while loop with no delay can bring the CPU to its knees.

[BADVI]

I just saved it for version 8.6, attached here.

Hmmm... sounds like I need to work on a solution that avoids all those propert nodes.

[Joe_H]

Just a couple of things i see rightoff.

1. Get rid of the property nodes. Only use property nodes to update the appearance of a control when neccesary.
2. The global stop that you implemented is HUGE NO NO!. Using the stop this way is basically them same as hitting the abort button.
3. The Stop functionality does not operate correctly. What it is doing is when you hit the stop button your queue is released and labview aborts. The other loop does not even run.
4. You need to correctly implement the producer consumer architecture. the way you have it now is not really an architecture it more of a HACK than anything. Take all of your code that is in the second while loop and take it out of the big loop.
5. Document your Subvi's. I cannot stress this enough. At least put something in the description of EVERY VI that you do.
6. Make a tip strip for ALL controls and indicators.
7. DOCUMENT EVERYTHING THAT YOU DO.
8. Even if you do not finish in the 4 hrs. make comments in your VI of what you intended to do and how it would be acheived.

I think that enough for now.

[Mark_Yedinak]

Here are a few more suggestions after reviewing your code.

1. Clean up your wires. There are unnecessary bends all over the place as well as wires that run back on themselves (Out of position property node).
2. Definitely get reid of all the property nodes. You really aren't using data flow with property nodes all over the place.
3. As mentioned earlier, everything including the subVIs eed to be documented.
4. Verify your functionality. This application doesn't meet the requirements. The out of position LED is not light properly for one. As mentioned your stop is not a graceful shutdown of the application.
5. Use a project for your application.
6. Consider placing your timing control in a subVI. You effectively have multiple copies of this functionality in your code.
7. Don't use the Wait VI for timing control.
8. As mentioned this really isn't a producer/consumer architecture. Take a look at the examples for how this normally looks.
9. All constants should be labeled.
10. Resize the front panel. Front panels should be size appropriately and shouldn't be a huge window with a few controls and indicators on it.
11. Resize the block diagram to fit on a single screen and avoid the need to scroll to see everything.
12. Make sure to align the controls and indicators are aligned on the front panel.

[BADVI]

OK. 

Back to the drawing board.

[Spectre_Dave]

If you have access to VI Analyzer (tools >> VI Analyzer >> Analyze VIs ..) use it.  This is an invaluable learning tool as this will help you with style issues.

[Broken_Arrow]

Just for fun, take a look at this Car Wash done recently in OOP, which I thought looked nice. You aren't doing OOP, so don't take too long on this one, but note how it seems to be less code than yours (even though OOP requires more encapsulation VI's).

As far as your VI's: Prop Nodes and just too many structure types in general.

[BADVI]

Hey...

  Here's my second attempt. I tried to neaten things up and eliminate the abuse of property nodes.  

I'll have to go back and try a more standard supplier/consumer architecture later on. 

I greatly appreciate your comments and feedback.

Apparently the VI analyzer has to be purchased separately for labview 9?

[cirrusio]

Hey BadVI,

Here are some things that I see;

1. car_wash_indicators.ctl needs to be included in the zip file
2. Document, document, document - a real weakness for most programmers in real life and of the utmost necessity
3. In the same vein - the comments are almost impossible to read due to the font selected -change this so it is more legible to the average human.
4.  There seems to be some unexpected behavior in the execution when one of the wash buttons is pressed and then depressed forcing an abort to be necessary.
5. As Mark stated previously, the whole set of VIs and controls ought to be collected in a project.
6. It appears that you need either a state machine or an event structure or some combination therein to maintain the appropriate dataflow
7. There is no reason to be obtaining and destroying the queue in each full cycle.  But, this goes along with the graceful shutdown that Mark spoke of and the need toimplement the producer/consumer architecture.

The last point should be a priority for your next revision.  Without it, it is going to be impossible to do a lot of the things that previous posters have suggested.  Anyway, hope this helps.

Cheers, Matt

Message Edited by mtat76 on 10-20-2009 12:18 PM