How to fix a bug on my queue of devices in PLC

Question

I am trying to create a simple queue in .st with 6 devices that should be turned on and off in the queue order only the ones that are available should be connected. For example I did a test with 6 devices available and then I was unavailable one by one but always the last one does not turn off at the output and leaves the program stopped. I use the OpenPCS IDE of infoteam.


VAR_INPUT

    ENABLE                      :   BOOL                    ;                   
    STATE_DEVICE1               :   BOOL                    ;                   
    STATE_DEVICE2               :   BOOL                    ;                   
    STATE_DEVICE3               :   BOOL                    ;                   
    STATE_DEVICE4               :   BOOL                    ;                       
    STATE_DEVICE5               :   BOOL                    ;                   
    STATE_DEVICE6               :   BOOL                    ;                   
    NUMBER_DEVICES              :   USINT                   ;                   
    POWER_REQUEST               :   USINT                   ;                   

END_VAR


VAR_OUTPUT

    REQUEST_DEVICE1             :   BOOL                    ;                   
    REQUEST_DEVICE2             :   BOOL                    ;                   
    REQUEST_DEVICE3             :   BOOL                    ;                   
    REQUEST_DEVICE4             :   BOOL                    ;                   
    REQUEST_DEVICE5             :   BOOL                    ;                   
    REQUEST_DEVICE6             :   BOOL                    ;                   

END_VAR


VAR

    STATE_DEVICES_ARR           :   ARRAY[1..6] OF BOOL     ;   
    REQUEST_DEVICES_ARR         :   ARRAY[1..6] OF BOOL     ;   

    NUMBER_DEVICES_STATE        :   USINT                   ;                   

    NUM_DEV_REAL                :   USINT                   ;                   
    NUM_DEV_ON                  :   USINT                   ;                   

    DEVICES_TO_ON               :   USINT                   ;                   
    DEVICES_TO_OFF              :   USINT                   ;                   

    P_ON                        :   USINT   :=  0           ;           
    P_OFF                       :   USINT   :=  0           ;           

    COUNT                       :   USINT                   ;

END_VAR

IF ENABLE = TRUE THEN                               


    STATE_DEVICES_ARR[1] := STATE_DEVICE1;                          
    STATE_DEVICES_ARR[2] := STATE_DEVICE2;
    STATE_DEVICES_ARR[3] := STATE_DEVICE3;
    STATE_DEVICES_ARR[4] := STATE_DEVICE4;
    STATE_DEVICES_ARR[5] := STATE_DEVICE5;
    STATE_DEVICES_ARR[6] := STATE_DEVICE6;

    NUM_DEV_ON := 0;                                

    FOR COUNT := 1 TO 6 DO
        IF STATE_DEVICES_ARR[COUNT] = FALSE THEN                    
            REQUEST_DEVICES_ARR[COUNT] := FALSE;                    
        END_IF;

        IF STATE_DEVICES_ARR[COUNT] = TRUE THEN                 
            NUMBER_DEVICES_STATE := NUMBER_DEVICES_STATE + 1;           
        END_IF;

        IF REQUEST_DEVICES_ARR[COUNT] = TRUE THEN
            DEVICES_TO_ON := DEVICES_TO_ON + 1;             
        END_IF;
    END_FOR;




    IF POWER_REQUEST > NUM_DEV_ON THEN                  
        DEVICES_TO_ON   := POWER_REQUEST-NUM_DEV_ON;            
        DEVICES_TO_OFF  := 0;                           
    END_IF;

    IF POWER_REQUEST < NUM_DEV_ON THEN                  
        DEVICES_TO_ON   := 0;                           
        DEVICES_TO_OFF  := NUM_DEV_ON-POWER_REQUEST;            
    END_IF;

    IF POWER_REQUEST = NUM_DEV_ON THEN                  
        DEVICES_TO_ON   := 0;                           
        DEVICES_TO_OFF  := 0;                           
    END_IF;



    IF NUMBER_DEVICES_STATE = 0 THEN
        DEVICES_TO_ON := 0;
    END_IF;


