Suggestions about improving my code

[shjukheter]

Hello,

               It seems no broken arrow in my code but I know I have misused shift registers and tunnels on case structure. Before, I used local variables to save my previous values but I changed all them to shift registers. I am not sure whether I used them properly or not.

Please help me, if there is any misuse and I attached my vi down here. It having 8 sub vi's in it and currently I am worrying about only the attached VI. Any suggestions to improve my coding ability is much appreciated. I am going to convert this vi to sub VI.

[dyckynson]

Hi.

Please, give here any different version of your VI (suitable to read on older version of Labview). Maybe the best way would be any image.

But now I can tell you that using of local variable isn't quiet good way to read a programm. 

[Norbert_B]

While it is a good suggestion to "downsave" VIs to older versions in order to address more people to help you, the idea to post images instead of VIs is a bad idea.

In addition to this, you should try to get used to post all needed components, in your case the subvis.

Nevertheless, i think the issue you have are "defaulted tunnels". If you have a case which does not pass data to this tunnel, LV simply uses the default data value for this outputtunnel:

0 for numeric

false for bool

"" for string

Nothing/Null Reference for references.

So in your code, you are e.g. resetting your VISA reference to Nothing. That will create issues.

Another point which *might* be a problem is the un-initialized shift register on the top and the bottom.

hope this helps,

Norbert

[shjukheter]

Here is the versions code. Those who have later versions can download the up VI.

                         Thank you.

[ben64]

As Norbert said the big issue is the use of default values for tunnels.

It appear you used the clean up diagram feature, personnaly I never use it as the result are never what I expect for a clean diagram.

Some suggestions:

- reorder cases in state machine in a logical way representing a normal state flow

- try to minimize bends in wires

- label wires  (not all but you have long wires that we have to go through a couple of states to know what they represent)

- try to avoid case structure, you seem to use them mostly to add delay. You can wrap the wait function in a subVI with error in and error out clusters.     You can then use the error wire to control the dataflow.

- You used the Select function instead of a case structure in the "Read RAM" state, you should do the same thing in the other states.

Hope this help.

Ben64

[johnsold]

What do you have against straight wires and compact code?  With a little time spent cleaning up (not requiring any significant LV programming skill) your block diagram can be reduced to about half its original width and be much more readable.

I agree with the comments by Norbert and Ben64.  A comment describig the purpose and content of the U8 array -> string -> VISA write in the EEProm states would be helpful.  At a glance it looks like it could be an array with one element of value zero.

Lynn

[shjukheter]

I saw new problem that about storing a value. You know, I have 6 cases inside the while loop. In one of the case, I am getting a value U16 which has to be used after several iterations(means several cases switches). Note that, for each iteration the value coming from U16 is changes continuously but I need to store the value when it read first time. 

 For that, I have read the value and stored it in shift register and at the same time I have selected the option "use default if un wired" at tunnel. It seems that after several case switches, the shift register will lose that stored value because of selected option "use default if un wired". How can I store this value for reusable after some iterations in other case?

[johnsold]

Deselect "use default if unwired."  Except for the boolean which stops the loop, this is almost never the desired setting.  Then wire through EVERY case.  Change the value in the case where it is read.  The data will stay in the shift register until you change it or stop the program.

If you have an Initialization case, you can set it to a default value there.  Also you should initialize the shift register unless you have a good reason to not do so.  In a top level VI initialization outside the loop is the usual method unless you have a state machine with an Initialization case.

Lynn

[shjukheter]

---

@johnsold wrote:

Deselect "use default if unwired."  Except for the boolean which stops the loop, this is almost never the desired setting.  Then wire through EVERY case.  Change the value in the case where it is read.  The data will stay in the shift register until you change it or stop the program.

---

Really, I am stupid and i have been thinking about this for days. What a simple solution.

[johnsold]

Read the detailed help for a case structure.  It is in there.  I know. No one reads the help on simple things like case structures, including me.  But checking the detailed help is often the first and best place to look when something does not seem to be working the way you expect.

I always keep the Context Help window open while I am programming and frequently check the detailed help.

Lynn