FC and Temp/Output area

userxyz · Mar 7, 2019

Hi,

Today I checked the code of a collegue and I didn't agree with his code. So he wanted proof that his code was not okay and I tried to explain with a little test.

The thing I didn't like is that CheckLDWS_DONE and plaatsInArray are not initialized and in my opinion should have. These are now outputs that are written once when the looping is active.
But it is an FC, so the output area is just like the temp area not saved. In my opinion can have any value.

Code:

FUNCTION FC621 : VOID

VAR_INPUT
    REQ_checkLDWS:BOOL;
    
END_VAR

VAR_OUTPUT
    plaatsInArray: INT;
    CheckLDWS_DONE:BOOL;
END_VAR


VAR_TEMP
    k: INT;
    FOUND: BOOL;
    i: INT;
    j: INT;
    compareString:BOOL;
END_VAR


BEGIN

compareString:=false;

IF req_checkLDWS THEN
 i:=0;
FOR i:=-3 TO 5 DO
 j:=0;  
 FOUND:= false;    

REPEAT 
    
compareString:= EQ_Strng(S1:="DB619: LDWS_LijnBuffer".positie[i].transportNR, S2:="LDWSgelezenOpslag".TRNUMMERS[j]);

   IF compareString THEN
       
       FOUND:=true;
       plaatsInArray:=j;
       "DB619: LDWS_LijnBuffer".positie[i].LDWS_DATA_Gelezen:=true;
   END_IF;
    
    j:=j+1;
        
    UNTIL j >=11 OR FOUND
END_REPEAT;
 

END_FOR;  
END_IF;

As a test I tried to influence the local memory by writing things to LB0, LB1, and so on in the FC where the problem FC is called. The output parameter didn't change.
When I added the line:

plaatsInArray:= k;

Then the output of the FC did change when changing local bytes:

But without the line plaatsInArray:= k; nothing on the output changes.

I tought the Output area in an FC is also like temp memory and uses the local memory, but now it looks like, if you don't write to the Output area inside the FC, the Output will not change ?

I still think this conclusion is incorrect and there should be an initialisation of outputs in an FC when nothing else is writing to it...

Anyone sees clear in this case?

Thanks in advance,
Kind regards,
C

userxyz · Mar 7, 2019

Answer

Hi,

Well I found the answer in a Siemens manual

Any ideas how to proof this... I think I have bad luck in my tests that I am not in the critical random memory area to see chaos

kind regards,
C

L D[AR2P#0.0] · Mar 7, 2019

Have a read of this:

cardosocea · Mar 7, 2019

Depends on what is connected to the outputs of that function.

If you run it with a condition you know will never make compareString == true, then you can see what it does to the output of the function.

If the program was done by a single person, he may have had that in mind for the next blocks using this data... however, it is still bad practice.

kamenges · Mar 7, 2019

I don't use Siemens control systems very often. This is good to know. I have always been careful to make sure "input" information (items used to make logical decisions or as values used in later equations) have been operated on prior to use in a function. However, I've never made the logical leap to consider what that means to "output" data, which can be just as damaging. I will definitely keep this in mind for the future.

I think you may have a hard time testing this unless you get some detailed information about how the S7 operating system selects temporary memory for allocation. In effect you need to design a test where you know that multiple function instances that will definitely manipulate their temp data are all using the same section of processor memory. Without knowing how temp memory is allocated that could be hard to do.

Keith

userxyz · Mar 7, 2019

Thanks

This was the code, I just saw that I posted the wrong code.
Anyway, thanks for the example, I will forward this as proof to my collegue.

Code:

FUNCTION FC621 : VOID

VAR_INPUT
    REQ_checkLDWS:BOOL;
    
END_VAR

VAR_OUTPUT
    plaatsInArray: INT;
    CheckLDWS_DONE:BOOL;
END_VAR


VAR_TEMP
    k: INT;
    FOUND: BOOL;
    i: INT;
    j: INT;
    compareString:BOOL;
END_VAR


BEGIN

compareString:=false;

IF req_checkLDWS THEN
 i:=0;
FOR i:=-3 TO 5 DO
 j:=0;  
 FOUND:= false;    

REPEAT 
    
compareString:= EQ_Strng(S1:="DB619: LDWS_LijnBuffer".positie[i].transportNR, S2:="LDWSgelezenOpslag".TRNUMMERS[j]);

   IF compareString THEN
       
       FOUND:=true;
       plaatsInArray:=j;
       "DB619: LDWS_LijnBuffer".positie[i].LDWS_DATA_Gelezen:=true;
   END_IF;
    
    j:=j+1;
        
    UNTIL j >=11 OR FOUND
END_REPEAT;

END_FOR;  
CheckLDWS_DONE:= true;
END_IF;

JesperMP · Mar 7, 2019

It is not pretty, but if the variable assigned to #CheckLDWS_DONE is set FALSE before the call of FC621, then after the call the state will indicate if the FC621 was run till the end, and that does say that the "check was done".

So FC621 and the output parameter #CheckLDWS_DONE works as intended.

userxyz · Mar 7, 2019

Prefer

I do prefer to make it IN/OUT or to initialise internally in FC621 so that the done bit is only true for one cycle for example.

JesperMP said:
It is not pretty, but if the variable assigned to #CheckLDWS_DONE is set FALSE before the call of FC621, then after the call the state will indicate if the FC621 was run till the end, and that does say that the "check was done".

So FC621 and the output parameter #CheckLDWS_DONE works as intended.

JesperMP · Mar 8, 2019

The IN parameter REQ_checkLDWS could indicate that FC621 is scanned cyclically, rather than only called once when the check has to be done.

Such functions I only call when needed, and then only for one scan. In that case the need for the input REQ_checkLDWS disappears. The variable that was assigned to REQ_checkLDWS instead calls FC621.
And you can use the ENO bit outside the block for a check that the block was executed. So that eliminiates the need for the output CheckLDWS_DONE.

Something like this.

Code:

   Do_check                      Check_done
------| |-------[FC621]------x------( )
                             |
                             |   Do_check
                             x------(R)

userxyz · Mar 8, 2019

Also

Also possible, thanks.

JesperMP said:
The IN parameter REQ_checkLDWS could indicate that FC621 is scanned cyclically, rather than only called once when the check has to be done.

Such functions I only call when needed, and then only for one scan. In that case the need for the input REQ_checkLDWS disappears. The variable that was assigned to REQ_checkLDWS instead calls FC621.
And you can use the ENO bit outside the block for a check that the block was executed. So that eliminiates the need for the output CheckLDWS_DONE.

Something like this.

Code:

Do_check Check_done ------| |-------[FC621]------x------( ) | | Do_check x------(R)

mk42 · Mar 8, 2019

Two things to note, although not necessarily affecting your scenario above:

1) If you're using a 1200/1500 with optimized code, it will automatically initialize the local memory used by the block when it is called (and I think at the end as well).

2) I couldn't find the reference in the manual, but the understanding that I've always had is that each priority level and nesting depth has its own separate local memory.
This means that OB1 has its local storage, which calls FC1. FC1 has its separate local storage which calls FC2. FC2 uses its own local storage. FC1 then calls FC3, which uses the same local storage as FC2. OB1 then calls FC4, which uses the same local memory as FC1. FC4 calls FC5, which uses the same local memory as FC2 & FC3.

You might need to call something before your colleagues FC to mess with the local memory, instead of messing with the OB's local memory.

wimpiesplc · Mar 10, 2019

So I got curious about this. A while a ago I made a sort of proof for almost the same issue with TEMP var's but not for OUT.
So I took your code and modified it a bit for testing purposes:

Code:

FUNCTION FC620 : VOID

VAR_INPUT
    REQ_checkLDWS:BOOL;
    
END_VAR

VAR_OUTPUT
    plaatsInArray: INT;
    CheckLDWS_DONE:BOOL;
END_VAR

VAR_TEMP
    k: INT;
    FOUND: BOOL;
    i: INT;
    j: INT;
    compareString:BOOL;
END_VAR


BEGIN

plaatsInArray:=999;
k:=888;
CheckLDWS_DONE:= false;

END_FUNCTION




FUNCTION FC621 : VOID

VAR_INPUT
    REQ_checkLDWS:BOOL;
    
END_VAR

VAR_OUTPUT
    plaatsInArray: INT;
    CheckLDWS_DONE:BOOL;
    out_k : INT;
END_VAR

VAR_TEMP
    k: INT;
    FOUND: BOOL;
    i: INT;
    j: INT;
    compareString:BOOL;
END_VAR


BEGIN

IF req_checkLDWS THEN
    plaatsInArray:=123;
    CheckLDWS_DONE:= true;
    k:=456;
END_IF;

out_k := k;

END_FUNCTION




FUNCTION FC622 : VOID

VAR_OUTPUT
    Int1 : INT;
    Int2 : INT;
END_VAR

BEGIN

Int2 := Int1;

END_FUNCTION

FC620: some values always written to
FC621: same values conditionally written to (different values)
FC622: write uninitialized output Int1 to Int2

Here's a screenshot running them in OB1:
(S7 v5.6 with PLCSIM v5.4.8)

VirtualBox_Win7 ult_lite 64_10_03_2019_16_26_50.png

Obeservation:
- uninitialized TEMP var uses the data still in the local memory from previous FC's
- uninitialized OUT var gets the data from the variable filled in at the FC call and seems to work like it was an IN_OUT var. (probably handled by the underlying code?)

Good day.

FC and Temp/Output area

userxyz

Member

userxyz

Member

L D[AR2P#0.0]

Lifetime Supporting Member

cardosocea

Member

kamenges

Member

userxyz

Member

JesperMP

Lifetime Supporting Member + Moderator

userxyz

Member

JesperMP

Lifetime Supporting Member + Moderator

userxyz

Member

mk42

Lifetime Supporting Member

wimpiesplc

Member

Similar Topics