Search code examples
plccodesysstructured-textsiemens

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


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;
;




Solution

  • 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;