Opinions on methods

g.mccormick · Jan 8, 2021

I would like to use this thread to ask for opinions on methods/practices. I am starting on a fairly large complete rewrite of code for several systems and I would like to "do it correctly". So, I ask for some opinions here.

1. I have an AOI for dealing with analog inputs. This AOI does scaling, filtering, alarming, etc. The systems I am working on have between 16 and 64 analog inputs. Do you have individual instances for each AOI call, or just reuse the same instance? I can see an argument for each way. If individual instances and you're not using in a loop, I can see the values in code for each instance. This could help with troubleshooting, but I also can/will have the AOI data type be equal to my analog input data type and pass by reference.
If I just use one instance, I save creating a lot of unused tags.

More questions will come as I think of them.

L D[AR2P#0.0] · Jan 8, 2021

Not familiar with AOI but if you re-use the same instance will the filtering still work?

ndzied1 · Jan 8, 2021

I believe best practice would be to have an instance of the AOI for each analog input. As LD said, I'm not sure how it would work otherwise.

The number of tags you need will depend on how much configuration you give down the line.

If the same filtering would work for all analogs then you can encapsulate it within the AOI (no external tags required). If you want to configure the filtering at startup but don't want to give the ability to change it later through the programming then you can send constants into the AOI instances at startup. If you need the ability to change the filtering pragmatically later then you would need to assign tags to your filter parameters.

Likewise with the scaling.

If you want tags for everything, then I would look to using arrays and possibly arrays of UDTs to make things easier to look at in the program.

g.mccormick · Jan 8, 2021

L D[AR2 said:
Not familiar with AOI but if you re-use the same instance will the filtering still work?

Yes. The INPUT/OUTPUT UDT holds the values and is passed by reference. The AOI grabs the last value first before doing any scaling etc.

// AOI to scale an input, filter and limit its output.

// First we want to remember the last output value. Used in the filter calculation.
LastEngVal := UDT_DataTag.Engr_Val;
LastSigVal := UDT_DataTag.Signal_Val;

James Mcquade · Jan 8, 2021

use an aoi for each input. yes, you add code, BUT it's easier on maintenance to understand and troubleshoot at 2 am. if they can fix the issue, no 2 am phone calls.
james

parky · Jan 8, 2021

I think the use of an AOI (FB) as one instance is fine providing things like filtering etc. is passed both ways i.e. IN/OUT parameters, the idea is that you debug the code in the AOI before using it in earnest so there should be no need to look at it again if it causes problems then you have not tested it properly as it should work the same for all calls. If there is a problem then just create a separate instance and find the problem. In saying that, I see no reason other than saving program memory not to have separate instances. In actual fact all an instance is just a macro in effect (saves you typing).

cjd1965 · Jan 8, 2021

Use a single AOI once for every analog input

Pass a UDT tag containing all required scaling/alarm setpoints etc where each UDT represents an analog input and all its properties

This tag structure will become the equivalent of the siemens DB

g.mccormick · Jan 8, 2021

parky said:
I think the use of an AOI (FB) as one instance is fine providing things like filtering etc. is passed both ways i.e. IN/OUT parameters, the idea is that you debug the code in the AOI before using it in earnest so there should be no need to look at it again if it causes problems then you have not tested it properly as it should work the same for all calls. If there is a problem then just create a separate instance and find the problem. In saying that, I see no reason other than saving program memory not to have separate instances. In actual fact all an instance is just a macro in effect (saves you typing).

Yes I planned to define the AOI with UDT as inout parameter. That is currently how I have it.
The thing that bugs me about having an instance each, other than wasted tag space is that effectively all of the data is in two places. The values are in the AOI data tag and in the the In, out, and/or In/Out tag/udt.

I guess that does pose a new question, does anyone just name/use the AOI tag as the "main" tag?

g.mccormick · Jan 8, 2021

cjd1965 said:
Use a single AOI once for every analog input

Pass a UDT tag containing all required scaling/alarm setpoints etc where each UDT represents an analog input and all its properties

This tag structure will become the equivalent of the siemens DB

Just to clarify, you are saying only one instance reused correct?

ndzied1 · Jan 8, 2021

Here's what I would do. This is definitely a simplified version of what you describe (and hasn't been tested) and is in CoDeSys but the ideas apply.

Define a UDT with the filtering and scaling parameters

Code:

TYPE AnalogParameters :
STRUCT
	rFilterTimeConst:	REAL;
	wScale_In_Min:		WORD;
	wScale_In_Max:		WORD;
	rScale_Out_Min:		REAL;
	rScale_Out_Max:		REAL;
END_STRUCT
END_TYPE

Define an AOI (Function Block)

Code:

FUNCTION_BLOCK AnalogConditioning
VAR_INPUT
	wRawValue:		WORD;
	udtParam:		AnalogParameters;
END_VAR
VAR_OUTPUT
	rScaledValue:	REAL;
END_VAR
VAR
	rThisScaled:	REAL;
	rLastFiltered:	REAL;
END_VAR

// Scale the value - Check for division by zero
IF (udtParam.wScale_In_Max-udtParam.wScale_In_Min) <> 0 THEN
rThisScaled :=	udtParam.rScale_Out_Min +
				wRawValue * (
				(udtParam.rScale_Out_Max - udtparam.rScale_Out_Min)/(udtParam.wScale_In_Max-udtParam.wScale_In_Min)
				);
ELSE
	rThisScaled := 0.0;
END_IF

// Filter the value and save last for next iteration
rScaledValue	:= rLastFiltered + udtParam.rFilterTimeConst * (rThisScaled-rLastFiltered);
rLastFiltered	:= rScaledValue;

Define arrays for everything in the program and call in a FOR loop. Note that the maintenance guys are going to hate this method. Individual calls would be much more visual and allow you to see the values in the program vs. having to monitor the tags to see them. But you seem to want to minimize typing so

Code:

PROGRAM Main
VAR
	Analogs:	ARRAY [1..10] OF AnalogConditioning;
	AnalogParam:	ARRAY [1..10] OF Analogparameters;
	wRawAnalog:	ARRAY [1..10] OF WORD;
	rScaledAnalog:	ARRAY [1..10] OF REAL;
	
	iIndex:			int;
END_VAR


FOR iIndex:= 1 TO 10 DO
	Analogs[iIndex](wRawValue:= wRawAnalog[iIndex], udtParam:=AnalogParam[iIndex] , rScaledValue=>rScaledAnalog[iIndex] );
END_FOR

willxfmr · Jan 8, 2021

If I were the one doing the troubleshooting, I would prefer one instance of the AOI for each signal. Of course you will likely encounter some of my kin folk that will ask,
"AO what?" I apologize in advance on their behalf.

Bubba.

GaryS · Jan 9, 2021

An AOI for each input would be the best way to go
But why the analog cards / modules have all that built is you just have to set it up
I can't believe you can do it better in an AOI then then can in hardware

Opinions on methods

g.mccormick

Lifetime Supporting Member

L D[AR2P#0.0]

Lifetime Supporting Member

ndzied1

Lifetime Supporting Member

g.mccormick

Lifetime Supporting Member

James Mcquade

Member

parky

Member

cjd1965

Lifetime Supporting Member

g.mccormick

Lifetime Supporting Member

g.mccormick

Lifetime Supporting Member

ndzied1

Lifetime Supporting Member

willxfmr

Lifetime Supporting Member

GaryS

Member

Similar Topics