01-29-2009 01:48 AM
okay, i finally got a chance to do it at home.
its too bad i can't work on it while i am at work though..
but maybe someone can give me some advices on approaching this situation.
thank you guys for helping me out.
01-29-2009 01:01 PM
Hi krispiekream,
It's difficult for me to tell exactly what your code is doing, but it appears to be running some pressure/temperature profiles with some type of oven chamber.
My thoughts:
1) Your interlacing EVERYTHING in a complex code. When I do something like this, I try and separate the problem into components. For example, code to set the pressure would be in a separate loop from everything else. I would have a limited set of variables to interface with the main part of the code, say "Pressure Target", "Current Pressure" and "Pressure Reached", the later being a Boolean that indicates when the desired pressure is reached. Similarly for code to set Temperature.
Somewhere else would be a loop that controls your automation, which might have a sequence in it that sets "Pressure Target"and "Temperature Target", then waits for "Pressure Reached" and "Temperature Reached" to both be true before proceeding to the next step. This automation sequence would be far easier to read than your current super-complicated automation sequence with all it's technical details.
2) Your also tying all sorts of controls together in one huge cluster. There is seldom any reason to do this. Also, your using Global Variables as well. Very confusing. Personally I would advise never using Global Variables for any reason.
3) Your Front Panel also crams in every control and indicator. Consider tab control tabs for all the small details that the operator only needs to look at occasionally. The Main panel should only have major controls and indicators, like "Current Pressure" or "Abort".
Basically, divide the big problem into several smaller parts, with simpler connections between the parts, rather than tying everything together in a massive superloop.
-- James
01-29-2009 01:11 PM
01-29-2009 01:48 PM
With the current awful structure of the code, I'd suggest sticking with the local variables. Poor coding, perhaps, but switching to wires or a huge cluster will just make an even more unintelligable mess. And there is mothing wrong with a FEW local variables in a program, just not unnecessary ones.
BTW, "bundle" and "unbundle" work on the wire directly; they don't change any control or indicators at all, until you connect the wire to something.
-- James
01-29-2009 02:35 PM
01-29-2009 03:16 PM
Having faced the code, it's much easier to give some advices. The overall size makes it really difficult to suggest an architecture for the program, that would make more sense in an earlier stage where you have little more than specs and components. I assume that the code is working, so you wont start from the scratch. It looks horrible at first glance, with a huge BD, a messy FP and about 160 SubVis. BTW: you didn't post these along with the code, which means 160 clicks of 'Ignore SubVI', which propably shortens the audience. Maybe a picture (not inlined in the text!) would be better (as we have no possibility to test any code).
So let`s share and brainstrom some ideas to clean up the code. Generally, it would be nice to bring the number down from 160 SubVis to 16 SubVis (which should containe some of the former Second-Levl Sub-Vis), Building up a nice hirarchy tree. Propably you could group some SubVis together that operate on the same data/devices and put them all into a AE (Action Engine). I guess that this will lead to failure most of the times now, but keep some notes (pencil and paper are greate tools for programmers!), they might help you in the future. And most important, you will start developing some design structure of your ow, whoch apperently that code lacks of.
Also
pay attention to the nested 'aztec pyramide style' segments (case
inside sequence inside ...), get rid of these as soon as possible.
Pencile and paper are your friends again to show the logic. And then
you easily might refactore the code into a state machine.
Appart from that 'big picture'-approach above, there are several things to be done on simple coding style. Do it whenever the design stuff gets over your head and frustration starts to arise:
* If a terminal is unconnected and there is only one local variable, replace the local variable with the terminal (as you mentioned, you want to get rid of taht overuse).
* Hunt down the globals as well, if there are few references, a wire would be the much better choice.
* Restructure the Front Panel. Several Controls/Indicators are related. Put them together in a cluster (Most likely they are also used in the BD together). >For example the TemStart, TempEnd ... could be grouped into a cluster nicely. Make it a strict type def in case you need to add/change something and use the typedef in the SubVi calls as well. Go further and donate them a seperate window 'Temperature Settings' or along with the pressure settings as 'Measurement Settings' or 'Test Settings'. Then eigther use a dialog to enter these or a tab page with Sub Vi Panel.
* Here again would be a time for redesign. Identify which parameters will be changed by frequncy and by knownledge/priviledge. Here it depends a bit of your users, I have to startegies:
-- Scientists/Research (the user is academic and requests to be able to define evereything): Measurement Settings Dialog -> Performe Test
-- Industrial Design: Different access levels (Operator, Admin, Service); Service handles all device specivic settings, like bus type, com port ...., Admin creates 'recipes' (predefined sets of measurment settings) and operator only fills out DUT number and selects recipe -> perform test.
Any of those will break down the code in several SubVis.
Ok, guess thats input for a week of work.
And don't forget:
* Document changes
* Keep backup copies
* Test after major changes if the code is still working
Felix
01-29-2009 03:54 PM
01-29-2009 06:40 PM
F. Schubert wrote:
BTW: you didn't post these along with the code, which means 160 clicks of 'Ignore SubVI', which propably shortens the audience.
Speaking of this, I find it extremely annoying to have to click ignore more than a few times. Why can't LabVIEW have an "Ignore All" button in that dialog box as well? Probably something I should send to the product suggestion center.
02-04-2009 10:21 AM - edited 02-04-2009 10:21 AM
ok, i am back..
i have cleaned up what i could. there is no errors on my end, but i dont know if it is working problably yet.
i think it is..
but right now..i dont like the way the local varialbes are still there..
what can i do?
i can leave it as is and just add local variables to all the BUNDLED by name to indicate value changes on the screen?
or should i move it toward a better structure?
i dont like how its such a big block diagram.
someone told me that it should only be a screen size ("ideal") of my monitor for all the codes..
let me know if u guys can give me any suggestions.
thank you..
02-04-2009 11:49 AM
Hi krispiekream,
You seem to think that replacing all the local variables with a giant everything-but-the-kitchen-sink cluster is an improvement. It isn't. You need to work on organizing the code cleanly. Personally, if I had to deal with this code I would start be writing a independent VIs for controlling Pressure, Temperature, and reading out whatever a "2503" is. Then I would build up from there using these as subVI's. If you did that well you could get to a main vi diagram fitting on one screen, but I wouldn't make that a goal.
-- James