(*===============================================================================================================*)
(*switches the devices on or off according to FIFO logic.*)
(*===============================================================================================================*)

    IF DEVICES_TO_ON > 0 THEN                           (* check if a device was requested to connect*)
        WHILE DEVICES_TO_ON >   0 DO                    (* as long as there are devices to be connected *)
            P_ON := P_ON + 1;                           (* increase the "pointer" connect devices *)
            IF P_ON > 6 THEN                            (* check if the pointer position is at the end of the device queue *)
                P_ON :=1;                               (* if it is at the end, it returns to the start *)
            END_IF;

            IF STATE_DEVICES_ARR[P_ON] = TRUE THEN      (* check if the device is available to be connected *)              
                REQUEST_DEVICES_ARR[P_ON] := TRUE;      (* connect the device of position P_ON *)                           
                DEVICES_TO_ON := DEVICES_TO_ON-1;       (* decrements the number of devices to be connected *)  
            END_IF;
        END_WHILE;
    END_IF;

    IF DEVICES_TO_OFF > 0 THEN                          (* check if you are asked to disconnect from some device *)
        WHILE DEVICES_TO_OFF >  0 DO                    (* as long as there are devices to be switched off *)
            P_OFF := P_OFF + 1;                         (* increments the "pointer" to turn off devices *)
            IF P_OFF > 6 THEN                           (* check if the pointer position is at the end of the device queue *)
                P_OFF :=1;                              (* check if the pointer position is at the end of the device queue *)
            END_IF;

            IF STATE_DEVICES_ARR[P_OFF] = TRUE THEN     (* check if the device is available to be switched off *)   
                REQUEST_DEVICES_ARR[P_OFF] := FALSE;    (* disconnect device from position P_OFF *)       
                DEVICES_TO_OFF := DEVICES_TO_OFF-1;     (* decrements the number of devices to be disconnected *)
            END_IF;
        END_WHILE;
    END_IF;

    (* I THINK THE BUG WAS HERE *)


    REQUEST_DEVICE1 := REQUEST_DEVICES_ARR[1];                          
    REQUEST_DEVICE2 := REQUEST_DEVICES_ARR[2];
    REQUEST_DEVICE3 := REQUEST_DEVICES_ARR[3];
    REQUEST_DEVICE4 := REQUEST_DEVICES_ARR[4];
    REQUEST_DEVICE5 := REQUEST_DEVICES_ARR[5];
    REQUEST_DEVICE6 := REQUEST_DEVICES_ARR[6];


END_IF;


IF ENABLE = FALSE THEN                              

    REQUEST_DEVICE1 := FALSE;                               
    REQUEST_DEVICE2 := FALSE;
    REQUEST_DEVICE3 := FALSE;
    REQUEST_DEVICE4 := FALSE;
    REQUEST_DEVICE5 := FALSE;
    REQUEST_DEVICE6 := FALSE;

END_IF;
;

Sergey Romanov · Accepted Answer · 2019-05-24T04:30:06.140

There are many things to improve in your code. For instance:

IF REQUEST_DEVICES_ARR[COUNT] = TRUE THEN
    DEVICES_TO_ON := DEVICES_TO_ON + 1;             
END_IF;

This is senseless because right after it you override DEVICES_TO_ON and do not use it. So why would you set it?

Or you do this

IF POWER_REQUEST > NUM_DEV_ON THEN                  
    DEVICES_TO_ON   := POWER_REQUEST-NUM_DEV_ON;            
    DEVICES_TO_OFF  := 0;                           
END_IF;

But nowhere before you set NUM_DEV_ON.

Or you have an input variable NUMBER_DEVICES but nowhere used in the code.

But Generally, you've chosen the wrong approach to the problem.

So, first of all, you have to create a type

TYPE MY_DEVICE: STRUCT
        Available: BOOL; (* If a device is available *)
        State: BOOL; (* Current device state *)
        Queue: BOOL; (* What to do with device *)
    END_STRUCT
END_TYPE

Then set global variables

VAR_GLOBAL
    garDevices: ARARY[1.._DEVICE_NUM] OF MY_DEVICE; (* Comment *)
END_VAR

VAR_GLOBAL CONSTANT
    _DEVICE_NUM: USINT := 6; (* Comment *)
END_VAR

This way you can change the number of devices by simply changing _DEVICE_NUM constant without changing the rest of the code.

Now your function

FUNCTION QUEUE_DEVICES: BOOL
    VAR_INPUT
        ENABLE        : BOOL;                     
        POWER_REQUEST : USINT;                   
    END_VAR

    VAR
        iDeviceOnOff: INT;
        usiCount: USINT;
        usiCountOnDevices: USINT;
    END_VAR

    (* If not enabled, set all devices to turn off and quite function *)
    IF NOT ENABLE THEN 
        FOR usiCount TO _DEVICE_NUM DO
            garDevices[usiCount].Queue := FALSE;
        END_FOR;

        RETURN;
    END_IF;                              

    (* Count how many devices is on already *)
    usiCountOnDevices := 0;                                
    FOR usiCount := 1 TO _DEVICE_NUM DO
        IF garDevices[usiCount].State THEN
            usiCountOnDevices := usiCountOnDevices + 1;
        END_IF;
    END_FOR;

    (* Find the difference between power request and power on. 
    Might be negative or positive *)
    iDeviceOnOff := POWER_REQUEST - usiCountOnDevices; 

    FOR usiCount := 1 TO _DEVICE_NUM DO
        (* If device is not available for turning on or off 
        continue to the other device *)
        IF garDevices[usiCount].Available THEN
            (* if iDeviceOnOff is positive, then we have to turn on devices *)
            IF iDeviceOnOff > 0 AND NOT garDevices[usiCount].Queue THEN
                garDevices[usiCount].Queue := TRUE;
                iDeviceOnOff := iDeviceOnOff - 1;
            END_IF;

            (* if iDeviceOnOff is negative we have to turn OFF devices *)
            IF iDeviceOnOff < 0 AND garDevices[usiCount].Queue THEN
                garDevices[usiCount].Queue := FALSE;
                iDeviceOnOff := iDeviceOnOff + 1;
            END_IF;

            (* If iDeviceOnOff is 0 means balance is reached *)
            IF iDeviceOnOff = 0 THEN
                EXIT;
            END_IF;
        END_IF;
    END_FOR;
