Really Long Parameter Logic

david.stanford7 · October 11, 2017, 1:02am

So I crack open a code block and find the code shown below the line. What do you think is a better way to handle the logic shown in the “if” statements?

The toggle variables are HMI input. For example:

If Pressure_Pump_Toggle = 1 and Pressure_Pump_Toggle_SS == 0 the pump is in auto.

But if Pressure_Pump_Toggle == 1 Pressure_Pump_Toggle_SS == 1 it is manual on until the operator turns it off.

If Pressure_Pump_Toggle == 0 the pump won’t run at all.

if (High_Pressure_Flag == 1 or IsOn(Fire_Pump_SS) or HasTimerExpired (dt_Pressure_Pump_Idle_Time) or Pressure_Pump_Toggle == 0 and Pressure_Pump_Toggle_SS == 0 or Storage_Tank_Level <= 2) then

Pressure_Pump_SS = 0; // Turn off the pump

endif

if (High_Pressure_Flag == 0 and Pressure_Pump_Toggle == 1 and dt_Pressure_Pump_Idle_Time > 0 and IsOff(Fire_Pump_SS) or Pressure_Pump_Toggle_SS == 1 and High_Pressure_Flag == 0 and Storage_Tank_Level > 2) then

Pressure_Pump_SS = 1; // Turn On the pump

endif

philip · October 11, 2017, 3:50am

You could break the statement into multiple lines as below. You could also pull it all out of Optoscript and use condition blocks if you really wanted the process to be visually clear.
I added some parentheses to make the order of operations a little more explicit (there is no change in the logic).
I’m not sure what all this logic does (hopefully the down timer is protecting the pump from cycling), but I would think about adding some hysteresis on the Storage_Tank_Level.

if (High_Pressure_Flag == 1 
	or IsOn(Fire_Pump_SS)
	or HasTimerExpired (dt_Pressure_Pump_Idle_Time)
	or (Pressure_Pump_Toggle == 0 and Pressure_Pump_Toggle_SS == 0)
	or Storage_Tank_Level <= 2
	) then
		Pressure_Pump_SS = 0; // Turn off the pump
endif

if ((High_Pressure_Flag == 0 
	and Pressure_Pump_Toggle == 1 
	and dt_Pressure_Pump_Idle_Time > 0 
	and IsOff(Fire_Pump_SS))
	or 
	(Pressure_Pump_Toggle_SS == 1 
	and High_Pressure_Flag == 0 
	and Storage_Tank_Level > 2
	)) then
		Pressure_Pump_SS = 1; // Turn On the pump
endif

Good luck!

david.stanford7 · October 11, 2017, 3:18pm

Yes, the Storage_Tank variable is what brought me to this code. Hysteresis was indeed a problem that’s been fixed. I like your manner of typing out the logic. Anything to make it more understandable. Thank you for your input Philip.

philip · October 11, 2017, 4:08pm

You’re welcome David.

alanlevezu · October 11, 2017, 7:31pm

So here’s a long response… with a couple options to make it more readable.

I’m a big believer in internal variables that tell you what’s going on.

Note that the first assumption I am making is that the HMI is truly toggling between 0 and 1 for the variables named as such. If that’s true, logic writing can be simplified because a value that is zero is considered false, and any non-zero value is considered to be true. This also applies to digital I/O points, the “IsOn” is nice because it lets us know that it’s actually an I/O point, but it’s not required.

Correct me if I’m wrong, but the assumption is that when the pump is in auto, you want the tank to stay above a specific setpoint, and to get there, you want to run it for a set time so you’re above the setpoint and then it can drop back down.

First: without a flag variable – You don’t say when the timer is being set, but if that’s truly what you want you can simply set the down timer when it needs to run. As long as it’s below the setpoint the timer will be continually reset.

if (Storage_Tank_Level <= Low_Limit_Setpoint) then 
  dt_Pressure_Pump_Idle_Time = Time_To_Run;
endif

