FILTER_MAV_DW wrong calculation

RWTH-EBC / AixOCAT

A repository combining OSCAT community with sophisticated automation communication structures

MIT License

77 stars 28 forks source link

FILTER_MAV_DW wrong calculation #71

Open pldroit opened 3 years ago

pldroit commented 3 years ago

What is the issue/what feature do you want to develop?

FILTER_MAV_DW calculates wrong moving average values when input value decreases

here a small example run with codesys 3.5 SP17

PROGRAM POU
VAR
    mav : OSCAT_BASIC.FILTER_MAV_DW;
END_VAR

mav(X := 10, N := 2, RST := FALSE);
mav(X := 8);
IF mav.Y <> 9 THEN
    mav.X := mav.X; // we should never get here
END_IF

What are the requirements?

codesys 3.5
N > 1
RST = FALSE
X value decreases

How do we want to solve it?

the problem is an underflow of the interim result (X - buffer[i]) https://github.com/RWTH-EBC/AixOCAT/blob/b34ffd698296056d62b585420758c5a2c3b77f74/OSCAT/OSCAT/OSCAT/oscat_basic/POUs/Engineering/signal%20processing/FILTER_MAV_DW.TcPOU#L45
changing the divisor to the signed variable tmp solves the issue but triggers various conversion warnings

Maghnie commented 3 years ago

Thank you @pldroit for the notification. We looked into it, and your initial guess about the problem cause seems right. To better solve the issue, could you please tell use what is your use-case for using the function with DWORD instead of REAL?

pldroit commented 3 years ago

Hi, many fieldbus systems don't support REAL types. It's common to sent values as integer types like DINT with a defined scale and unit (eg. 3 decimal places and volt). If you want to filter this input data to reduce noise, you would use this function.

Maghnie commented 2 years ago

@LZimmermannLZI This is also interesting for the unit tests initial runs

StGoebel commented 8 months ago

Hi @Maghnie, did you solve the problem and can we close this issue?

Maghnie commented 8 months ago

Thanks @StGoebel for the reminder!

Unfortunately, we didn't get to solve this bug when we looked into it the first time. And the person who was working on this issue already left the team.

I did a little memory-refreshing now, and it appears that there was a similar bug in the WORD variant of this moving average function. However, it had been solved:

https://github.com/RWTH-EBC/AixOCAT/blob/b34ffd698296056d62b585420758c5a2c3b77f74/OSCAT/OSCAT/OSCAT/oscat_basic/POUs/Engineering/signal%20processing/FILTER_MAV_W.TcPOU#L35C1-L49C17

For the next step, I'd suggest testing if the fix from FILTER_MAV_W works in this function as well. I could do that, but I'd have to set up my Twincat environment again, so that might get scheduled until after a couple of weeks.