END_FUNCTION

Then, you can add some other tests at the end of the function. For example.

IF iDeviceOnOff > 0 THEN
   _ERROR: = 'More power requested than available devices';
END_IF;
IF iDeviceOnOff < 0 THEN
   _ERROR: = 'There is a power excess';
END_IF;

Thank you Mr Romanov. In ``` IF NOT garDevices[usiCount].Available THEN CONTINUE; END_IF; ``` word "CONTINUE" dont work because OpenPCS IDE of infoteam do not support, i need a expression. — Apach3, May 22 '19 at 17:03
I've changed the code without `CONTINUE`, and man, I spent quite some time to express you my decades of experience. — Sergey Romanov, May 24 '19 at 04:30
I'm starting now in the plc world, even this is a school work. I tried to make a condition out of your approach but I did not succeed. Thanks a lot for the help. — Apach3, May 24 '19 at 18:44

Alex Marks · Answer 2 · 2019-05-24T16:15:51.940

Programming in a PLC is a bit different than in a normal OS application where the application runs once. It looks to me like you have alot of while and for loops, and I don't think these would be necessary.

Looks, to me, like your program could benefit from the use of a state machine. You can also use enumerated types to improve readability of state machines. This is a simple concept used to control sequences of events in a PLC. Simple State Machine example:

    PROGRAM MAIN
    VAR
        bPizzaToCook : BOOL;
        bPizzaCooking : BOOL;
        bPizzaCooked : BOOL;
        bLoadPizza : BOOL;
        bUnloadPizza : BOOL;
        fb_t_CookTimer : TON;
        iPizzasCooked : UDINT;
        bBuzzer : BOOL;
        iPizzaState : DINT;
        sPizzaState : STRING;
    END_VAR
    IF bPizzaToCook AND NOT bPizzaCooking THEN
    //simulates conveyor moving pizza into oven
        bPizzaToCook := FALSE;
        bPizzaCooking := TRUE;
    END_IF
    IF bLoadPizza THEN  //pizza loaded onto conveyor
        bLoadPizza := FALSE;
        bPizzaToCook := TRUE;
    END_IF
    IF bUnloadPizza THEN    //pizza unloaded off of conveyor
        bUnloadPizza := FALSE;
        bPizzaCooked := FALSE;
    END_IF

    CASE iOvenState OF
        0 : //wait for pizza to cook
            sPizzaState := ‘Waiting for pizza…’;
            IF bPizzaCooking THEN
                iPizzaState := 10;
            END_IF
        10: //cook the pizza (start the timer)
            sPizzaState := ‘Baking Pizza…’
            fb_t_CookTimer(INT := TRUE, PT := T#10s);
            IF fb_t_CookTimer.Q THEN
                fb_t_CookTimer(IN := FALSE);
                iPizzaState := 20;
            END_IF

        20: //is there space to move pizza out of oven?
            IF NOT bPizzaCooked THEN
                bPizzaCooking := FALSE;
                bPizzaCooked := TRUE;
                bBuzzer := FALSE;
                iPizzaState := 30;
            ELSE    //pizza burning
                sPizzaState := ‘BURNING’;
                bBuzzer := TRUE;
            END_IF
        30://pizza ready on conveyor
            iPizzasCooked := iPizzasCooked + 1;
            iPizzaState := 0;               //reset state
    ELSE
        sPizzaState := ‘Invalid State!’;
    END_CASE

Plenty of other state machine examples out there. This one is from https://www.youtube.com/watch?v=XmcXRZXPRWs

By the rules, you cannot in the answer just link to another page or youtube video. Your answer can contain links but it has to contain the answer itself. So you have to explain the concept here in the comment so that watching a video might be optional. — Sergey Romanov, May 24 '19 at 04:33

How to fix a bug on my queue of devices in PLC

2 Answers2