if you want to add a hysteresis you can do that by modifying the above

if (Storage_Tank_Level <= Low_Limit_Setpoint) then 
  dt_Pressure_Pump_Idle_Time = Time_To_Run;
elseif (    Storage_Tank_Level <= Low_Limit_Setpoint + Hysteresis_Setpoint 
        and not HasTimerExpired(dt_Pressure_Pump_Idle_Time)
        ) then
  dt_Pressure_Pump_Idle_Time = Time_To_Run;
endif

But, better yet this could be done with a new flag variable (Auto_Pump_Desired_Flag) and make it more understandable, plus it would allow you to combine the timer with the hysteresis better:

if (Storage_Tank_Level <= Low_Limit_Setpoint) then 
  SetVariableTrue(Auto_Pump_Desired_Flag);
  dt_Pressure_Pump_Idle_Time = Time_To_Run;
elseif ( Storage_Tank_Level >= Low_Limit_Setpoint + Hysteresis_Setpoint) then
  SetVariableFalse(Auto_Pump_Desired_Flag);
endif

Based on the logic, even in “Manual” mode, it will not run if High_Pressure_Flag is running, or if the Fire_Pump_SS is true, (or actually if the Storage_Tank_Level is too low either, not sure whether that is by intent or not). Therefore, you might want to add a Pump_Allowed “flag” variable:

Pump_Allowed_Flag = not (   High_Pressure_Flag 
                         or Fire_Pump_SS);
// You may need to add more reasons to run here... your original code has more (e.g. tank level must be above 2 and timer can't be expired) but those seemed incorrect based on your description at the beginning of the message.

Next:
I often use “mode” variables that allow you to set a value and thus display it for HMI, as well as see it instantly when debugging.

for instance, in your case: an option to handle the logic for an HOA is to assign an internal “mode” variable.

if (not Pressure_Pump_Toggle) then
  Pump_Mode = 0; // (Off Mode)
elseif (Pressure_Pump_Toggle_SS) then  // In order to get here, Pressure_Pump_Toggle must not be zero
 Pump_Mode = 1; // (Manual Mode)
else
 Pump_Mode = 2; // (Auto Mode)
endif

Then you can use a switch statement

switch (Pump_Mode)

  case 1: // Manual Mode
   Pressure_Pump_SS = Pump_Allowed_Flag;
   break

  case 0: // Off Mode
   SetVariableFalse(Pressure_Pump_SS);

  default: // must be in Auto Mode
    Pressure_Pump_SS = Auto_Pump_Desired_Flag and Pump_Allowed_Flag and not     
    break

endswitch

Note that this would work basically the same if Pressure_Pump_SS is a digital output, but I’d change the Off Mode to TurnOff() instead of SetVariableFalse().

BTW: Looking at your existing logic:
if Pressure_Pump_SS is actually an I/O point, right now it is possible to chatter it (e.g. have it go off, then on again) while executing the current logic: if the pump is in “Manual” and the timer is expired, the first if will turn it off, and the second if will turn it right back on. Since Opto 22 does things with I/O immediately, that will actually turn the point off for a few milliseconds then turn it back on.

philip · October 11, 2017, 7:40pm

Good catch sir. You get a like from me.

david.stanford7 · October 12, 2017, 3:32pm

Alan you have some interesting ideas that I’ll certainly use. Some others don’t apply. For further clarification this is a pressure pump that draws from a 120,000 gallon tank. The Storage_Tank level cuts off the pump if it’s low (cutoff points have hysteresis now to avoid chatter). If High_Pressure_Flag = 1 it shuts down or won’t come on, same with Fire_Pump_SS. The dt_Pressure_Pump_Idle_Time down timer is unique, if the VFD stays to idle for 15 minutes it shuts down, usually at night. Logic for these variables is handled in an earlier block. Absolute numbers (2) have been replaced with variables. Fire_Pump_SS is going to be replaced with a flag since Fire_Pump_SS is an actual output.