critique?

ganutenator · Dec 10, 2016

Code:

(*convert rpm into inches per sec //1.5708 * rpm /1800 * 12"*)
BridgeWriteSpdInSec:= 1.5708 * INT_TO_REAL(Bridge_write_speed_rpm) /1800.0 * 12.0;

If (PassNum = 1) & ((MachineMotionStep = 120) or (MachineMotionStep = 130) ) Then
	RecordCar:= true;
Else
	RecordCar:= false;
End_if;

TimerForDist (IN := RecordCar & not OneInchReset,
    PT := t#10m);

If RecordCar Then
	RecordDist:= BridgeWriteSpdInSec * Time_To_Real(TimerForDist.et) / 1000.0; (*distance = rate x time*)
	If (RecordDist>= InchesCompare) Then
		sim_pulse:= true;
		RecordDist:= 0.0;
		(*OneInchReset:= true;*)
		InchesCompare:= InchesCompare + 1.0;
	Else
		sim_pulse:= false;
		OneInchReset:= false;
	End_if;	
end_if;



(*r_trigs*)
sim_pulse_re (CLK := sim_pulse);
RecordCarRE (CLK := RecordCar);



(*Determines what speed to run the Hi Press Pump for the Top*)
If RecordCar Then
	IF (not XP4_clear) THEN
		ProfileTopRec:= 5;
	ELSIF (not XP3_clear) THEN
		ProfileTopRec:= 4;
	ELSIF (not XP2_clear) THEN
		ProfileTopRec:= 3;
	ELSIF (not XP1_clear) THEN
		ProfileTopRec:= 2;
	else
		ProfileTopRec:= 1;	
	END_IF;
else
	ProfileTopRec:= 0;		
end_if;
	
	

If RecordCar & sim_pulse_re.q Then
	RecordedProfile[profPointer]:= ProfileTopRec;
	inc(profPointer);
End_if;

(*clear array at beginning of next recording*)
If RecordCarRE.q Then
	InchesCompare:= 1.0;
	profPointer:=0;
	RecClearPntr:=0;
	While RecClearPntr < 1001 Do
		RecordedProfile[RecClearPntr]:= 0;
		inc(RecClearPntr);
	End_while;
End_if;

janner_10 · Dec 11, 2016

Does it work?

robertmee · Dec 11, 2016

From a straight programming standpoint you don't need the if else for recordcar. Just assign recordcar the value of the expression....it'll either be 0 or 1.

Diameter157 · Dec 11, 2016

IT needs more comments explaining what the code does especially using/calling other functions/aio.

I also prefer to use constants instead of using raw numbers like 1800 and 1.5708 so yeah u have a better idea what the 1.5708 is and why. Especially in larger code if you need to change the number. Using ctrl-f and replacing 1800 with 3600 works, but what if somewhere else you use 1800 and it needed to stay 1800?

Do you really want some guy calling you at 2am in 5 years time asking why you used 1.5708 ?

Whereas if you would call your constant rad_to_deg, he may not need to call you.

Im guessing machinemotionstep refers to another grafcet/sfc, again I much prefer to use constants (unsigned integers), just in case the unlikely event you need to add 10 extra steps. Also if (MixingSFC = AddDryIngredients) means a lot more to me than than if (MixingSFC = 110)

critique?

ganutenator

Lifetime Supporting Member

janner_10

Lifetime Supporting Member

robertmee

Lifetime Supporting Member

Diameter157

Member

Similar Topics