LabVIEW
Is this good or bad programming?
Hello therealkilkenny,
some comments:
1) In the inner case statements you always have a "Set Channel" and a "Clear Channel" in the true/false cases with the same channel number wired to it. Why not make a subvi that has two inputs: channel number and a boolean flag (Set/Clear). Then your main vi only uses one subvi and in this subvi you make the difference between "Set Channel" and "Clear Channel". Btw. most times the difference between a Set and a Clear operation is only the parameter of a given command. Why not join those two vi to one and send only the right command to your DAQ device? (Example: A device needs the command F0 to clear a channel and F1 to set a channel. Then I make a vi with a boolean input, wire this input to "format into string" with format strin "F%b" and send the resulting string. It's easy to program and to debug.)
2) You compare controls with some hidden controls. It's much better to use some shift registers for this purpose. Then it is clear that you compare the actual value with the value from the last loop iteration. And it's easier to "read"...
3) If you change the comparision to "non equal" (instead of "equal") you get an array of bits, where (normally) only one bit is set (except the user can change switches faster than you check them). Instead of searching for a bit in that array, like you do at the moment, you convert the array to a number ("boolean array to number", numbers/conversions palette). Now you can use cases with appropriate numbers, which are easy to calculate. Example: bit 0 is set -> case number 1, bit 2 is set -> case number 2 and so on (powers of two...)
4) You included a delay of 1 ms. For a normal user interface a value of 100-200 ms is ok. How often can you click in a second? You can also use "Wait for Front Panel Activity" (time & dialog palette), perhaps with an appropriate timeout value.
5) Have a look at "LabView Development Guidelines", it should have come with your Labview installation. (Chapter 6, "wiring")
Hope this helps,
GerdW
some comments:
1) In the inner case statements you always have a "Set Channel" and a "Clear Channel" in the true/false cases with the same channel number wired to it. Why not make a subvi that has two inputs: channel number and a boolean flag (Set/Clear). Then your main vi only uses one subvi and in this subvi you make the difference between "Set Channel" and "Clear Channel". Btw. most times the difference between a Set and a Clear operation is only the parameter of a given command. Why not join those two vi to one and send only the right command to your DAQ device? (Example: A device needs the command F0 to clear a channel and F1 to set a channel. Then I make a vi with a boolean input, wire this input to "format into string" with format strin "F%b" and send the resulting string. It's easy to program and to debug.)
2) You compare controls with some hidden controls. It's much better to use some shift registers for this purpose. Then it is clear that you compare the actual value with the value from the last loop iteration. And it's easier to "read"...
3) If you change the comparision to "non equal" (instead of "equal") you get an array of bits, where (normally) only one bit is set (except the user can change switches faster than you check them). Instead of searching for a bit in that array, like you do at the moment, you convert the array to a number ("boolean array to number", numbers/conversions palette). Now you can use cases with appropriate numbers, which are easy to calculate. Example: bit 0 is set -> case number 1, bit 2 is set -> case number 2 and so on (powers of two...)
4) You included a delay of 1 ms. For a normal user interface a value of 100-200 ms is ok. How often can you click in a second? You can also use "Wait for Front Panel Activity" (time & dialog palette), perhaps with an appropriate timeout value.
5) Have a look at "LabView Development Guidelines", it should have come with your Labview installation. (Chapter 6, "wiring")
Hope this helps,
GerdW
In addition to Gerds comments above, I would suggest a few more improvements.
Motto: Simplify!, Simplify!, Simplify!
Instead of having 8 individual channel controls, use an array control with 8 elements.
You have 8 cases with basically the same code except for the channel number. One case structure is enough!
Get the changed channel, get its value (true or false), and write the channel number (index+1) to your subVI as needed
Use an event structure with 3 events [load device | channels | stop], each with "value changed"
Place the cleanup operations in the "stop" case
Maybe change "Load device 0" to latch action and elminate anotehr case structure?
get rid of that bewildering array of local variables and hidden indicators
I gave your VI a very quick makeover to give you some ideas. Modify as needed.
Programmatically, it should behave similar to yours, but please check for bugs (I don't have your driver VIs, so I cannot test). Enjoy!
The total size on disk miraculously shrunk to 25%! 🙂
Motto: Simplify!, Simplify!, Simplify!
I gave your VI a very quick makeover to give you some ideas. Modify as needed.
Programmatically, it should behave similar to yours, but please check for bugs (I don't have your driver VIs, so I cannot test). Enjoy!
The total size on disk miraculously shrunk to 25%! 🙂
Sorry, it seems my array control had accidentally grown to 10 items :(. Here's a better version 🙂
Hi,
for programming tips and style guides etc, try the labview power page here :
http://www.ni.com/labview/power
For large projects, I'd also recommend looking at Steve Watt's LabVIEW book
http://www.amazon.co.uk/exec/obidos/ASIN/0130093653/qid%3D1109152420/026-1704751-2456435
Thanks
Sacha Emery
National Instruments (UK)
for programming tips and style guides etc, try the labview power page here :
http://www.ni.com/labview/power
For large projects, I'd also recommend looking at Steve Watt's LabVIEW book
http://www.amazon.co.uk/exec/obidos/ASIN/0130093653/qid%3D1109152420/026-1704751-2456435
Thanks
Sacha Emery
National Instruments (UK)
