Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] LowPassFilter returns wrong values at startup #433

Open
sylque opened this issue Sep 26, 2024 · 2 comments
Open

[BUG] LowPassFilter returns wrong values at startup #433

sylque opened this issue Sep 26, 2024 · 2 comments

Comments

@sylque
Copy link

sylque commented Sep 26, 2024

Not sure what kind of actual impact this might have on the rest of the library, but I believe there is something wrong in the LowPassFilter class:

LowPassFilter::LowPassFilter(float time_constant)
    : ...
    , y_prev(0.0f)
{
    ...
}


float LowPassFilter::operator() (float x)
{
    ...
    float y = alpha*y_prev + (1.0f - alpha)*x;
    y_prev = y;
	...
    return y;
}

Because y_prev is initialized to zero, the first calls to operator() returns the wrong result. At the first call, y_prev should probably be initialized with x instead of zero. In other words, the first call to LowPassFilter(5) should return 5 and not a weighted average of 5 with zero.

Notice that, in LowPassFilter::operator(), you can also see:

if(dt > 0.3f) {
        y_prev = x;
        timestamp_prev = timestamp;
        return x;
    }

So if you are lucky enough to have spent 0.3 seconds between the class construction and the first call to operator(), then y_prev is correctly initialized.

@runger1101001
Copy link
Member

You're right, in principle. The code has been this way for a long time. I think it works well in practice because the assumption is more or less that the motor starts at 0, so x would be 0.
But in the case where the motor is moving when you start, or if you're applying the LPF to angle rather than velocity, it probably makes more sense to initialize it from x.

Another goal we have is to make the filter configurable, so you could swap it for a different kind of filter implementation. Maybe these issues can be solved at the same time by giving the user more control over the filter setup and initialisation.

@sylque
Copy link
Author

sylque commented Sep 30, 2024

Yes, I discovered this when investigating an issue with BLDCMotor::shaftAngle(): the result is wrong at startup because it uses the LPF with an angle which is never initially zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants